| Message ID | 20200106154929.4331-5-stanimir.varbanov@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | None | expand |
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@linux.intel.com> >>> Signed-off-by: Stanimir Varbanov <stanimir.varbanov@linaro.org> >>> --- >>> 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. 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? Regards, Hans > > I would say that for Venus (and probably others) on initialization time > both could be changed on the same time, because we cannot predict the > resolution and bit-depth before parsing bitstream headers. > >> input connector or from a video decoder. Applications will have to query >> the new pixel format and re-negotiate the queue. >> >> For stateful decoders follow the guidelines in :ref:`decoder`. >> Video capture devices will in most cases have to stop and restart >> streaming (:ref:`VIDIOC_STREAMOFF <VIDIOC_STREAMON>` followed by >> :ref:`VIDIOC_STREAMON <VIDIOC_STREAMON>`). >> >> And update dev-decoder.rst where needed with this new event flag. >> >> As to your question on irc: once I've acked this patch it can be merged >> via a venus PR. >> >> Regards, >> >> Hans >> >>> >>> Return Value >>> ============ >>> diff --git a/Documentation/media/videodev2.h.rst.exceptions b/Documentation/media/videodev2.h.rst.exceptions >>> index cb6ccf91776e..209709114378 100644 >>> --- a/Documentation/media/videodev2.h.rst.exceptions >>> +++ b/Documentation/media/videodev2.h.rst.exceptions >>> @@ -490,6 +490,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_COLOR_DEPTH 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 5f9357dcb060..1d349c9d57a7 100644 >>> --- a/include/uapi/linux/videodev2.h >>> +++ b/include/uapi/linux/videodev2.h >>> @@ -2332,6 +2332,7 @@ struct v4l2_event_frame_sync { >>> }; >>> >>> #define V4L2_EVENT_SRC_CH_RESOLUTION (1 << 0) >>> +#define V4L2_EVENT_SRC_CH_COLOR_DEPTH (1 << 1) >>> >>> struct v4l2_event_src_change { >>> __u32 changes; >>> >> >
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@linux.intel.com> >>>> Signed-off-by: Stanimir Varbanov <stanimir.varbanov@linaro.org> >>>> --- >>>> 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.
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>`). Return Value ============ diff --git a/Documentation/media/videodev2.h.rst.exceptions b/Documentation/media/videodev2.h.rst.exceptions index cb6ccf91776e..209709114378 100644 --- a/Documentation/media/videodev2.h.rst.exceptions +++ b/Documentation/media/videodev2.h.rst.exceptions @@ -490,6 +490,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_COLOR_DEPTH 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 5f9357dcb060..1d349c9d57a7 100644 --- a/include/uapi/linux/videodev2.h +++ b/include/uapi/linux/videodev2.h @@ -2332,6 +2332,7 @@ struct v4l2_event_frame_sync { }; #define V4L2_EVENT_SRC_CH_RESOLUTION (1 << 0) +#define V4L2_EVENT_SRC_CH_COLOR_DEPTH (1 << 1) struct v4l2_event_src_change { __u32 changes;