On 12/03/2014 01:16 AM, Sakari Ailus wrote:
> Hi Hans,
> 
> On Mon, Dec 01, 2014 at 10:03:46AM +0100, Hans Verkuil wrote:
>> From: Hans Verkuil <hans.verk...@cisco.com>
>>
>> Add and copy the new ycbcr_enc and quantization fields.
>>
>> Signed-off-by: Hans Verkuil <hans.verk...@cisco.com>
>> ---
>>  include/media/v4l2-mediabus.h      | 4 ++++
>>  include/uapi/linux/v4l2-mediabus.h | 6 +++++-
>>  2 files changed, 9 insertions(+), 1 deletion(-)
>>
>> diff --git a/include/media/v4l2-mediabus.h b/include/media/v4l2-mediabus.h
>> index 59d7397..38d960d 100644
>> --- a/include/media/v4l2-mediabus.h
>> +++ b/include/media/v4l2-mediabus.h
>> @@ -94,6 +94,8 @@ static inline void v4l2_fill_pix_format(struct 
>> v4l2_pix_format *pix_fmt,
>>      pix_fmt->height = mbus_fmt->height;
>>      pix_fmt->field = mbus_fmt->field;
>>      pix_fmt->colorspace = mbus_fmt->colorspace;
>> +    pix_fmt->ycbcr_enc = mbus_fmt->ycbcr_enc;
>> +    pix_fmt->quantization = mbus_fmt->quantization;
>>  }
>>  
>>  static inline void v4l2_fill_mbus_format(struct v4l2_mbus_framefmt 
>> *mbus_fmt,
>> @@ -104,6 +106,8 @@ static inline void v4l2_fill_mbus_format(struct 
>> v4l2_mbus_framefmt *mbus_fmt,
>>      mbus_fmt->height = pix_fmt->height;
>>      mbus_fmt->field = pix_fmt->field;
>>      mbus_fmt->colorspace = pix_fmt->colorspace;
>> +    mbus_fmt->ycbcr_enc = pix_fmt->ycbcr_enc;
>> +    mbus_fmt->quantization = pix_fmt->quantization;
>>      mbus_fmt->code = code;
>>  }
>>  
>> diff --git a/include/uapi/linux/v4l2-mediabus.h 
>> b/include/uapi/linux/v4l2-mediabus.h
>> index b1934a3..5a86d8e 100644
>> --- a/include/uapi/linux/v4l2-mediabus.h
>> +++ b/include/uapi/linux/v4l2-mediabus.h
>> @@ -22,6 +22,8 @@
>>   * @code:   data format code (from enum v4l2_mbus_pixelcode)
>>   * @field:  used interlacing type (from enum v4l2_field)
>>   * @colorspace:     colorspace of the data (from enum v4l2_colorspace)
>> + * @ycbcr_enc:      YCbCr encoding of the data (from enum 
>> v4l2_ycbcr_encoding)
>> + * @quantization: quantization of the data (from enum v4l2_quantization)
>>   */
>>  struct v4l2_mbus_framefmt {
>>      __u32                   width;
>> @@ -29,7 +31,9 @@ struct v4l2_mbus_framefmt {
>>      __u32                   code;
>>      __u32                   field;
>>      __u32                   colorspace;
>> -    __u32                   reserved[7];
>> +    __u32                   ycbcr_enc;
>> +    __u32                   quantization;
>> +    __u32                   reserved[5];
> 
> If you feel these can fit to 8 bits in planes, I would consider to use 8
> bits here as well. Adding frame descriptor support later on might eat some
> fields from here as well.

You can do this in a number of ways:

        __u8 ycbcr_enc;
        __u8 quantization;
        __u32 reserved[6];

This would leave a hole before the reserved field. That's hard to zero.

        __u8 ycbcr_enc;
        __u8 quantization;
        __u8 reserved[2 + 6 * 4];

This will work for now, but if a __u32 needs to be added later, then I get
a hole again.

        __u16 ycbcr_enc;
        __u16 quantization;
        __u32 reserved[6];

This is the only alternative that doesn't leave a hole. Would this be OK?
I have no problem changing the API to this.

Regards,

        Hans
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to