On Wed, 2020-11-11 at 00:28 +0200, Adrian Ratiu wrote: > Hi Ezequiel, > > On Tue, 10 Nov 2020, Ezequiel Garcia > <ezequiel@xxxxxxxxxxxxxxxxxxxx> wrote: > > On Mon, 2 Nov 2020 at 16:04, Adrian Ratiu > > <adrian.ratiu@xxxxxxxxxxxxx> wrote: > > > Dear all, > > > > > > This is v5 of the series adding VP9 profile 0 decoding to > > > rkvdec. > > > > > > All feedback from v4 should be addressed, there's just one > > > thing I did not address: ref_frame_sign_biases in the uAPI. The > > > userspace tool I'm > > > > I believe that Hantro G2 VP9 needs ref_frame_sign_biases. > > > > I think that it's also needed for the MTK decoder. Might be > > worth checking that as well, if the code is publicly available > > somewhere. > > I consulted the imx8m app ref manual for the Hantro G2 core and > indeed there's not one, but three fields at SWREG11 and 13 (last, > gold, alt) to signify sign biases for ref frames. Thanks for the > hint! > > > Coming to think about it, I think we are really close to having > > this uAPI directly upstream. > > > > Let's take a step back on why we have these uAPIs in the staging > > area. Couple years ago, there were some doubts in the media > > community about these uAPIs, and we wanted to wait a bit for > > more users before moving to public land. > > > > The uAPIs were meant to be in staging until enough users > > appeared and we were confident enough to move to stable. > > > > For VP9, given the feedback received through the year was > > already addressed, I think all that's left is to check the > > interface and make sure it can support Rockchip (RK3399, RK3326, > > etc), Hantro G2 and Mediatek, > > > > We will be very close to having a public API, and we could even > > merge it directly there. > > Thank you very much for this background. I understand that the > uAPI is independent from the driver implementations, so having a > good stable uAPI is beneficial when (for example) adding support > for VP9 on G2 in hantro or for upstream adoption of these > drivers. > > Given this rkvdec driver implementation is also adding the VP9 > uAPI and it's very close to stability (maybe only missing ref > frame sign bias, but who knows?) would you like to block its > submission until the uAPI is finalized or would it make sense to > treat the uAPI de-staging process separately because the uAPI is > independent from the driver? > I don't mean to block it, quite the opposite, to make sure we take this opportunity to go through Rockchip, Hantro and Mediatek, double-check the uAPI is covering all the VP9 syntax, and then target for public API. Cheers, Ezequiel > Thanks, > Adrian > > > Thanks, > > Ezequiel > > > > > using [1] apparently doesn't need it or the default hwreg value for it > > > is capable of decoding the bitstreams I used on the driver, so I don't > > > really have a use-case to change and test that. :) > > > > > > Considering the uAPI is a work in progress and expected to be modified, > > > ref_frame_sign_biases can be added later with others which might be > > > required to enable more functionality (for eg profiles >= 1). > > > > > > Series tested on rk3399 and applies on next-20201030. > > > > > > [1] https://github.com/Kwiboo/FFmpeg/tree/v4l2-request-hwaccel-4.2.2-rkvdec > > > > > > Changelog > > > --------- > > > > > > v5: > > > > > > * Drop unnecessary OUTPUT buffer payload set in .buf_prepare. > > > * Drop obsolete .per_request ctrl flag > > > * Added new vp9 ctrls to v4l2_ctrl_ptr > > > * Fix pahole detected padding issues > > > * Send userspace an error if it tries to reconfigure decode resolution > > > as v4l2 or rkvdec-vp9 backend do not support dynamic res changes yet > > > * Allow frame ctx probability tables to be non-mandatory so users can > > > set them directly during frame decoding in cases where no defaults > > > have been set previously (eg. ffmpeg vp9 backend) > > > * Some comments and documentation clarifications > > > * Minor checkpatch fixes > > > > > > v4: > > > > > > * Drop color_space field from the VP9 interface. > > > V4L2 API should be used for it. > > > * Clarified Segment-ID comments. > > > * Moved motion vector probabilities to a separate > > > struct. > > > > > > v3: > > > > > > * Fix documentation issues found by Hans. > > > * Fix smatch detected issues as pointed out by Hans. > > > * Added patch to fix wrong bytesused set on .buf_prepare. > > > > > > v2: > > > > > > * Documentation style issues pointed out by Nicolas internally. > > > * s/VP9_PROFILE_MAX/V4L2_VP9_PROFILE_MAX/ > > > * Fix wrong kfree(ctx). > > > * constify a couple structs on rkvdec-vp9.c > > > > > > > > > Boris Brezillon (2): > > > media: uapi: Add VP9 stateless decoder controls > > > media: rkvdec: Add the VP9 backend > > > > > > Ezequiel Garcia (1): > > > media: rkvdec: Fix .buf_prepare > > > > > > .../userspace-api/media/v4l/biblio.rst | 10 + > > > .../media/v4l/ext-ctrls-codec.rst | 550 ++++++ > > > drivers/media/v4l2-core/v4l2-ctrls.c | 239 +++ > > > drivers/media/v4l2-core/v4l2-ioctl.c | 1 + > > > drivers/staging/media/rkvdec/Makefile | 2 +- > > > drivers/staging/media/rkvdec/rkvdec-vp9.c | 1577 +++++++++++++++++ > > > drivers/staging/media/rkvdec/rkvdec.c | 72 +- > > > drivers/staging/media/rkvdec/rkvdec.h | 6 + > > > include/media/v4l2-ctrls.h | 5 + > > > include/media/vp9-ctrls.h | 486 +++++ > > > 10 files changed, 2942 insertions(+), 6 deletions(-) > > > create mode 100644 drivers/staging/media/rkvdec/rkvdec-vp9.c > > > create mode 100644 include/media/vp9-ctrls.h > > > > > > -- > > > 2.29.0 > > >
