2020-07-02 10:03:46

by Stanimir Varbanov

[permalink] [raw]
Subject: [PATCH] v4l: Add source change event for colorimetry

This event indicate that the source colorspace is changed
during run-time. The client has to retrieve the new colorspace
identifiers by getting the format (G_FMT).

Signed-off-by: Stanimir Varbanov <[email protected]>
---
.../userspace-api/media/v4l/vidioc-dqevent.rst | 11 ++++++++++-
.../userspace-api/media/videodev2.h.rst.exceptions | 1 +
include/uapi/linux/videodev2.h | 1 +
3 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/Documentation/userspace-api/media/v4l/vidioc-dqevent.rst b/Documentation/userspace-api/media/v4l/vidioc-dqevent.rst
index a9a176d5256d..3f69c753db58 100644
--- a/Documentation/userspace-api/media/v4l/vidioc-dqevent.rst
+++ b/Documentation/userspace-api/media/v4l/vidioc-dqevent.rst
@@ -381,7 +381,16 @@ call.
that many Video Capture devices are not able to recover from a temporary
loss of signal and so restarting streaming I/O is required in order for
the hardware to synchronize to the video signal.
-
+ * - ``V4L2_EVENT_SRC_CH_COLORIMETRY``
+ - 0x0002
+ - This event gets triggered when a colorspace change is detected at
+ an input. By colorspace change here we include also changes in the
+ colorspace specifiers (transfer function, Y'CbCr encoding and
+ quantization). This event can come from an input or from video decoder.
+ Once the event has been send to the client the driver has to update
+ the colorspace specifiers internally so that they could be retrieved by
+ client. In that case queue re-negotiation is not needed as this change
+ only reflects on the interpretation of the data.

Return Value
============
diff --git a/Documentation/userspace-api/media/videodev2.h.rst.exceptions b/Documentation/userspace-api/media/videodev2.h.rst.exceptions
index ca05e4e126b2..54fc21af852d 100644
--- a/Documentation/userspace-api/media/videodev2.h.rst.exceptions
+++ b/Documentation/userspace-api/media/videodev2.h.rst.exceptions
@@ -492,6 +492,7 @@ replace define V4L2_EVENT_CTRL_CH_FLAGS ctrl-changes-flags
replace define V4L2_EVENT_CTRL_CH_RANGE ctrl-changes-flags

replace define V4L2_EVENT_SRC_CH_RESOLUTION src-changes-flags
+replace define V4L2_EVENT_SRC_CH_COLORIMETRY src-changes-flags

replace define V4L2_EVENT_MD_FL_HAVE_FRAME_SEQ :c:type:`v4l2_event_motion_det`

diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index 303805438814..b5838bc4e3a3 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -2351,6 +2351,7 @@ struct v4l2_event_frame_sync {
};

#define V4L2_EVENT_SRC_CH_RESOLUTION (1 << 0)
+#define V4L2_EVENT_SRC_CH_COLORIMETRY (1 << 1)

