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