Re: [PATCH v7 6/9] media: Add parsing for APP14 data segment in jpeg helpers
From: Philipp Zabel <p.zabel@pengutronix.de>
Date: 2021-01-12 14:59:29
Also in:
linux-media, lkml
On Mon, 2021-01-11 at 21:28 +0200, Mirela Rabulea wrote:
quoted hunk ↗ jump to hunk
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