struct v4l2_event_src_change {
__u32 changes;
--
2.17.1


2020-07-02 11:55:31

by Stanimir Varbanov

[permalink] [raw]
Subject: Re: [PATCH] v4l: Add source change event for colorimetry

Hi,

Once we have this event there is still open question how the client will
know the data buffer on which the new colorspace is valid/applied.

The options could be:
* a new buffer flag and
* some information in the v4l2_event structure.

Thoughts?

On 7/2/20 1:00 PM, Stanimir Varbanov wrote:
> This event indicate that the source colorspace is changed
> during run-time. The client has to retrieve the new colorspace
> identifiers by getting the format (G_FMT).
>
> Signed-off-by: Stanimir Varbanov <[email protected]>
> ---
> .../userspace-api/media/v4l/vidioc-dqevent.rst | 11 ++++++++++-
> .../userspace-api/media/videodev2.h.rst.exceptions | 1 +
> include/uapi/linux/videodev2.h | 1 +
> 3 files changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/userspace-api/media/v4l/vidioc-dqevent.rst b/Documentation/userspace-api/media/v4l/vidioc-dqevent.rst
> index a9a176d5256d..3f69c753db58 100644
> --- a/Documentation/userspace-api/media/v4l/vidioc-dqevent.rst
> +++ b/Documentation/userspace-api/media/v4l/vidioc-dqevent.rst
> @@ -381,7 +381,16 @@ call.
> that many Video Capture devices are not able to recover from a temporary
> loss of signal and so restarting streaming I/O is required in order for
> the hardware to synchronize to the video signal.
> -
> + * - ``V4L2_EVENT_SRC_CH_COLORIMETRY``
> + - 0x0002
> + - This event gets triggered when a colorspace change is detected at
> + an input. By colorspace change here we include also changes in the
> + colorspace specifiers (transfer function, Y'CbCr encoding and
> + quantization). This event can come from an input or from video decoder.
> + Once the event has been send to the client the driver has to update
> + the colorspace specifiers internally so that they could be retrieved by
> + client. In that case queue re-negotiation is not needed as this change
> + only reflects on the interpretation of the data.
>
> Return Value
> ============
> diff --git a/Documentation/userspace-api/media/videodev2.h.rst.exceptions b/Documentation/userspace-api/media/videodev2.h.rst.exceptions
> index ca05e4e126b2..54fc21af852d 100644
> --- a/Documentation/userspace-api/media/videodev2.h.rst.exceptions
> +++ b/Documentation/userspace-api/media/videodev2.h.rst.exceptions
> @@ -492,6 +492,7 @@ replace define V4L2_EVENT_CTRL_CH_FLAGS ctrl-changes-flags
> replace define V4L2_EVENT_CTRL_CH_RANGE ctrl-changes-flags
>
> replace define V4L2_EVENT_SRC_CH_RESOLUTION src-changes-flags
> +replace define V4L2_EVENT_SRC_CH_COLORIMETRY src-changes-flags
>
> replace define V4L2_EVENT_MD_FL_HAVE_FRAME_SEQ :c:type:`v4l2_event_motion_det`
>
> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
> index 303805438814..b5838bc4e3a3 100644
> --- a/include/uapi/linux/videodev2.h
> +++ b/include/uapi/linux/videodev2.h
> @@ -2351,6 +2351,7 @@ struct v4l2_event_frame_sync {
> };
>
> #define V4L2_EVENT_SRC_CH_RESOLUTION (1 << 0)
> +#define V4L2_EVENT_SRC_CH_COLORIMETRY (1 << 1)
>
> struct v4l2_event_src_change {
> __u32 changes;
>

--
regards,
Stan

2020-10-13 16:38:01

by Tomasz Figa

[permalink] [raw]
Subject: Re: [PATCH] v4l: Add source change event for colorimetry

On Tue, Oct 13, 2020 at 11:03 AM Stanimir Varbanov
<[email protected]> wrote:
>
> Hi,
>
> On 7/2/20 2:52 PM, Stanimir Varbanov wrote:
> > Hi,
> >
> > Once we have this event there is still open question how the client will
> > know the data buffer on which the new colorspace is valid/applied.
> >
> > The options could be:
> > * a new buffer flag and
> > * some information in the v4l2_event structure.
> >
> > Thoughts?
>
> Kindly ping.
>

The event itself sounds good to me, but how do we know which buffer is
the first to have the new colorimetry?

Best regards,
Tomasz

> >
> > On 7/2/20 1:00 PM, Stanimir Varbanov wrote:
> >> This event indicate that the source colorspace is changed
> >> during run-time. The client has to retrieve the new colorspace
> >> identifiers by getting the format (G_FMT).
> >>
> >> Signed-off-by: Stanimir Varbanov <[email protected]>
> >> ---
> >> .../userspace-api/media/v4l/vidioc-dqevent.rst | 11 ++++++++++-
> >> .../userspace-api/media/videodev2.h.rst.exceptions | 1 +
> >> include/uapi/linux/videodev2.h | 1 +
> >> 3 files changed, 12 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/Documentation/userspace-api/media/v4l/vidioc-dqevent.rst b/Documentation/userspace-api/media/v4l/vidioc-dqevent.rst
> >> index a9a176d5256d..3f69c753db58 100644
> >> --- a/Documentation/userspace-api/media/v4l/vidioc-dqevent.rst
> >> +++ b/Documentation/userspace-api/media/v4l/vidioc-dqevent.rst
> >> @@ -381,7 +381,16 @@ call.
> >> that many Video Capture devices are not able to recover from a temporary
> >> loss of signal and so restarting streaming I/O is required in order for
> >> the hardware to synchronize to the video signal.
> >> -
> >> + * - ``V4L2_EVENT_SRC_CH_COLORIMETRY``
> >> + - 0x0002
> >> + - This event gets triggered when a colorspace change is detected at
> >> + an input. By colorspace change here we include also changes in the
> >> + colorspace specifiers (transfer function, Y'CbCr encoding and
> >> + quantization). This event can come from an input or from video decoder.
> >> + Once the event has been send to the client the driver has to update
> >> + the colorspace specifiers internally so that they could be retrieved by
> >> + client. In that case queue re-negotiation is not needed as this change
> >> + only reflects on the interpretation of the data.
> >>
> >> Return Value
> >> ============
> >> diff --git a/Documentation/userspace-api/media/videodev2.h.rst.exceptions b/Documentation/userspace-api/media/videodev2.h.rst.exceptions
> >> index ca05e4e126b2..54fc21af852d 100644
> >> --- a/Documentation/userspace-api/media/videodev2.h.rst.exceptions
> >> +++ b/Documentation/userspace-api/media/videodev2.h.rst.exceptions
> >> @@ -492,6 +492,7 @@ replace define V4L2_EVENT_CTRL_CH_FLAGS ctrl-changes-flags
> >> replace define V4L2_EVENT_CTRL_CH_RANGE ctrl-changes-flags
> >>
> >> replace define V4L2_EVENT_SRC_CH_RESOLUTION src-changes-flags
> >> +replace define V4L2_EVENT_SRC_CH_COLORIMETRY src-changes-flags
> >>
> >> replace define V4L2_EVENT_MD_FL_HAVE_FRAME_SEQ :c:type:`v4l2_event_motion_det`
> >>
> >> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
> >> index 303805438814..b5838bc4e3a3 100644
> >> --- a/include/uapi/linux/videodev2.h
> >> +++ b/include/uapi/linux/videodev2.h
> >> @@ -2351,6 +2351,7 @@ struct v4l2_event_frame_sync {
> >> };
> >>
> >> #define V4L2_EVENT_SRC_CH_RESOLUTION (1 << 0)
> >> +#define V4L2_EVENT_SRC_CH_COLORIMETRY (1 << 1)
> >>
> >> struct v4l2_event_src_change {
> >> __u32 changes;
> >>
> >
>
> --
> regards,
> Stan

2020-10-13 16:42:59

by Stanimir Varbanov

[permalink] [raw]
Subject: Re: [PATCH] v4l: Add source change event for colorimetry



On 10/13/20 4:40 PM, Tomasz Figa wrote:
> On Tue, Oct 13, 2020 at 11:03 AM Stanimir Varbanov
> <[email protected]> wrote:
>>
>> Hi,
>>
>> On 7/2/20 2:52 PM, Stanimir Varbanov wrote:
>>> Hi,
>>>
>>> Once we have this event there is still open question how the client will
>>> know the data buffer on which the new colorspace is valid/applied.
>>>
>>> The options could be:
>>> * a new buffer flag and
>>> * some information in the v4l2_event structure.
>>>
>>> Thoughts?
>>
>> Kindly ping.
>>
>
> The event itself sounds good to me, but how do we know which buffer is
> the first to have the new colorimetry?

I think Hans have a very good idea to have width/height and colorspace
specifiers in v4l2_ext_buffer [1].

[1] https://lkml.org/lkml/2020/9/9/531

>
> Best regards,
> Tomasz
>
>>>
>>> On 7/2/20 1:00 PM, Stanimir Varbanov wrote:
>>>> This event indicate that the source colorspace is changed
>>>> during run-time. The client has to retrieve the new colorspace
>>>> identifiers by getting the format (G_FMT).
>>>>
>>>> Signed-off-by: Stanimir Varbanov <[email protected]>
>>>> ---
>>>> .../userspace-api/media/v4l/vidioc-dqevent.rst | 11 ++++++++++-
>>>> .../userspace-api/media/videodev2.h.rst.exceptions | 1 +
>>>> include/uapi/linux/videodev2.h | 1 +
>>>> 3 files changed, 12 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/Documentation/userspace-api/media/v4l/vidioc-dqevent.rst b/Documentation/userspace-api/media/v4l/vidioc-dqevent.rst
>>>> index a9a176d5256d..3f69c753db58 100644
>>>> --- a/Documentation/userspace-api/media/v4l/vidioc-dqevent.rst
>>>> +++ b/Documentation/userspace-api/media/v4l/vidioc-dqevent.rst
>>>> @@ -381,7 +381,16 @@ call.
>>>> that many Video Capture devices are not able to recover from a temporary
>>>> loss of signal and so restarting streaming I/O is required in order for
>>>> the hardware to synchronize to the video signal.
>>>> -
>>>> + * - ``V4L2_EVENT_SRC_CH_COLORIMETRY``
>>>> + - 0x0002
>>>> + - This event gets triggered when a colorspace change is detected at
>>>> + an input. By colorspace change here we include also changes in the
>>>> + colorspace specifiers (transfer function, Y'CbCr encoding and
>>>> + quantization). This event can come from an input or from video decoder.
>>>> + Once the event has been send to the client the driver has to update
>>>> + the colorspace specifiers internally so that they could be retrieved by
>>>> + client. In that case queue re-negotiation is not needed as this change
>>>> + only reflects on the interpretation of the data.
>>>>
>>>> Return Value
>>>> ============
>>>> diff --git a/Documentation/userspace-api/media/videodev2.h.rst.exceptions b/Documentation/userspace-api/media/videodev2.h.rst.exceptions
>>>> index ca05e4e126b2..54fc21af852d 100644
>>>> --- a/Documentation/userspace-api/media/videodev2.h.rst.exceptions
>>>> +++ b/Documentation/userspace-api/media/videodev2.h.rst.exceptions
>>>> @@ -492,6 +492,7 @@ replace define V4L2_EVENT_CTRL_CH_FLAGS ctrl-changes-flags
>>>> replace define V4L2_EVENT_CTRL_CH_RANGE ctrl-changes-flags
>>>>
>>>> replace define V4L2_EVENT_SRC_CH_RESOLUTION src-changes-flags
>>>> +replace define V4L2_EVENT_SRC_CH_COLORIMETRY src-changes-flags
>>>>
>>>> replace define V4L2_EVENT_MD_FL_HAVE_FRAME_SEQ :c:type:`v4l2_event_motion_det`
>>>>
>>>> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
>>>> index 303805438814..b5838bc4e3a3 100644
>>>> --- a/include/uapi/linux/videodev2.h
>>>> +++ b/include/uapi/linux/videodev2.h
>>>> @@ -2351,6 +2351,7 @@ struct v4l2_event_frame_sync {
>>>> };
>>>>
>>>> #define V4L2_EVENT_SRC_CH_RESOLUTION (1 << 0)
>>>> +#define V4L2_EVENT_SRC_CH_COLORIMETRY (1 << 1)
>>>>
>>>> struct v4l2_event_src_change {
>>>> __u32 changes;
>>>>
>>>
>>
>> --
>> regards,
>> Stan

--
regards,
Stan

2020-10-13 17:21:47

by Stanimir Varbanov

[permalink] [raw]
Subject: Re: [PATCH] v4l: Add source change event for colorimetry



On 10/13/20 5:07 PM, Tomasz Figa wrote:
> On Tue, Oct 13, 2020 at 3:53 PM Stanimir Varbanov
> <[email protected]> wrote:
>>
>>
>>
>> On 10/13/20 4:40 PM, Tomasz Figa wrote:
>>> On Tue, Oct 13, 2020 at 11:03 AM Stanimir Varbanov
>>> <[email protected]> wrote:
>>>>
>>>> Hi,
>>>>
>>>> On 7/2/20 2:52 PM, Stanimir Varbanov wrote:
>>>>> Hi,
>>>>>
>>>>> Once we have this event there is still open question how the client will
>>>>> know the data buffer on which the new colorspace is valid/applied.
>>>>>
>>>>> The options could be:
>>>>> * a new buffer flag and
>>>>> * some information in the v4l2_event structure.
>>>>>
>>>>> Thoughts?
>>>>
>>>> Kindly ping.
>>>>
>>>
>>> The event itself sounds good to me, but how do we know which buffer is
>>> the first to have the new colorimetry?
>>
>> I think Hans have a very good idea to have width/height and colorspace
>> specifiers in v4l2_ext_buffer [1].
>>
>> [1] https://lkml.org/lkml/2020/9/9/531
>>
>
> Hmm, I think that would basically make the event obsolete and without
> solving that problem I suspect the event is not very useful, because
> one could already receive and display (incorrectly) some buffers
> before realizing that they have different colorimetry

Yes, I agree. I wasn't sure does Hans's idea will be well received, thus
I pinged.

>
> I believe for now we would have to handle this like a resolution
> change - flush the CAPTURE queue and the next buffer after the flush
> would have the new colorimetry. With the new API we could optimize the

I'm not sure what you mean by flush capture queue? Dequeue until LAST
flag (EPIPE) and stop streaming g_fmt(capture queue) and decide what is
changed and start streaming ?

> decoding by getting rid of the flushes and relying on the in-bound
> information.
>
> Best regards,
> Tomasz
>
>>>
>>> Best regards,
>>> Tomasz
>>>
>>>>>
>>>>> On 7/2/20 1:00 PM, Stanimir Varbanov wrote:
>>>>>> This event indicate that the source colorspace is changed
>>>>>> during run-time. The client has to retrieve the new colorspace
>>>>>> identifiers by getting the format (G_FMT).
>>>>>>
>>>>>> Signed-off-by: Stanimir Varbanov <[email protected]>
>>>>>> ---
>>>>>> .../userspace-api/media/v4l/vidioc-dqevent.rst | 11 ++++++++++-
>>>>>> .../userspace-api/media/videodev2.h.rst.exceptions | 1 +
>>>>>> include/uapi/linux/videodev2.h | 1 +
>>>>>> 3 files changed, 12 insertions(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/Documentation/userspace-api/media/v4l/vidioc-dqevent.rst b/Documentation/userspace-api/media/v4l/vidioc-dqevent.rst
>>>>>> index a9a176d5256d..3f69c753db58 100644
>>>>>> --- a/Documentation/userspace-api/media/v4l/vidioc-dqevent.rst
>>>>>> +++ b/Documentation/userspace-api/media/v4l/vidioc-dqevent.rst
>>>>>> @@ -381,7 +381,16 @@ call.
>>>>>> that many Video Capture devices are not able to recover from a temporary
>>>>>> loss of signal and so restarting streaming I/O is required in order for
>>>>>> the hardware to synchronize to the video signal.
>>>>>> -
>>>>>> + * - ``V4L2_EVENT_SRC_CH_COLORIMETRY``
>>>>>> + - 0x0002
>>>>>> + - This event gets triggered when a colorspace change is detected at
>>>>>> + an input. By colorspace change here we include also changes in the
>>>>>> + colorspace specifiers (transfer function, Y'CbCr encoding and
>>>>>> + quantization). This event can come from an input or from video decoder.
>>>>>> + Once the event has been send to the client the driver has to update
>>>>>> + the colorspace specifiers internally so that they could be retrieved by
>>>>>> + client. In that case queue re-negotiation is not needed as this change
>>>>>> + only reflects on the interpretation of the data.
>>>>>>
>>>>>> Return Value
>>>>>> ============
>>>>>> diff --git a/Documentation/userspace-api/media/videodev2.h.rst.exceptions b/Documentation/userspace-api/media/videodev2.h.rst.exceptions
>>>>>> index ca05e4e126b2..54fc21af852d 100644
>>>>>> --- a/Documentation/userspace-api/media/videodev2.h.rst.exceptions
>>>>>> +++ b/Documentation/userspace-api/media/videodev2.h.rst.exceptions
>>>>>> @@ -492,6 +492,7 @@ replace define V4L2_EVENT_CTRL_CH_FLAGS ctrl-changes-flags
>>>>>> replace define V4L2_EVENT_CTRL_CH_RANGE ctrl-changes-flags
>>>>>>
>>>>>> replace define V4L2_EVENT_SRC_CH_RESOLUTION src-changes-flags
>>>>>> +replace define V4L2_EVENT_SRC_CH_COLORIMETRY src-changes-flags
>>>>>>
>>>>>> replace define V4L2_EVENT_MD_FL_HAVE_FRAME_SEQ :c:type:`v4l2_event_motion_det`
>>>>>>
>>>>>> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
>>>>>> index 303805438814..b5838bc4e3a3 100644
>>>>>> --- a/include/uapi/linux/videodev2.h
>>>>>> +++ b/include/uapi/linux/videodev2.h
>>>>>> @@ -2351,6 +2351,7 @@ struct v4l2_event_frame_sync {
>>>>>> };
>>>>>>
>>>>>> #define V4L2_EVENT_SRC_CH_RESOLUTION (1 << 0)
>>>>>> +#define V4L2_EVENT_SRC_CH_COLORIMETRY (1 << 1)
>>>>>>
>>>>>> struct v4l2_event_src_change {
>>>>>> __u32 changes;
>>>>>>
>>>>>
>>>>
>>>> --
>>>> regards,
>>>> Stan
>>
>> --
>> regards,
>> Stan

--
regards,
Stan

2020-10-13 17:23:55

by Tomasz Figa

[permalink] [raw]
Subject: Re: [PATCH] v4l: Add source change event for colorimetry

On Tue, Oct 13, 2020 at 4:53 PM Stanimir Varbanov
<[email protected]> wrote:
>
>
>
> On 10/13/20 5:07 PM, Tomasz Figa wrote:
> > On Tue, Oct 13, 2020 at 3:53 PM Stanimir Varbanov
> > <[email protected]> wrote:
> >>
> >>
> >>
> >> On 10/13/20 4:40 PM, Tomasz Figa wrote:
> >>> On Tue, Oct 13, 2020 at 11:03 AM Stanimir Varbanov
> >>> <[email protected]> wrote:
> >>>>
> >>>> Hi,
> >>>>
> >>>> On 7/2/20 2:52 PM, Stanimir Varbanov wrote:
> >>>>> Hi,
> >>>>>
> >>>>> Once we have this event there is still open question how the client will
> >>>>> know the data buffer on which the new colorspace is valid/applied.
> >>>>>
> >>>>> The options could be:
> >>>>> * a new buffer flag and
> >>>>> * some information in the v4l2_event structure.
> >>>>>
> >>>>> Thoughts?
> >>>>
> >>>> Kindly ping.
> >>>>
> >>>
> >>> The event itself sounds good to me, but how do we know which buffer is
> >>> the first to have the new colorimetry?
> >>
> >> I think Hans have a very good idea to have width/height and colorspace
> >> specifiers in v4l2_ext_buffer [1].
> >>
> >> [1] https://lkml.org/lkml/2020/9/9/531
> >>
> >
> > Hmm, I think that would basically make the event obsolete and without
> > solving that problem I suspect the event is not very useful, because
> > one could already receive and display (incorrectly) some buffers
> > before realizing that they have different colorimetry
>
> Yes, I agree. I wasn't sure does Hans's idea will be well received, thus
> I pinged.
>
> >
> > I believe for now we would have to handle this like a resolution
> > change - flush the CAPTURE queue and the next buffer after the flush
> > would have the new colorimetry. With the new API we could optimize the
>
> I'm not sure what you mean by flush capture queue? Dequeue until LAST
> flag (EPIPE) and stop streaming g_fmt(capture queue) and decide what is
> changed and start streaming ?

Yes, although no strict need to stop streaming, other ways are defined
as well, e.g. DEC_CMD_START.

Of course we would need to make appropriate changes to the spec and so
I'd just unify it with the resolution change sequence. I think we
could rename it to "Stream parameter change".

>
> > decoding by getting rid of the flushes and relying on the in-bound
> > information.
> >
> > Best regards,
> > Tomasz
> >
> >>>
> >>> Best regards,
> >>> Tomasz
> >>>
> >>>>>
> >>>>> On 7/2/20 1:00 PM, Stanimir Varbanov wrote:
> >>>>>> This event indicate that the source colorspace is changed
> >>>>>> during run-time. The client has to retrieve the new colorspace
> >>>>>> identifiers by getting the format (G_FMT).
> >>>>>>
> >>>>>> Signed-off-by: Stanimir Varbanov <[email protected]>
> >>>>>> ---
> >>>>>> .../userspace-api/media/v4l/vidioc-dqevent.rst | 11 ++++++++++-
> >>>>>> .../userspace-api/media/videodev2.h.rst.exceptions | 1 +
> >>>>>> include/uapi/linux/videodev2.h | 1 +
> >>>>>> 3 files changed, 12 insertions(+), 1 deletion(-)
> >>>>>>
> >>>>>> diff --git a/Documentation/userspace-api/media/v4l/vidioc-dqevent.rst b/Documentation/userspace-api/media/v4l/vidioc-dqevent.rst
> >>>>>> index a9a176d5256d..3f69c753db58 100644
> >>>>>> --- a/Documentation/userspace-api/media/v4l/vidioc-dqevent.rst
> >>>>>> +++ b/Documentation/userspace-api/media/v4l/vidioc-dqevent.rst
> >>>>>> @@ -381,7 +381,16 @@ call.
> >>>>>> that many Video Capture devices are not able to recover from a temporary
> >>>>>> loss of signal and so restarting streaming I/O is required in order for
> >>>>>> the hardware to synchronize to the video signal.
> >>>>>> -
> >>>>>> + * - ``V4L2_EVENT_SRC_CH_COLORIMETRY``
> >>>>>> + - 0x0002
> >>>>>> + - This event gets triggered when a colorspace change is detected at
> >>>>>> + an input. By colorspace change here we include also changes in the
> >>>>>> + colorspace specifiers (transfer function, Y'CbCr encoding and
> >>>>>> + quantization). This event can come from an input or from video decoder.
> >>>>>> + Once the event has been send to the client the driver has to update
> >>>>>> + the colorspace specifiers internally so that they could be retrieved by
> >>>>>> + client. In that case queue re-negotiation is not needed as this change
> >>>>>> + only reflects on the interpretation of the data.
> >>>>>>
> >>>>>> Return Value
> >>>>>> ============
> >>>>>> diff --git a/Documentation/userspace-api/media/videodev2.h.rst.exceptions b/Documentation/userspace-api/media/videodev2.h.rst.exceptions
> >>>>>> index ca05e4e126b2..54fc21af852d 100644
> >>>>>> --- a/Documentation/userspace-api/media/videodev2.h.rst.exceptions
> >>>>>> +++ b/Documentation/userspace-api/media/videodev2.h.rst.exceptions
> >>>>>> @@ -492,6 +492,7 @@ replace define V4L2_EVENT_CTRL_CH_FLAGS ctrl-changes-flags
> >>>>>> replace define V4L2_EVENT_CTRL_CH_RANGE ctrl-changes-flags
> >>>>>>
> >>>>>> replace define V4L2_EVENT_SRC_CH_RESOLUTION src-changes-flags
> >>>>>> +replace define V4L2_EVENT_SRC_CH_COLORIMETRY src-changes-flags
> >>>>>>
> >>>>>> replace define V4L2_EVENT_MD_FL_HAVE_FRAME_SEQ :c:type:`v4l2_event_motion_det`
> >>>>>>
> >>>>>> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
> >>>>>> index 303805438814..b5838bc4e3a3 100644
> >>>>>> --- a/include/uapi/linux/videodev2.h
> >>>>>> +++ b/include/uapi/linux/videodev2.h
> >>>>>> @@ -2351,6 +2351,7 @@ struct v4l2_event_frame_sync {
> >>>>>> };
> >>>>>>
> >>>>>> #define V4L2_EVENT_SRC_CH_RESOLUTION (1 << 0)
> >>>>>> +#define V4L2_EVENT_SRC_CH_COLORIMETRY (1 << 1)
> >>>>>>
> >>>>>> struct v4l2_event_src_change {
> >>>>>> __u32 changes;
> >>>>>>
> >>>>>
> >>>>
> >>>> --
> >>>> regards,
> >>>> Stan
> >>
> >> --
> >> regards,
> >> Stan
>
> --
> regards,
> Stan

2020-10-13 18:25:14

by Stanimir Varbanov

[permalink] [raw]
Subject: Re: [PATCH] v4l: Add source change event for colorimetry

Hi,

On 7/2/20 2:52 PM, Stanimir Varbanov wrote:
> Hi,
>
> Once we have this event there is still open question how the client will
> know the data buffer on which the new colorspace is valid/applied.
>
> The options could be:
> * a new buffer flag and
> * some information in the v4l2_event structure.
>
> Thoughts?

Kindly ping.

>
> On 7/2/20 1:00 PM, Stanimir Varbanov wrote:
>> This event indicate that the source colorspace is changed
>> during run-time. The client has to retrieve the new colorspace
>> identifiers by getting the format (G_FMT).
>>
>> Signed-off-by: Stanimir Varbanov <[email protected]>
>> ---
>> .../userspace-api/media/v4l/vidioc-dqevent.rst | 11 ++++++++++-
>> .../userspace-api/media/videodev2.h.rst.exceptions | 1 +
>> include/uapi/linux/videodev2.h | 1 +
>> 3 files changed, 12 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/userspace-api/media/v4l/vidioc-dqevent.rst b/Documentation/userspace-api/media/v4l/vidioc-dqevent.rst
>> index a9a176d5256d..3f69c753db58 100644
>> --- a/Documentation/userspace-api/media/v4l/vidioc-dqevent.rst
>> +++ b/Documentation/userspace-api/media/v4l/vidioc-dqevent.rst
>> @@ -381,7 +381,16 @@ call.
>> that many Video Capture devices are not able to recover from a temporary
>> loss of signal and so restarting streaming I/O is required in order for
>> the hardware to synchronize to the video signal.
>> -
>> + * - ``V4L2_EVENT_SRC_CH_COLORIMETRY``
>> + - 0x0002
>> + - This event gets triggered when a colorspace change is detected at
>> + an input. By colorspace change here we include also changes in the
>> + colorspace specifiers (transfer function, Y'CbCr encoding and
>> + quantization). This event can come from an input or from video decoder.
>> + Once the event has been send to the client the driver has to update
>> + the colorspace specifiers internally so that they could be retrieved by
>> + client. In that case queue re-negotiation is not needed as this change
>> + only reflects on the interpretation of the data.
>>
>> Return Value
>> ============
>> diff --git a/Documentation/userspace-api/media/videodev2.h.rst.exceptions b/Documentation/userspace-api/media/videodev2.h.rst.exceptions
>> index ca05e4e126b2..54fc21af852d 100644
>> --- a/Documentation/userspace-api/media/videodev2.h.rst.exceptions
>> +++ b/Documentation/userspace-api/media/videodev2.h.rst.exceptions
>> @@ -492,6 +492,7 @@ replace define V4L2_EVENT_CTRL_CH_FLAGS ctrl-changes-flags
>> replace define V4L2_EVENT_CTRL_CH_RANGE ctrl-changes-flags
>>
>> replace define V4L2_EVENT_SRC_CH_RESOLUTION src-changes-flags
>> +replace define V4L2_EVENT_SRC_CH_COLORIMETRY src-changes-flags
>>
>> replace define V4L2_EVENT_MD_FL_HAVE_FRAME_SEQ :c:type:`v4l2_event_motion_det`
>>
>> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
>> index 303805438814..b5838bc4e3a3 100644
>> --- a/include/uapi/linux/videodev2.h
>> +++ b/include/uapi/linux/videodev2.h
>> @@ -2351,6 +2351,7 @@ struct v4l2_event_frame_sync {
>> };
>>
>> #define V4L2_EVENT_SRC_CH_RESOLUTION (1 << 0)
>> +#define V4L2_EVENT_SRC_CH_COLORIMETRY (1 << 1)
>>
>> struct v4l2_event_src_change {
>> __u32 changes;
>>
>

--
regards,
Stan

2020-10-13 23:51:55

by Tomasz Figa

[permalink] [raw]
Subject: Re: [PATCH] v4l: Add source change event for colorimetry

On Tue, Oct 13, 2020 at 3:53 PM Stanimir Varbanov
<[email protected]> wrote:
>
>
>
> On 10/13/20 4:40 PM, Tomasz Figa wrote:
> > On Tue, Oct 13, 2020 at 11:03 AM Stanimir Varbanov
> > <[email protected]> wrote:
> >>
> >> Hi,
> >>
> >> On 7/2/20 2:52 PM, Stanimir Varbanov wrote:
> >>> Hi,
> >>>
> >>> Once we have this event there is still open question how the client will
> >>> know the data buffer on which the new colorspace is valid/applied.
> >>>
> >>> The options could be:
> >>> * a new buffer flag and
> >>> * some information in the v4l2_event structure.
> >>>
> >>> Thoughts?
> >>
> >> Kindly ping.
> >>
> >
> > The event itself sounds good to me, but how do we know which buffer is
> > the first to have the new colorimetry?
>
> I think Hans have a very good idea to have width/height and colorspace
> specifiers in v4l2_ext_buffer [1].
>
> [1] https://lkml.org/lkml/2020/9/9/531
>

Hmm, I think that would basically make the event obsolete and without
solving that problem I suspect the event is not very useful, because
one could already receive and display (incorrectly) some buffers
before realizing that they have different colorimetry.

I believe for now we would have to handle this like a resolution
change - flush the CAPTURE queue and the next buffer after the flush
would have the new colorimetry. With the new API we could optimize the
decoding by getting rid of the flushes and relying on the in-bound
information.

Best regards,
Tomasz

> >
> > Best regards,
> > Tomasz
> >
> >>>
> >>> On 7/2/20 1:00 PM, Stanimir Varbanov wrote:
> >>>> This event indicate that the source colorspace is changed
> >>>> during run-time. The client has to retrieve the new colorspace
> >>>> identifiers by getting the format (G_FMT).
> >>>>
> >>>> Signed-off-by: Stanimir Varbanov <[email protected]>
> >>>> ---
> >>>> .../userspace-api/media/v4l/vidioc-dqevent.rst | 11 ++++++++++-
> >>>> .../userspace-api/media/videodev2.h.rst.exceptions | 1 +
> >>>> include/uapi/linux/videodev2.h | 1 +
> >>>> 3 files changed, 12 insertions(+), 1 deletion(-)
> >>>>
> >>>> diff --git a/Documentation/userspace-api/media/v4l/vidioc-dqevent.rst b/Documentation/userspace-api/media/v4l/vidioc-dqevent.rst
> >>>> index a9a176d5256d..3f69c753db58 100644
> >>>> --- a/Documentation/userspace-api/media/v4l/vidioc-dqevent.rst
> >>>> +++ b/Documentation/userspace-api/media/v4l/vidioc-dqevent.rst
> >>>> @@ -381,7 +381,16 @@ call.
> >>>> that many Video Capture devices are not able to recover from a temporary
> >>>> loss of signal and so restarting streaming I/O is required in order for
> >>>> the hardware to synchronize to the video signal.
> >>>> -
> >>>> + * - ``V4L2_EVENT_SRC_CH_COLORIMETRY``
> >>>> + - 0x0002
> >>>> + - This event gets triggered when a colorspace change is detected at
> >>>> + an input. By colorspace change here we include also changes in the
> >>>> + colorspace specifiers (transfer function, Y'CbCr encoding and
> >>>> + quantization). This event can come from an input or from video decoder.
> >>>> + Once the event has been send to the client the driver has to update
> >>>> + the colorspace specifiers internally so that they could be retrieved by
> >>>> + client. In that case queue re-negotiation is not needed as this change
> >>>> + only reflects on the interpretation of the data.
> >>>>
> >>>> Return Value
> >>>> ============
> >>>> diff --git a/Documentation/userspace-api/media/videodev2.h.rst.exceptions b/Documentation/userspace-api/media/videodev2.h.rst.exceptions
> >>>> index ca05e4e126b2..54fc21af852d 100644
> >>>> --- a/Documentation/userspace-api/media/videodev2.h.rst.exceptions
> >>>> +++ b/Documentation/userspace-api/media/videodev2.h.rst.exceptions
> >>>> @@ -492,6 +492,7 @@ replace define V4L2_EVENT_CTRL_CH_FLAGS ctrl-changes-flags
> >>>> replace define V4L2_EVENT_CTRL_CH_RANGE ctrl-changes-flags
> >>>>
> >>>> replace define V4L2_EVENT_SRC_CH_RESOLUTION src-changes-flags
> >>>> +replace define V4L2_EVENT_SRC_CH_COLORIMETRY src-changes-flags
> >>>>
> >>>> replace define V4L2_EVENT_MD_FL_HAVE_FRAME_SEQ :c:type:`v4l2_event_motion_det`
> >>>>
> >>>> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
> >>>> index 303805438814..b5838bc4e3a3 100644
> >>>> --- a/include/uapi/linux/videodev2.h
> >>>> +++ b/include/uapi/linux/videodev2.h
> >>>> @@ -2351,6 +2351,7 @@ struct v4l2_event_frame_sync {
> >>>> };
> >>>>
> >>>> #define V4L2_EVENT_SRC_CH_RESOLUTION (1 << 0)
> >>>> +#define V4L2_EVENT_SRC_CH_COLORIMETRY (1 << 1)
> >>>>
> >>>> struct v4l2_event_src_change {
> >>>> __u32 changes;
> >>>>
> >>>
> >>
> >> --
> >> regards,
> >> Stan
>
> --
> regards,
> Stan

2020-11-03 10:32:03

by Hans Verkuil

[permalink] [raw]
Subject: Re: [PATCH] v4l: Add source change event for colorimetry

On 13/10/2020 16:56, Tomasz Figa wrote:
> On Tue, Oct 13, 2020 at 4:53 PM Stanimir Varbanov
> <[email protected]> wrote:
>>
>>
>>
>> On 10/13/20 5:07 PM, Tomasz Figa wrote:
>>> On Tue, Oct 13, 2020 at 3:53 PM Stanimir Varbanov
>>> <[email protected]> wrote:
>>>>
>>>>
>>>>
>>>> On 10/13/20 4:40 PM, Tomasz Figa wrote:
>>>>> On Tue, Oct 13, 2020 at 11:03 AM Stanimir Varbanov
>>>>> <[email protected]> wrote:
>>>>>>
>>>>>> Hi,
>>>>>>
>>>>>> On 7/2/20 2:52 PM, Stanimir Varbanov wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>> Once we have this event there is still open question how the client will
>>>>>>> know the data buffer on which the new colorspace is valid/applied.
>>>>>>>
>>>>>>> The options could be:
>>>>>>> * a new buffer flag and
>>>>>>> * some information in the v4l2_event structure.
>>>>>>>
>>>>>>> Thoughts?
>>>>>>
>>>>>> Kindly ping.
>>>>>>
>>>>>
>>>>> The event itself sounds good to me, but how do we know which buffer is
>>>>> the first to have the new colorimetry?
>>>>
>>>> I think Hans have a very good idea to have width/height and colorspace
>>>> specifiers in v4l2_ext_buffer [1].
>>>>
>>>> [1] https://lkml.org/lkml/2020/9/9/531
>>>>
>>>
>>> Hmm, I think that would basically make the event obsolete and without
>>> solving that problem I suspect the event is not very useful, because
>>> one could already receive and display (incorrectly) some buffers
>>> before realizing that they have different colorimetry
>>
>> Yes, I agree. I wasn't sure does Hans's idea will be well received, thus
>> I pinged.

I think we should wait with this patch. If this can be added to the
proposed extended buffer API, then that will solve this issue in a
clean way.

For the time being I'll mark this patch as RFC in patchwork.

Regards,

Hans

>>
>>>
>>> I believe for now we would have to handle this like a resolution
>>> change - flush the CAPTURE queue and the next buffer after the flush
>>> would have the new colorimetry. With the new API we could optimize the
>>
>> I'm not sure what you mean by flush capture queue? Dequeue until LAST
>> flag (EPIPE) and stop streaming g_fmt(capture queue) and decide what is
>> changed and start streaming ?
>
> Yes, although no strict need to stop streaming, other ways are defined
> as well, e.g. DEC_CMD_START.
>
> Of course we would need to make appropriate changes to the spec and so
> I'd just unify it with the resolution change sequence. I think we
> could rename it to "Stream parameter change".
>
>>
>>> decoding by getting rid of the flushes and relying on the in-bound
>>> information.
>>>
>>> Best regards,
>>> Tomasz
>>>
>>>>>
>>>>> Best regards,
>>>>> Tomasz
>>>>>
>>>>>>>
>>>>>>> On 7/2/20 1:00 PM, Stanimir Varbanov wrote:
>>>>>>>> This event indicate that the source colorspace is changed
>>>>>>>> during run-time. The client has to retrieve the new colorspace
>>>>>>>> identifiers by getting the format (G_FMT).
>>>>>>>>
>>>>>>>> Signed-off-by: Stanimir Varbanov <[email protected]>
>>>>>>>> ---
>>>>>>>> .../userspace-api/media/v4l/vidioc-dqevent.rst | 11 ++++++++++-
>>>>>>>> .../userspace-api/media/videodev2.h.rst.exceptions | 1 +
>>>>>>>> include/uapi/linux/videodev2.h | 1 +
>>>>>>>> 3 files changed, 12 insertions(+), 1 deletion(-)
>>>>>>>>
>>>>>>>> diff --git a/Documentation/userspace-api/media/v4l/vidioc-dqevent.rst b/Documentation/userspace-api/media/v4l/vidioc-dqevent.rst
>>>>>>>> index a9a176d5256d..3f69c753db58 100644
>>>>>>>> --- a/Documentation/userspace-api/media/v4l/vidioc-dqevent.rst
>>>>>>>> +++ b/Documentation/userspace-api/media/v4l/vidioc-dqevent.rst
>>>>>>>> @@ -381,7 +381,16 @@ call.
>>>>>>>> that many Video Capture devices are not able to recover from a temporary
>>>>>>>> loss of signal and so restarting streaming I/O is required in order for
>>>>>>>> the hardware to synchronize to the video signal.
>>>>>>>> -
>>>>>>>> + * - ``V4L2_EVENT_SRC_CH_COLORIMETRY``
>>>>>>>> + - 0x0002
>>>>>>>> + - This event gets triggered when a colorspace change is detected at
>>>>>>>> + an input. By colorspace change here we include also changes in the
>>>>>>>> + colorspace specifiers (transfer function, Y'CbCr encoding and
>>>>>>>> + quantization). This event can come from an input or from video decoder.
>>>>>>>> + Once the event has been send to the client the driver has to update
>>>>>>>> + the colorspace specifiers internally so that they could be retrieved by
>>>>>>>> + client. In that case queue re-negotiation is not needed as this change
>>>>>>>> + only reflects on the interpretation of the data.
>>>>>>>>
>>>>>>>> Return Value
>>>>>>>> ============
>>>>>>>> diff --git a/Documentation/userspace-api/media/videodev2.h.rst.exceptions b/Documentation/userspace-api/media/videodev2.h.rst.exceptions
>>>>>>>> index ca05e4e126b2..54fc21af852d 100644
>>>>>>>> --- a/Documentation/userspace-api/media/videodev2.h.rst.exceptions
>>>>>>>> +++ b/Documentation/userspace-api/media/videodev2.h.rst.exceptions
>>>>>>>> @@ -492,6 +492,7 @@ replace define V4L2_EVENT_CTRL_CH_FLAGS ctrl-changes-flags
>>>>>>>> replace define V4L2_EVENT_CTRL_CH_RANGE ctrl-changes-flags
>>>>>>>>
>>>>>>>> replace define V4L2_EVENT_SRC_CH_RESOLUTION src-changes-flags
>>>>>>>> +replace define V4L2_EVENT_SRC_CH_COLORIMETRY src-changes-flags
>>>>>>>>
>>>>>>>> replace define V4L2_EVENT_MD_FL_HAVE_FRAME_SEQ :c:type:`v4l2_event_motion_det`
>>>>>>>>
>>>>>>>> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
>>>>>>>> index 303805438814..b5838bc4e3a3 100644
>>>>>>>> --- a/include/uapi/linux/videodev2.h
>>>>>>>> +++ b/include/uapi/linux/videodev2.h
>>>>>>>> @@ -2351,6 +2351,7 @@ struct v4l2_event_frame_sync {
>>>>>>>> };
>>>>>>>>
>>>>>>>> #define V4L2_EVENT_SRC_CH_RESOLUTION (1 << 0)
>>>>>>>> +#define V4L2_EVENT_SRC_CH_COLORIMETRY (1 << 1)
>>>>>>>>
>>>>>>>> struct v4l2_event_src_change {
>>>>>>>> __u32 changes;
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>> --
>>>>>> regards,
>>>>>> Stan
>>>>
>>>> --
>>>> regards,
>>>> Stan
>>
>> --
>> regards,
>> Stan