On 7/18/19 10:39 AM, Maxime Jourdan wrote: > On Mon, Jul 15, 2019 at 2: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. >> >> 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. >> > > I think that not implementing SOURCE_CHANGE is sufficient as well. The > issue (in my case), is that the amlogic decoder _does_ support the > event (case 3) for anything recent (H264, HEVC, VP9), but not for e.g > MPEG 1/2 (case 1). > > A possible solution would be to create 2 separate devices, one > implementing the event, the other not. Do you think this is reasonable > ? This would discard the need for all the proposed flags, unless there > are other decoder drivers that fall in case 2. I don't think it is a good idea to create two device nodes, that's really confusing. Instead I think we just need a V4L2_FMT_FLAG_MANUAL_RESOLUTION flag. BTW, what happens if the application sets the format to e.g. 640x480 but the MPEG file is a different resolution? Does the decoder fail to produce anything? Or does it internally parse the resolution from the bitstream and start decoding it? What if the bitstream resolution is larger than the resolution set with S_FMT? Does it check for the buffer size? I just want to make sure it won't write past the end of the buffer. Regards, Hans > >> 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(-) >>>> >>> >>
