Hi Hans, On 1/15/20 5:03 PM, Hans Verkuil wrote: > On 1/10/20 11:54 AM, Stanimir Varbanov wrote: >> Hi Hans, >> >> On 1/9/20 10:57 AM, Hans Verkuil wrote: >>> On 1/9/20 8:41 AM, Stanimir Varbanov wrote: >>>> Hi Hans, >>>> >>>> On 1/8/20 6:09 PM, Hans Verkuil wrote: >>>>> On 1/6/20 4:49 PM, Stanimir Varbanov wrote: >>>>>> This event indicate that the source color bit-depth is changed >>>>>> during run-time. The client must get the new format and re-allocate >>>>>> buffers for it. This can usually happens with video decoder (encoders) >>>>>> when the bit-stream color bit-depth is changed from 8 to 10bits >>>>>> or vice versa. >>>>>> >>>>>> Acked-by: Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx> >>>>>> Signed-off-by: Stanimir Varbanov <stanimir.varbanov@xxxxxxxxxx> >>>>>> --- >>>>>> Documentation/media/uapi/v4l/vidioc-dqevent.rst | 8 +++++++- >>>>>> Documentation/media/videodev2.h.rst.exceptions | 1 + >>>>>> include/uapi/linux/videodev2.h | 1 + >>>>>> 3 files changed, 9 insertions(+), 1 deletion(-) >>>>>> >>>>>> diff --git a/Documentation/media/uapi/v4l/vidioc-dqevent.rst b/Documentation/media/uapi/v4l/vidioc-dqevent.rst >>>>>> index 42659a3d1705..fad853d440cf 100644 >>>>>> --- a/Documentation/media/uapi/v4l/vidioc-dqevent.rst >>>>>> +++ b/Documentation/media/uapi/v4l/vidioc-dqevent.rst >>>>>> @@ -402,7 +402,13 @@ 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_COLOR_DEPTH`` >>>>>> + - 0x0002 >>>>>> + - This event gets triggered when color bit-depth change is detected >>>>>> + from a video decoder. Applications will have to query the new pixel >>>>>> + format and re-negotiate the queue. In most cases the streaming must be >>>>>> + stopped and restarted (:ref:`VIDIOC_STREAMOFF <VIDIOC_STREAMON>` >>>>>> + followed by :ref:`VIDIOC_STREAMON <VIDIOC_STREAMON>`). >>>>> >>>>> I think this is too specific for decoders. Something similar to the >>>>> CH_RESOLUTION description would be more appropriate: >>>>> >>>>> - This event gets triggered when a color bit-depth change (but not a >>>>> resolution change!) is detected at an input. This can come from an >>>> >>>> What you mean by "but not a resolution change" here? Resolution change >>>> and bit-depth change cannot occur on the same time, or something else. >>> >>> What I was trying to say is that a resolution change implies a possible bit-depth >>> change as well, whereas V4L2_EVENT_SRC_CH_COLOR_DEPTH is only set if there is >>> a bit-depth change but no resolution change. >>> >>> V4L2_EVENT_SRC_CH_RESOLUTION requires that userspace does a full resync to the >>> new format, CH_COLOR_DEPTH implies that only the bit depth changed. >> >> CH_COLOR_DEPTH implies format re-negotiation as well. In Venus case >> 8->10bit change will change the format of OPB buffers (now it is not >> possible because of lack of v4l modifiers) and DPB buffers becomes >> compressed raw buffers (to optimize bandwidth). >> >>> >>> Which actually makes me wonder: is there a difference between the two change flags >>> w.r.t. userspace behavior? If there is, then that should be carefully documented, >>> if there isn't, then is this new flag really needed? >> >> Looking into semantics of v4l events, CH_COLOR_DEPTH makes sense because >> it describes what actually changed (similar to CH_RESOLUTION). I would >> say that v4l2_event::type V4L2_EVENT_SOURCE_CHANGE implies format >> re-negotiation and v4l2_event::src_change just informs userland what >> exactly is changed. >> >> I'll postpone this patch until we have clear vision what will be the >> usage in user-space. >> > > My main problem regarding semantics is what should be done if you disconnect > and reconnect an HDMI (for example) connector. You get a V4L2_EVENT_SOURCE_CHANGE > when the new signal is detected, but should it just set V4L2_EVENT_SRC_CH_RESOLUTION > (as it does today), or also V4L2_EVENT_SRC_CH_COLOR_DEPTH? I think disconnect -> connect should imply that everything is changed i.e. source resolution, color depth and colorimetry. We cannot guess is it the same HDMI source or not. > > In my view V4L2_EVENT_SRC_CH_COLOR_DEPTH only makes sense if the resolution > stays the same, but only the color depth changes. I can imagine a bitstream which changing resolution and color-depth at the same time. And I guess h264|265 doesn't denied this to happen. > > BTW, one thing that will be added here as well in the future is a > V4L2_EVENT_SRC_CH_COLORIMETRY for when the colorspace etc. information changes, > but nothing else. In that case there is no need to renegotiate the format etc., > it's just the interpretation of the video data that changes. In case of colorimetry, maybe the buffer format will not change but colorspace, quantization, ycbcr_encoding and transfer function will change. And this imply that the userspace have to streamoff -> gfmt -> do-some-action -> streamon ? I have to figure out what is for example Android is doing when the colorimentry is changed. If someone knows, please shed some light on that subject. > > An alternative approach is to define a V4L2_EVENT_SRC_CH_ALL bit mask, OR-ing > all the V4L2_EVENT_SRC_CH_* defines, and change all drivers that use > V4L2_EVENT_SRC_CH_RESOLUTION at the moment to use V4L2_EVENT_SRC_CH_ALL instead, > and only drivers that detect that only one of these changes took place will > use a specific V4L2_EVENT_SRC_CH_ flag. This will be primarily codec drivers, > I believe. I can do that as an RFC. > > There aren't that many of those, so this shouldn't be too difficult to do. > > Perhaps this is the cleanest approach to this problem... > > Regards, > > Hans > -- regards, Stan
