Re: [PATCH v2 2/3] [media] hva: multi-format video encoder V4L2 driver

[Jean Christophe TROTIN <jean-christophe.trotin@st.com>]

On 07/11/2016 08:00 PM, Nicolas Dufresne wrote:
> Le lundi 11 juillet 2016 à 17:14 +0200, Jean-Christophe Trotin a écrit :

[snip]

>> +static int hva_g_fmt_stream(struct file *file, void *fh, struct v4l2_format *f)
>> +{
>> +	struct hva_ctx *ctx = fh_to_ctx(file->private_data);
>> +	struct device *dev = ctx_to_dev(ctx);
>> +	struct hva_streaminfo *streaminfo = &ctx->streaminfo;
>> +
>> +	f->fmt.pix.width = streaminfo->width;
>> +	f->fmt.pix.height = streaminfo->height;
>> +	f->fmt.pix.field = V4L2_FIELD_NONE;
>> +	f->fmt.pix.colorspace = V4L2_COLORSPACE_SMPTE170M;
>
> Hard coding this is not great.Ideally the colorimetry (if not modified) should be copied from OUTPUT to CAPTURE, you may also set this to V4L2_COLORSPACE_DEFAULT.
>

Nicolas,

Thank you for the remark.
Colorspace was hard-coded because only V4L2_COLORSPACE_SMPTE170M is supported. 
However, I understand that hard-coding is not great: I will align the code in 
version 3 on the colorspace management made in the coda driver.

Best regards,
Jean-Christophe.

>> +	f->fmt.pix.pixelformat = streaminfo->streamformat;
>> +	f->fmt.pix.bytesperline = 0;
>> +	f->fmt.pix.sizeimage = ctx->max_stream_size;
>> +
>> +	dev_dbg(dev, "%s V4L2 G_FMT (CAPTURE): %dx%d fmt:%.4s size:%d\n",
>> +		ctx->name, f->fmt.pix.width, f->fmt.pix.height,
>> +		(u8 *)&f->fmt.pix.pixelformat, f->fmt.pix.sizeimage);
>> +	return 0;
>> +}

[snip]