Hi Philipp,
On 04/04/2022 18:35, Philipp Zabel wrote:
> Set default colorspace to SRGB for JPEG encoder and decoder devices,
> to fix the following v4l2-compliance test failure:
>
> test VIDIOC_TRY_FMT: OK
> fail: v4l2-test-formats.cpp(818): fmt_raw.g_colorspace() != V4L2_COLORSPACE_SRGB
>
> Also explicitly set transfer function, YCbCr encoding and quantization
> range, as required by v4l2-compliance for the JPEG encoded side.
I'm not quite sure if this patch addresses the correct issue.
>
> Signed-off-by: Philipp Zabel <p.zabel@xxxxxxxxxxxxxx>
> ---
> .../media/platform/chips-media/coda-common.c | 36 +++++++++++++------
> 1 file changed, 25 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/media/platform/chips-media/coda-common.c b/drivers/media/platform/chips-media/coda-common.c
> index 4a7346ed771e..c068c16d1eb4 100644
> --- a/drivers/media/platform/chips-media/coda-common.c
> +++ b/drivers/media/platform/chips-media/coda-common.c
> @@ -732,13 +732,22 @@ static int coda_try_fmt_vid_cap(struct file *file, void *priv,
> return 0;
> }
>
> -static void coda_set_default_colorspace(struct v4l2_pix_format *fmt)
> +static void coda_set_default_colorspace(struct coda_ctx *ctx,
> + struct v4l2_pix_format *fmt)
> {
> enum v4l2_colorspace colorspace;
>
> - if (fmt->pixelformat == V4L2_PIX_FMT_JPEG)
> - colorspace = V4L2_COLORSPACE_JPEG;
It's perfectly fine to keep this, the problem occurs with the raw image side
(capture for the decoder, output for the encoder).
There the colorspace must be SRGB, the xfer_func may be 0 or SRGB, and the
ycbcr_enc is 0 (if not a YUV pixelformat) or ENC_601 (if it is a YUV format).
Actually, if the hardware can convert from other YUV encodings such as 709,
then other YUV encodings are valid, but I assume that's not the case.
> - else if (fmt->width <= 720 && fmt->height <= 576)
> + if (ctx->cvd->src_formats[0] == V4L2_PIX_FMT_JPEG ||
> + ctx->cvd->dst_formats[0] == V4L2_PIX_FMT_JPEG ||
> + fmt->pixelformat == V4L2_PIX_FMT_JPEG) {
> + fmt->colorspace = V4L2_COLORSPACE_SRGB;
> + fmt->xfer_func = V4L2_XFER_FUNC_SRGB;
> + fmt->ycbcr_enc = V4L2_YCBCR_ENC_601;
> + fmt->quantization = V4L2_QUANTIZATION_FULL_RANGE;
> + return;
> + }
> +
> + if (fmt->width <= 720 && fmt->height <= 576)
> colorspace = V4L2_COLORSPACE_SMPTE170M;
> else
> colorspace = V4L2_COLORSPACE_REC709;
> @@ -763,7 +772,7 @@ static int coda_try_fmt_vid_out(struct file *file, void *priv,
> return ret;
>
> if (f->fmt.pix.colorspace == V4L2_COLORSPACE_DEFAULT)
> - coda_set_default_colorspace(&f->fmt.pix);
> + coda_set_default_colorspace(ctx, &f->fmt.pix);
>
> q_data_dst = get_q_data(ctx, V4L2_BUF_TYPE_VIDEO_CAPTURE);
> codec = coda_find_codec(dev, f->fmt.pix.pixelformat, q_data_dst->fourcc);
> @@ -1640,13 +1649,18 @@ static void set_default_params(struct coda_ctx *ctx)
> csize = coda_estimate_sizeimage(ctx, usize, max_w, max_h);
>
> ctx->params.codec_mode = ctx->codec->mode;
> - if (ctx->cvd->src_formats[0] == V4L2_PIX_FMT_JPEG)
> - ctx->colorspace = V4L2_COLORSPACE_JPEG;
> - else
> + if (ctx->cvd->src_formats[0] == V4L2_PIX_FMT_JPEG ||
> + ctx->cvd->dst_formats[0] == V4L2_PIX_FMT_JPEG) {
> + ctx->colorspace = V4L2_COLORSPACE_SRGB;
> + ctx->xfer_func = V4L2_XFER_FUNC_SRGB;
> + ctx->ycbcr_enc = V4L2_YCBCR_ENC_601;
> + ctx->quantization = V4L2_QUANTIZATION_FULL_RANGE;
> + } else {
> ctx->colorspace = V4L2_COLORSPACE_REC709;
My guess is that the raw format colorspace is set to REC709, which is definitely
wrong for a JPEG codec. For a JPEG codec that must be set to SRGB.
I suspect that's the real bug here.
I'm skipping this patch for now.
Regards,
Hans
> - ctx->xfer_func = V4L2_XFER_FUNC_DEFAULT;
> - ctx->ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT;
> - ctx->quantization = V4L2_QUANTIZATION_DEFAULT;
> + ctx->xfer_func = V4L2_XFER_FUNC_DEFAULT;
> + ctx->ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT;
> + ctx->quantization = V4L2_QUANTIZATION_DEFAULT;
> + }
> ctx->params.framerate = 30;
>
> /* Default formats for output and input queues */
