lkml.org 
[lkml]   [2021]   [Jan]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH v7 6/9] media: Add parsing for APP14 data segment in jpeg helpers
    From
    Date
    On Mon, 2021-01-11 at 21:28 +0200, Mirela Rabulea wrote:
    > From: Mirela Rabulea <mirela.rabulea@nxp.com>
    >
    > According to Rec. ITU-T T.872 (06/2012) 6.5.3
    > APP14 segment is for color encoding, it contains a transform flag, which
    > may have values of 0, 1 and 2 and are interpreted as follows:
    > 0 - CMYK for images that are encoded with four components
    > - RGB for images that are encoded with three components
    > 1 - An image encoded with three components using YCbCr colour encoding.
    > 2 - An image encoded with four components using YCCK colour encoding.
    >
    > This is used in imx-jpeg decoder, to distinguish between
    > YUV444 and RGB24.
    >
    > Signed-off-by: Mirela Rabulea <mirela.rabulea@nxp.com>
    > ---
    > Changes in v7:
    > Check there are 6 bytes available in the stream before checking for "Adobe\0"
    > Change jpeg_parse_app14_data function to differentiate between the 3 scenarios: app14 missing, or app14 present but with/without parsing errors:
    > App14 missing => Added V4L2_JPEG_APP14_TF_UNKNOWN to the enum v4l2_jpeg_app14_tf, use it to indicate app14 & TF is missing
    > App14 present without parsing errors => Return the transform flag value as enum v4l2_jpeg_app14_tf (new paramater of jpeg_parse_app14_data function)
    > App14 present with parsing errors => Return -EINVAL from jpeg_parse_app14_data, also return from calling function (v4l2_jpeg_parse_header) when this error is met.
    >
    > drivers/media/v4l2-core/v4l2-jpeg.c | 47 +++++++++++++++++++++++++++--
    > include/media/v4l2-jpeg.h | 20 ++++++++++++
    > 2 files changed, 65 insertions(+), 2 deletions(-)
    >
    > diff --git a/drivers/media/v4l2-core/v4l2-jpeg.c b/drivers/media/v4l2-core/v4l2-jpeg.c
    > index 8947fd95c6f1..8d5fedb136dd 100644
    > --- a/drivers/media/v4l2-core/v4l2-jpeg.c
    > +++ b/drivers/media/v4l2-core/v4l2-jpeg.c
    > @@ -45,6 +45,7 @@ MODULE_LICENSE("GPL");
    > #define DHP 0xffde /* hierarchical progression */
    > #define EXP 0xffdf /* expand reference */
    > #define APP0 0xffe0 /* application data */
    > +#define APP14 0xffee /* application data for colour encoding */
    > #define APP15 0xffef
    > #define JPG0 0xfff0 /* extensions */
    > #define JPG13 0xfffd
    > @@ -444,8 +445,44 @@ static int jpeg_skip_segment(struct jpeg_stream *stream)
    > return jpeg_skip(stream, len - 2);
    > }
    >
    > +/* Rec. ITU-T T.872 (06/2012) 6.5.3 */
    > +static int jpeg_parse_app14_data(struct jpeg_stream *stream,
    > + enum v4l2_jpeg_app14_tf *tf)
    > +{
    > + int ret;
    > + int lp;
    > + int skip;
    > +
    > + lp = jpeg_get_word_be(stream);
    > + if (lp < 0)
    > + return lp;
    > +
    > + /* Check for "Adobe\0" in Ap1..6 */
    > + if (stream->curr + 6 > stream->end ||
    > + strncmp(stream->curr, "Adobe\0", 6))
    > + return -EINVAL;
    > +
    > + /* get to Ap12 */
    > + ret = jpeg_skip(stream, 11);
    > + if (ret < 0)
    > + return ret;
    > +
    > + ret = jpeg_get_byte(stream);
    > + if (ret < 0)
    > + return ret;
    > +
    > + *tf = ret;
    > +
    > + skip = lp - 2 - 11;

    > + ret = jpeg_skip(stream, skip);
    > + if (ret < 0)
    > + return ret;
    > +
    > + return 0;

    This could be simplified to

    return jpeg_skip(stream, skip);

    although it would be better style to move the *tf = ... assignment down
    past the last error return instead. Either way,

    Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>

    regards
    Philipp

    \
     
     \ /
      Last update: 2021-01-12 16:01    [W:2.594 / U:0.224 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site