Re: [EXT] Re: [PATCH v5 07/10] media: Add parsing for APP14 data segment in jpeg helpers

[Philipp Zabel]

On Fri, 2020-12-04 at 14:13 +0000, Mirela Rabulea (OSS) wrote:
> Hi Phipipp,
>
> On Wed, 2020-12-02 at 16:18 +0100, Philipp Zabel wrote:
> > Hi Mirela,
> > 
> > On Thu, 2020-11-12 at 05:05 +0200, Mirela Rabulea (OSS) wrote:
> > > From: Mirela Rabulea <mirela.rabulea@xxxxxxx>
> > > 
> > > 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@xxxxxxx>
> > > ---
> > > Changes in v5:
> > > This was patch 8 in previous version
> > > Replaced a struct for app14 data with just an int, since the
> > > transform flag is the only meaningfull information from this
> > > segment
> > 
> > Could we turn this into an enum for the transform flag, and include
> > the
> > above spec reference in its kerneldoc comment? I think this would be
> > better than checking for (app14_tf == <magic_number>) in the drivers.
> 
> Appreciate your feedback, for all patches, I'll address it in v6.
> Where would be a better place for this enum, v4l2-jpeg.h, or maybe
> include/uapi/linux/v4l2-controls.h?

v4l2-jpeg.h seems like the right place to me.

regards
Philipp