On 7/19/19 4:45 AM, Tomasz Figa wrote: > On Mon, Jul 15, 2019 at 9:37 PM Hans Verkuil <hverkuil@xxxxxxxxx> wrote: >> >> 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. > > Any particular reason for 2 being the default? I'm especially > wondering about that as most of the drivers actually provide 3. Various reasons: 1) I prefer to have a flag indicating what IS supported rather than what isn't. 2) An application that checks this flag and doesn't see it (i.e. if a flag-aware application is used with an older kernel where these flags aren't set) will still work, but with possibly reduced functionality. If the flag would indicate that something is NOT supported, then they would fail when combined with an older kernel and a driver that doesn't support dynamic resolution changes. 3) None of the encoders support it, so there too it makes sense to have 'no dynamic resolution change' as the default. It's nicely symmetrical for encoders and decoders. 4) Some formats do not support it, so again, having no dynamic res changes as the default makes sense. Regards, Hans > >> >> 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(-) >>>> >>> >>
