On 6/11/19 10:13 AM, Hans Verkuil 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. I've been thinking about this a bit more: there are three different HW capabilities: 1) The hardware cannot parse the resolution at all and userspace has to tell it via S_FMT. 2) The hardware can parse the initial resolution, but is not able to handle mid-stream resolution changes. 3) The hardware can parse the initial resolution and all following mid-stream resolution changes. We can consider 2 the default situation. In case of 1 the SOURCE_CHANGE event is absent and userspace cannot subscribe to it. Question: do we want to flag this with the format as well? I.e. with a V4L2_FMT_FLAG_MANUAL_RESOLUTION? I think just not implementing the SOURCE_CHANGE event (and documenting this) is sufficient. In case of 3 the format sets the V4L2_FMT_FLAG_DYN_RESOLUTION flag. What do you think? Regards, Hans > > 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(-) >> >
