[PATCH v2 3/4] media: imx-pxp: add i.MX Pixel Pipeline driver

[p.zabel@pengutronix.de (Philipp Zabel)]

On Thu, 2018-09-06 at 09:57 +0200, Hans Verkuil wrote:
[...]
> > If userspace sets xfer_func explicitly, it will get the explicit default
> > ycbcr_enc and quantization values.
> > 
> > I think I did this to make v4l2-compliance at some point, but it could
> > be that the explicit output->capture colorimetry copy for RGB->RGB and
> > YUV->YUV conversions has me covered now.
> 
> This xfer_func test makes no sense. xfer_func is completely ignored by the
> driver (other than copying it from output to capture queue) since it can't
> make any changes to it anyway.
> 
> What you are trying to do in pxp_fixup_colorimetry() is to figure out the
> ycbcr_enc and quantization values for the capture queue.

Yes. I checked again without that. Since there is the forced out->cap
copy for RGB->RGB and YUV->YUV conversions in pxp_fixup_colorimetry,
v4l2-compliance is happy anyway. The pxp_default_quant/ycbcr_enc
functions are removed now.

> BTW, can you rename pxp_fixup_colorimetry to pxp_fixup_colorimetry_cap or
> something? Since it is specifically for the capture queue.

Ok.

> These values depend entirely on the capture queue pixelformat and on the
> colorspace and not on the xfer_func value.
> 
> So just do:
> 
> bool is_rgb = !pxp_v4l2_pix_fmt_is_yuv(dst_fourcc);
> *ycbcr_enc = V4L2_MAP_YCBCR_ENC_DEFAULT(ctx->colorspace);
> *quantization = V4L2_MAP_QUANTIZATION_DEFAULT(is_rgb, ctx->colorspace,
> 					      *ycbcr_enc);

That's almost exactly what I ended up with, thank you.

> BTW, I just noticed that the V4L2_MAP_QUANTIZATION_DEFAULT macro no longer
> uses ycbcr_enc. The comment in videodev2.h should be updated. I can't
> change the define as it is used in applications (and we might need to
> depend on it again in the future anyway).
> 
> If this code will give you v4l2-compliance issues, please let me know.
> It shouldn't AFAICT.

There are no complaints anymore.

regards
Philipp