Re: [PATCH v2] v4l2-ctl: support for metadata output
On 10/14/19 1:44 PM, Hans Verkuil wrote:
> On 10/3/19 12:54 PM, Vandana BN wrote:
>> Adds support to test metadata output format V4L2_META_FMT_VIVID.
>>
>> Signed-off-by: Vandana BN
>> ---
>> contrib/freebsd/include/linux/videodev2.h | 1 +
>> include/linux/videodev2.h | 1 +
>> utils/v4l2-ctl/v4l2-ctl-meta.cpp | 35 +++
>> utils/v4l2-ctl/v4l2-ctl-streaming.cpp | 9 +-
>> utils/v4l2-ctl/v4l2-ctl.h | 1 +
>> 5 files changed, 46 insertions(+), 1 deletion(-)
>>
>> diff --git a/contrib/freebsd/include/linux/videodev2.h
>> b/contrib/freebsd/include/linux/videodev2.h
>> index 0c12d27f..6c0169be 100644
>> --- a/contrib/freebsd/include/linux/videodev2.h
>> +++ b/contrib/freebsd/include/linux/videodev2.h
>> @@ -783,6 +783,7 @@ struct v4l2_pix_format {
>> #define V4L2_META_FMT_VSP1_HGTv4l2_fourcc('V', 'S', 'P', 'T') /* R-Car
>> VSP1 2-D Histogram */
>> #define V4L2_META_FMT_UVC v4l2_fourcc('U', 'V', 'C', 'H') /* UVC
>> Payload Header metadata */
>> #define V4L2_META_FMT_D4XXv4l2_fourcc('D', '4', 'X', 'X') /* D4XX
>> Payload Header metadata */
>> +#define V4L2_META_FMT_VIVID v4l2_fourcc('V', 'I', 'V', 'D') /* Vivid
>> Metadata */
>>
>> /* priv field value to indicates that subsequent fields are valid. */
>> #define V4L2_PIX_FMT_PRIV_MAGIC 0xfeedcafe
>> diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
>> index e2847759..107f96d2 100644
>> --- a/include/linux/videodev2.h
>> +++ b/include/linux/videodev2.h
>> @@ -749,6 +749,7 @@ struct v4l2_pix_format {
>> #define V4L2_META_FMT_VSP1_HGTv4l2_fourcc('V', 'S', 'P', 'T') /* R-Car
>> VSP1 2-D Histogram */
>> #define V4L2_META_FMT_UVC v4l2_fourcc('U', 'V', 'C', 'H') /* UVC
>> Payload Header metadata */
>> #define V4L2_META_FMT_D4XXv4l2_fourcc('D', '4', 'X', 'X') /* D4XX
>> Payload Header metadata */
>> +#define V4L2_META_FMT_VIVID v4l2_fourcc('V', 'I', 'V', 'D') /* Vivid
>> Metadata */
>>
>> /* priv field value to indicates that subsequent fields are valid. */
>> #define V4L2_PIX_FMT_PRIV_MAGIC 0xfeedcafe
>> diff --git a/utils/v4l2-ctl/v4l2-ctl-meta.cpp
>> b/utils/v4l2-ctl/v4l2-ctl-meta.cpp
>> index eae7438f..33e42fcf 100644
>> --- a/utils/v4l2-ctl/v4l2-ctl-meta.cpp
>> +++ b/utils/v4l2-ctl/v4l2-ctl-meta.cpp
>> @@ -139,10 +139,18 @@ struct vivid_uvc_meta_buf {
>> #define UVC_STREAM_SCR (1 << 3)
>> #define UVC_STREAM_PTS (1 << 2)
>>
>> +struct vivid_meta_out_buf {
>> +__u16 brightness;
>> +__u16 contrast;
>> +__u16 saturation;
>> +__s16 hue;
>> +};
>> +
>> void print_meta_buffer(FILE *f, cv4l_buffer &buf, cv4l_fmt &fmt, cv4l_queue
>> &q)
>> {
>> struct vivid_uvc_meta_buf *vbuf;
>> int buf_off = 0;
>> +struct vivid_meta_out_buf *vbuf_out;
>>
>> switch (fmt.g_pixelformat()) {
>> case V4L2_META_FMT_UVC:
>> @@ -164,5 +172,32 @@ void print_meta_buffer(FILE *f, cv4l_buffer &buf,
>> cv4l_fmt &fmt, cv4l_queue &q)
>> le16toh(*(__u16*)(vbuf->buf + buf_off + 4)));
>> fprintf(f, "\n");
>> break;
>> +case V4L2_META_FMT_VIVID:
>> +fprintf(f, "VIVID:");
>> +vbuf_out = (vivid_meta_out_buf *)q.g_dataptr(buf.g_index(), 0);
>> +
>> +fprintf(f, " brightness: %u contrast: %u saturation: %u hue:
>> %d\n",
>> +vbuf_out->brightness, vbuf_out->contrast,
>> +vbuf_out->saturation, vbuf_out->hue);
>> +break;
>> +}
>> +}
>> +
>> +void meta_fillbuffer(cv4l_buffer &buf, cv4l_fmt &fmt, cv4l_queue &q)
>> +{
>> +struct vivid_meta_out_buf *vbuf;
>> +
>> +switch (fmt.g_pixelformat()) {
>> +case V4L2_META_FMT_VIVID:
>> +vbuf = (vivid_meta_out_buf *)q.g_dataptr(buf.g_index(),
>> 0);
>> +vbuf->brightness = buf.g_sequence() <= 255 ?
>> +buf.g_sequence() : buf.g_sequence() % 255;
>
> This is very cumbersome. Why not just do:
>
> vbuf->brightness = buf.g_sequence() % 256;
>
> (the range is 0-255, so that's modulo 256).
>
> Same elsewhere.
>
> Regards,
>
> Hans
>
>> +vbuf->contrast = buf.g_sequence() + 10 <= 255 ?
>> +buf.g_sequence(): (buf.g_sequence() + 10) % 255;
>> +vbuf->saturation = buf.g_sequence() + 20 <= 255 ?
>> +buf.g_sequence(): (buf.g_sequence() + 20) % 255;
>> +vbuf->hue = (__s16) buf.g_sequence() - 128 <= 128 ?
>> +buf.g_sequence() - 128 : buf.g_sequence() % 256
>> - 128;
Another problem with this code is that the image goes completely to black at
some point.
It is better to prevent that from happening by for example keeping the minimum
brightness
and contrast at 64.
Regards,
Hans
>> +
Re: [PATCH v2] v4l2-ctl: support for metadata output
On 10/3/19 12:54 PM, Vandana BN wrote:
> Adds support to test metadata output format V4L2_META_FMT_VIVID.
>
> Signed-off-by: Vandana BN
> ---
> contrib/freebsd/include/linux/videodev2.h | 1 +
> include/linux/videodev2.h | 1 +
> utils/v4l2-ctl/v4l2-ctl-meta.cpp | 35 +++
> utils/v4l2-ctl/v4l2-ctl-streaming.cpp | 9 +-
> utils/v4l2-ctl/v4l2-ctl.h | 1 +
> 5 files changed, 46 insertions(+), 1 deletion(-)
>
> diff --git a/contrib/freebsd/include/linux/videodev2.h
> b/contrib/freebsd/include/linux/videodev2.h
> index 0c12d27f..6c0169be 100644
> --- a/contrib/freebsd/include/linux/videodev2.h
> +++ b/contrib/freebsd/include/linux/videodev2.h
> @@ -783,6 +783,7 @@ struct v4l2_pix_format {
> #define V4L2_META_FMT_VSP1_HGTv4l2_fourcc('V', 'S', 'P', 'T') /* R-Car
> VSP1 2-D Histogram */
> #define V4L2_META_FMT_UVC v4l2_fourcc('U', 'V', 'C', 'H') /* UVC
> Payload Header metadata */
> #define V4L2_META_FMT_D4XXv4l2_fourcc('D', '4', 'X', 'X') /* D4XX
> Payload Header metadata */
> +#define V4L2_META_FMT_VIVID v4l2_fourcc('V', 'I', 'V', 'D') /* Vivid
> Metadata */
>
> /* priv field value to indicates that subsequent fields are valid. */
> #define V4L2_PIX_FMT_PRIV_MAGIC 0xfeedcafe
> diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
> index e2847759..107f96d2 100644
> --- a/include/linux/videodev2.h
> +++ b/include/linux/videodev2.h
> @@ -749,6 +749,7 @@ struct v4l2_pix_format {
> #define V4L2_META_FMT_VSP1_HGTv4l2_fourcc('V', 'S', 'P', 'T') /* R-Car
> VSP1 2-D Histogram */
> #define V4L2_META_FMT_UVC v4l2_fourcc('U', 'V', 'C', 'H') /* UVC
> Payload Header metadata */
> #define V4L2_META_FMT_D4XXv4l2_fourcc('D', '4', 'X', 'X') /* D4XX
> Payload Header metadata */
> +#define V4L2_META_FMT_VIVID v4l2_fourcc('V', 'I', 'V', 'D') /* Vivid
> Metadata */
>
> /* priv field value to indicates that subsequent fields are valid. */
> #define V4L2_PIX_FMT_PRIV_MAGIC 0xfeedcafe
> diff --git a/utils/v4l2-ctl/v4l2-ctl-meta.cpp
> b/utils/v4l2-ctl/v4l2-ctl-meta.cpp
> index eae7438f..33e42fcf 100644
> --- a/utils/v4l2-ctl/v4l2-ctl-meta.cpp
> +++ b/utils/v4l2-ctl/v4l2-ctl-meta.cpp
> @@ -139,10 +139,18 @@ struct vivid_uvc_meta_buf {
> #define UVC_STREAM_SCR (1 << 3)
> #define UVC_STREAM_PTS (1 << 2)
>
> +struct vivid_meta_out_buf {
> +__u16brightness;
> +__u16contrast;
> +__u16saturation;
> +__s16hue;
> +};
> +
> void print_meta_buffer(FILE *f, cv4l_buffer &buf, cv4l_fmt &fmt, cv4l_queue
> &q)
> {
> struct vivid_uvc_meta_buf *vbuf;
> int buf_off = 0;
> + struct vivid_meta_out_buf *vbuf_out;
>
> switch (fmt.g_pixelformat()) {
> case V4L2_META_FMT_UVC:
> @@ -164,5 +172,32 @@ void print_meta_buffer(FILE *f, cv4l_buffer &buf,
> cv4l_fmt &fmt, cv4l_queue &q)
> le16toh(*(__u16*)(vbuf->buf + buf_off + 4)));
> fprintf(f, "\n");
> break;
> + case V4L2_META_FMT_VIVID:
> + fprintf(f, "VIVID:");
> + vbuf_out = (vivid_meta_out_buf *)q.g_dataptr(buf.g_index(), 0);
> +
> + fprintf(f, " brightness: %u contrast: %u saturation: %u hue:
> %d\n",
> + vbuf_out->brightness, vbuf_out->contrast,
> + vbuf_out->saturation, vbuf_out->hue);
> + break;
> + }
> +}
> +
> +void meta_fillbuffer(cv4l_buffer &buf, cv4l_fmt &fmt, cv4l_queue &q)
> +{
> + struct vivid_meta_out_buf *vbuf;
> +
> + switch (fmt.g_pixelformat()) {
> + case V4L2_META_FMT_VIVID:
> + vbuf = (vivid_meta_out_buf *)q.g_dataptr(buf.g_index(),
> 0);
> + vbuf->brightness = buf.g_sequence() <= 255 ?
> + buf.g_sequence() : buf.g_sequence() % 255;
This is very cumbersome. Why not just do:
vbuf->brightness = buf.g_sequence() % 256;
(the range is 0-255, so that's modulo 256).
Same elsewhere.
Regards,
Hans
> + vbuf->contrast = buf.g_sequence() + 10 <= 255 ?
> + buf.g_sequence(): (buf.g_sequence() + 10) % 255;
> + vbuf->saturation = buf.g_sequence() + 20 <= 255 ?
> + buf.g_sequence(): (buf.g_sequence() + 20) % 255;
> + vbuf->hue = (__s16) buf.g_sequence() - 128 <= 128 ?
> + buf.g_sequence() - 128 : buf.g_sequence() % 256
> - 128;
> + break;
> }
> }
> diff --git a/utils/v4l2-ctl/v4l2-ctl-streaming.cpp
> b/utils/v4l2-ctl/v4l2-ctl-streaming.cpp
> index 47b7d3f8..184bfd64 100644
> --- a/utils/v4l2-ctl/v4l2-ctl-streaming.cpp
> +++ b/utils/v4l2-ctl/v4l2-ctl-streaming.cpp
> @@ -1146,6 +1146,7 @@ static int do_setup_out_buffers(cv4l_fd &fd, cv4l_queue
> &q, FILE *fin,
