| Message ID | 20190609143820.4662-2-mjourdan@baylibre.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add enum_fmt flag for coded formats with dynamic resolution switching | expand |
Hi Maxime, On Sun, Jun 9, 2019 at 11:38 PM Maxime Jourdan <mjourdan@baylibre.com> wrote: > > Add a enum_fmt format flag to specifically tag coded formats where > dynamic resolution switching is supported by the device. > > This is useful for some codec drivers that can't support dynamic > resolution switching for all their listed coded formats. It allows > userspace to know whether it should extract the video parameters itself, > or if it can rely on the device to send V4L2_EVENT_SOURCE_CHANGE when > such changes are detected. > First of all, thanks for the patch! Given the aspect of compatibility and also the general preference for the drivers to actually handle dynamic resolution changes, I'd suggest inverting the meaning of this flag. With something like "V4L2_FMT_FLAG_STATIC_RESOLUTION" it would be more of an exception rather than the default behavior. Best regards, Tomasz
On 6/9/19 4:38 PM, Maxime Jourdan wrote: > Add a enum_fmt format flag to specifically tag coded formats where > dynamic resolution switching is supported by the device. > > This is useful for some codec drivers that can't support dynamic can't -> can > resolution switching for all their listed coded formats. It allows > userspace to know whether it should extract the video parameters itself, > or if it can rely on the device to send V4L2_EVENT_SOURCE_CHANGE when > such changes are detected. > > Signed-off-by: Maxime Jourdan <mjourdan@baylibre.com> > --- > Documentation/media/uapi/v4l/vidioc-enum-fmt.rst | 7 +++++++ > include/uapi/linux/videodev2.h | 5 +++-- > 2 files changed, 10 insertions(+), 2 deletions(-) > > diff --git a/Documentation/media/uapi/v4l/vidioc-enum-fmt.rst b/Documentation/media/uapi/v4l/vidioc-enum-fmt.rst > index 822d6730e7d2..92ddd4ddbce2 100644 > --- a/Documentation/media/uapi/v4l/vidioc-enum-fmt.rst > +++ b/Documentation/media/uapi/v4l/vidioc-enum-fmt.rst > @@ -127,6 +127,13 @@ one until ``EINVAL`` is returned. > - This format is not native to the device but emulated through > software (usually libv4l2), where possible try to use a native > format instead for better performance. > + * - ``V4L2_FMT_FLAG_DYN_RESOLUTION`` > + - 0x0004 > + - Dynamic resolution switching is supported by the device for this > + coded format. It will notify the user via the event I'd say 'compressed bitstream format (aka coded format)'. Also mention that this flag can only be used in combination with the COMPRESSED flag, since this applies to compressed formats only. Regards, Hans > + ``V4L2_EVENT_SOURCE_CHANGE`` when changes in the video parameters > + are detected. > + > > > Return Value > diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h > index 1050a75fb7ef..834550e20ee7 100644 > --- a/include/uapi/linux/videodev2.h > +++ b/include/uapi/linux/videodev2.h > @@ -768,8 +768,9 @@ struct v4l2_fmtdesc { > __u32 reserved[4]; > }; > > -#define V4L2_FMT_FLAG_COMPRESSED 0x0001 > -#define V4L2_FMT_FLAG_EMULATED 0x0002 > +#define V4L2_FMT_FLAG_COMPRESSED 0x0001 > +#define V4L2_FMT_FLAG_EMULATED 0x0002 > +#define V4L2_FMT_FLAG_DYN_RESOLUTION 0x0004 > > /* Frame Size and frame rate enumeration */ > /* >
On 6/10/19 5:48 AM, Tomasz Figa wrote: > Hi Maxime, > > On Sun, Jun 9, 2019 at 11:38 PM Maxime Jourdan <mjourdan@baylibre.com> wrote: >> >> Add a enum_fmt format flag to specifically tag coded formats where >> dynamic resolution switching is supported by the device. >> >> This is useful for some codec drivers that can't support dynamic >> resolution switching for all their listed coded formats. It allows >> userspace to know whether it should extract the video parameters itself, >> or if it can rely on the device to send V4L2_EVENT_SOURCE_CHANGE when >> such changes are detected. >> > > First of all, thanks for the patch! > > Given the aspect of compatibility and also the general preference for > the drivers to actually handle dynamic resolution changes, I'd suggest > inverting the meaning of this flag. With something like > "V4L2_FMT_FLAG_STATIC_RESOLUTION" it would be more of an exception > rather than the default behavior. We had a discussion about that, see this thread: https://lkml.org/lkml/2019/5/31/256 Regards, Hans
Hi Tomasz, On Mon, Jun 10, 2019 at 5:48 AM Tomasz Figa <tfiga@chromium.org> wrote: > > Hi Maxime, > > On Sun, Jun 9, 2019 at 11:38 PM Maxime Jourdan <mjourdan@baylibre.com> wrote: > > > > Add a enum_fmt format flag to specifically tag coded formats where > > dynamic resolution switching is supported by the device. > > > > This is useful for some codec drivers that can't support dynamic > > resolution switching for all their listed coded formats. It allows > > userspace to know whether it should extract the video parameters itself, > > or if it can rely on the device to send V4L2_EVENT_SOURCE_CHANGE when > > such changes are detected. > > > > First of all, thanks for the patch! > > Given the aspect of compatibility and also the general preference for > the drivers to actually handle dynamic resolution changes, I'd suggest > inverting the meaning of this flag. With something like > "V4L2_FMT_FLAG_STATIC_RESOLUTION" it would be more of an exception > rather than the default behavior. > This is actually what I did to begin with [0], with the same reasoning: not supporting dynamic resolution for a certain coded format is more of an exception than the norm (for decoders). The patch was ultimately dropped from the meson vdec series after discussing with Hans, see [0] or the lkml link Hans provided in his answer. We have the chance today that stateful decoders in the kernel either support V4L2_EVENT_SOURCE_CHANGE and dynamic resolution switching for all their formats, or they don't expose V4L2_EVENT_SOURCE_CHANGE at all. While this flag would change the spec, it wouldn't break existing userspace using close-to-spec drivers like s5p-mfc or mtk-vcodec. Cheers, Maxime [0] https://patchwork.kernel.org/patch/10969829/ > Best regards, > Tomasz
On Wed, Jun 12, 2019 at 1:46 AM Maxime Jourdan <mjourdan@baylibre.com> wrote: > > Hi Tomasz, > On Mon, Jun 10, 2019 at 5:48 AM Tomasz Figa <tfiga@chromium.org> wrote: > > > > Hi Maxime, > > > > On Sun, Jun 9, 2019 at 11:38 PM Maxime Jourdan <mjourdan@baylibre.com> wrote: > > > > > > Add a enum_fmt format flag to specifically tag coded formats where > > > dynamic resolution switching is supported by the device. > > > > > > This is useful for some codec drivers that can't support dynamic > > > resolution switching for all their listed coded formats. It allows > > > userspace to know whether it should extract the video parameters itself, > > > or if it can rely on the device to send V4L2_EVENT_SOURCE_CHANGE when > > > such changes are detected. > > > > > > > First of all, thanks for the patch! > > > > Given the aspect of compatibility and also the general preference for > > the drivers to actually handle dynamic resolution changes, I'd suggest > > inverting the meaning of this flag. With something like > > "V4L2_FMT_FLAG_STATIC_RESOLUTION" it would be more of an exception > > rather than the default behavior. > > > > This is actually what I did to begin with [0], with the same > reasoning: not supporting dynamic resolution for a certain coded > format is more of an exception than the norm (for decoders). > The patch was ultimately dropped from the meson vdec series after > discussing with Hans, see [0] or the lkml link Hans provided in his > answer. > > We have the chance today that stateful decoders in the kernel either > support V4L2_EVENT_SOURCE_CHANGE and dynamic resolution switching for > all their formats, or they don't expose V4L2_EVENT_SOURCE_CHANGE at > all. > While this flag would change the spec, it wouldn't break existing > userspace using close-to-spec drivers like s5p-mfc or mtk-vcodec. Fair enough. Feel free to add my Reviewed-by, after Hans's comments are fixed. Best regards, Tomasz > > Cheers, > Maxime > > [0] https://patchwork.kernel.org/patch/10969829/ > > > Best regards, > > Tomasz
diff --git a/Documentation/media/uapi/v4l/vidioc-enum-fmt.rst b/Documentation/media/uapi/v4l/vidioc-enum-fmt.rst index 822d6730e7d2..92ddd4ddbce2 100644 --- a/Documentation/media/uapi/v4l/vidioc-enum-fmt.rst +++ b/Documentation/media/uapi/v4l/vidioc-enum-fmt.rst @@ -127,6 +127,13 @@ one until ``EINVAL`` is returned. - This format is not native to the device but emulated through software (usually libv4l2), where possible try to use a native format instead for better performance. + * - ``V4L2_FMT_FLAG_DYN_RESOLUTION`` + - 0x0004 + - Dynamic resolution switching is supported by the device for this + coded format. It will notify the user via the event + ``V4L2_EVENT_SOURCE_CHANGE`` when changes in the video parameters + are detected. + Return Value diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h index 1050a75fb7ef..834550e20ee7 100644 --- a/include/uapi/linux/videodev2.h +++ b/include/uapi/linux/videodev2.h @@ -768,8 +768,9 @@ struct v4l2_fmtdesc { __u32 reserved[4]; }; -#define V4L2_FMT_FLAG_COMPRESSED 0x0001 -#define V4L2_FMT_FLAG_EMULATED 0x0002 +#define V4L2_FMT_FLAG_COMPRESSED 0x0001 +#define V4L2_FMT_FLAG_EMULATED 0x0002 +#define V4L2_FMT_FLAG_DYN_RESOLUTION 0x0004 /* Frame Size and frame rate enumeration */ /*
Add a enum_fmt format flag to specifically tag coded formats where dynamic resolution switching is supported by the device. This is useful for some codec drivers that can't support dynamic resolution switching for all their listed coded formats. It allows userspace to know whether it should extract the video parameters itself, or if it can rely on the device to send V4L2_EVENT_SOURCE_CHANGE when such changes are detected. Signed-off-by: Maxime Jourdan <mjourdan@baylibre.com> --- Documentation/media/uapi/v4l/vidioc-enum-fmt.rst | 7 +++++++ include/uapi/linux/videodev2.h | 5 +++-- 2 files changed, 10 insertions(+), 2 deletions(-)