LinuxTV Patchwork [08/10] media: coda: allow encoder to set colorimetry on the output queue

login
register
mail settings
Submitter Philipp Zabel
Date April 8, 2019, 12:32 p.m.
Message ID <20190408123256.22868-8-p.zabel@pengutronix.de>
Download mbox | patch
Permalink /patch/55535/
State Under Review
Delegated to: Hans Verkuil
Headers show

Comments

Philipp Zabel - April 8, 2019, 12:32 p.m.
v4l2-compliance sets colorimetry on the output queue and then verifies
that querying colorimetry on the capture queue returns the same
configuration. For this to work, the encoder must allow setting context
colorimetry parameters on the output queue.

Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
---
 drivers/media/platform/coda/coda-common.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)
Hans Verkuil - April 10, 2019, 1:48 p.m.
On 4/8/19 2:32 PM, Philipp Zabel wrote:
> v4l2-compliance sets colorimetry on the output queue and then verifies
> that querying colorimetry on the capture queue returns the same
> configuration. For this to work, the encoder must allow setting context
> colorimetry parameters on the output queue.
> 
> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
> ---
>  drivers/media/platform/coda/coda-common.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/media/platform/coda/coda-common.c b/drivers/media/platform/coda/coda-common.c
> index 2966eb1c4d2d..7b89dbae938d 100644
> --- a/drivers/media/platform/coda/coda-common.c
> +++ b/drivers/media/platform/coda/coda-common.c
> @@ -819,6 +819,11 @@ static int coda_s_fmt_vid_out(struct file *file, void *priv,
>  	if (ret)
>  		return ret;
>  
> +	ctx->colorspace = f->fmt.pix.colorspace;
> +	ctx->xfer_func = f->fmt.pix.xfer_func;
> +	ctx->ycbcr_enc = f->fmt.pix.ycbcr_enc;
> +	ctx->quantization = f->fmt.pix.quantization;
> +
>  	if (ctx->inst_type != CODA_INST_DECODER)
>  		return 0;
>  
> @@ -831,11 +836,6 @@ static int coda_s_fmt_vid_out(struct file *file, void *priv,
>  	}
>  	ctx->codec = codec;
>  
> -	ctx->colorspace = f->fmt.pix.colorspace;
> -	ctx->xfer_func = f->fmt.pix.xfer_func;
> -	ctx->ycbcr_enc = f->fmt.pix.ycbcr_enc;
> -	ctx->quantization = f->fmt.pix.quantization;
> -
>  	dst_vq = v4l2_m2m_get_vq(ctx->fh.m2m_ctx, V4L2_BUF_TYPE_VIDEO_CAPTURE);
>  	if (!dst_vq)
>  		return -EINVAL;
> 

Isn't the colorimetry information encoded in the stream's metadata? So should
it be set in an encoder register so it ends up in the right place in the bitstream?

And for decoders it should then be read back from a register.

Just curious how this would work for this codec and if it is even possible.

For now this patch is OK, but I think more work is needed.

Regards,

	Hans
Philipp Zabel - April 10, 2019, 2:23 p.m.
On Wed, 2019-04-10 at 15:48 +0200, Hans Verkuil wrote:
[...]
> Isn't the colorimetry information encoded in the stream's metadata?

That depends on the codec. Colorimetry information can be stored in
optional headers (for example Sequence Display Extension for MPEG-2,
Video Usability Information for H.264) but I don't know that the CODA
firmware can parse or generate any of them.

> So should it be set i an encoder register so it ends up in the right
> place in the bitstream?

I could produce the necessary headers manually and inject them into the
bitstream in the driver.

But for example for JPEG it is not clear to me if there even is a
correct way to do that. Can V4L2 colorimetry settings be translated into
ICC profiles?

> And for decoders it should then be read back from a register.

Same as above, I'd have to parse the headers in the driver.

> Just curious how this would work for this codec and if it is even possible.
> 
> For now this patch is OK, but I think more work is needed.

Agreed.

regards
Philipp

Patch

diff --git a/drivers/media/platform/coda/coda-common.c b/drivers/media/platform/coda/coda-common.c
index 2966eb1c4d2d..7b89dbae938d 100644
--- a/drivers/media/platform/coda/coda-common.c
+++ b/drivers/media/platform/coda/coda-common.c
@@ -819,6 +819,11 @@  static int coda_s_fmt_vid_out(struct file *file, void *priv,
 	if (ret)
 		return ret;
 
+	ctx->colorspace = f->fmt.pix.colorspace;
+	ctx->xfer_func = f->fmt.pix.xfer_func;
+	ctx->ycbcr_enc = f->fmt.pix.ycbcr_enc;
+	ctx->quantization = f->fmt.pix.quantization;
+
 	if (ctx->inst_type != CODA_INST_DECODER)
 		return 0;
 
@@ -831,11 +836,6 @@  static int coda_s_fmt_vid_out(struct file *file, void *priv,
 	}
 	ctx->codec = codec;
 
-	ctx->colorspace = f->fmt.pix.colorspace;
-	ctx->xfer_func = f->fmt.pix.xfer_func;
-	ctx->ycbcr_enc = f->fmt.pix.ycbcr_enc;
-	ctx->quantization = f->fmt.pix.quantization;
-
 	dst_vq = v4l2_m2m_get_vq(ctx->fh.m2m_ctx, V4L2_BUF_TYPE_VIDEO_CAPTURE);
 	if (!dst_vq)
 		return -EINVAL;

Privacy Policy