Re: [RFC PATCH 0/5] Add enum_fmt flag for coded formats with dynamic resolution switching

[Maxime Jourdan <mjourdan@baylibre.com>]

On Tue, Jun 11, 2019 at 10:13 AM Hans Verkuil <hverkuil@xs4all.nl> wrote:
>
> On 6/9/19 4:38 PM, Maxime Jourdan wrote:
> > Hello,
> >
> > This RFC proposes a new format flag - V4L2_FMT_FLAG_DYN_RESOLUTION - used
> > to tag coded formats for which the device supports dynamic resolution
> > switching, via V4L2_EVENT_SOURCE_CHANGE.
> > This includes the initial "source change" where the device is able to
> > tell userspace about the coded resolution and the DPB size (which
> > sometimes translates to V4L2_CID_MIN_BUFFERS_FOR_CAPTURE).
>
> Shouldn't the initial source change still be there? The amlogic decoder
> is capable of determining the resolution of the stream, right? It just
> can't handle mid-stream changes.
>

Well, no, not for older compressed formats like MPEG2. Sorry that this
wasn't clear all along. If it did, then the meson vdec wouldn't fail
current v4l2 compliance on the dqevent(V4L2_EVENT_SOURCE_CHANGE).
Userspace is expected to S_FMT the coded resolution before starting up
the capture. If the bitstream has a different resolution than the
format set, then you end up with cropped images. There's no risk for
dma overflow since those are configured to write at most up to buffer
capacity.

To be more precise: the firmware does report back the coded
resolution, but by that time it's too late as it has already begun
consuming the bitstream and decoding to buffers regardless.

For newer compressed formats (H264, HEVC, VP9), the firmware will
pause and notify a source change only, but with the older formats it's
more like "Hey, I decoded frames, by the way here is the coded
resolution".

Regards,
Maxime

> Regards,
>
>         Hans
>
> > This flag is mainly aimed at stateful decoder drivers.
> >
> > This RFC is motivated by my development on the amlogic video decoder
> > driver, which does not support dynamic resolution switching for older
> > coded formats (MPEG 1/2, MPEG 4 part II, H263). It does however support
> > it for the newer formats (H264, HEVC, VP9).
> >
> > The specification regarding stateful video decoders should be amended
> > to include that, in the absence of this flag for a certain format,
> > userspace is expected to extract the coded resolution and allocate
> > a sufficient amount of capture buffers on its own.
> > I understand that this point may be tricky, since older kernels with
> > close-to-spec drivers would not have this flag available, yet would
> > fully support dynamic resolution switching.
> > However, with the spec not merged in yet, I wanted to have your opinion
> > on this late addition.
> >
> > The RFC patches also adds support for this flag for the 4 following
> > stateful decoder drivers:
> >  - venus
> >  - s5p-mfc
> >  - mtk-vcodec
> >  - vicodec
> >
> > Maxime Jourdan (5):
> >   media: videodev2: add V4L2_FMT_FLAG_DYN_RESOLUTION
> >   media: venus: vdec: flag OUTPUT formats with
> >     V4L2_FMT_FLAG_DYN_RESOLUTION
> >   media: s5p_mfc_dec: flag OUTPUT formats with
> >     V4L2_FMT_FLAG_DYN_RESOLUTION
> >   media: mtk-vcodec: flag OUTPUT formats with
> >     V4L2_FMT_FLAG_DYN_RESOLUTION
> >   media: vicodec: flag vdec/stateful OUTPUT formats with
> >     V4L2_FMT_FLAG_DYN_RESOLUTION
> >
> >  Documentation/media/uapi/v4l/vidioc-enum-fmt.rst   |  7 +++++++
> >  drivers/media/platform/mtk-vcodec/mtk_vcodec_dec.c |  4 ++++
> >  drivers/media/platform/mtk-vcodec/mtk_vcodec_drv.h |  1 +
> >  drivers/media/platform/qcom/venus/core.h           |  1 +
> >  drivers/media/platform/qcom/venus/vdec.c           | 11 +++++++++++
> >  drivers/media/platform/s5p-mfc/s5p_mfc_common.h    |  1 +
> >  drivers/media/platform/s5p-mfc/s5p_mfc_dec.c       | 13 +++++++++++++
> >  drivers/media/platform/vicodec/vicodec-core.c      |  2 ++
> >  include/uapi/linux/videodev2.h                     |  5 +++--
> >  9 files changed, 43 insertions(+), 2 deletions(-)
> >
>