lkml.org 
[lkml]   [2020]   [Nov]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH v5 0/3] media: rkvdec: Add a VP9 backend
From
Date
On Wed, 2020-11-11 at 00:28 +0200, Adrian Ratiu wrote:
> Hi Ezequiel,
>
> On Tue, 10 Nov 2020, Ezequiel Garcia
> <ezequiel@vanguardiasur.com.ar> wrote:
> > On Mon, 2 Nov 2020 at 16:04, Adrian Ratiu
> > <adrian.ratiu@collabora.com> 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
> > >


\
 
 \ /
  Last update: 2020-11-10 23:40    [W:0.056 / U:13.812 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site