From: Ezequiel Garcia <ezequiel@collabora.com>
To: Adrian Ratiu <adrian.ratiu@collabora.com>,
Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>
Cc: kernel@collabora.com, Jonas Karlman <jonas@kwiboo.se>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
"open list:ARM/Rockchip SoC..."
<linux-rockchip@lists.infradead.org>,
Boris Brezillon <boris.brezillon@collabora.com>,
Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
Hans Verkuil <hverkuil-cisco@xs4all.nl>,
Mauro Carvalho Chehab <mchehab@kernel.org>,
linux-media <linux-media@vger.kernel.org>
Subject: Re: [PATCH v5 0/3] media: rkvdec: Add a VP9 backend
Date: Tue, 10 Nov 2020 19:39:38 -0300 [thread overview]
Message-ID: <ee12e70c92bacf1080ceae82feafa736c0719a2e.camel@collabora.com> (raw)
In-Reply-To: <87y2j8hmoc.fsf@collabora.com>
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
> > >
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
next prev parent reply other threads:[~2020-11-10 22:44 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-11-02 19:05 [PATCH v5 0/3] media: rkvdec: Add a VP9 backend Adrian Ratiu
2020-11-02 19:05 ` [PATCH v5 1/3] media: rkvdec: Fix .buf_prepare Adrian Ratiu
2020-11-02 19:05 ` [PATCH v5 2/3] media: uapi: Add VP9 stateless decoder controls Adrian Ratiu
2021-02-22 21:00 ` Nicolas Dufresne
2021-02-22 21:43 ` Nicolas Dufresne
2020-11-02 19:05 ` [PATCH v5 3/3] media: rkvdec: Add the VP9 backend Adrian Ratiu
2020-11-10 21:03 ` [PATCH v5 0/3] media: rkvdec: Add a " Ezequiel Garcia
2020-11-10 22:28 ` Adrian Ratiu
2020-11-10 22:39 ` Ezequiel Garcia [this message]
2020-11-11 0:11 ` Adrian Ratiu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ee12e70c92bacf1080ceae82feafa736c0719a2e.camel@collabora.com \
--to=ezequiel@collabora.com \
--cc=adrian.ratiu@collabora.com \
--cc=boris.brezillon@collabora.com \
--cc=ezequiel@vanguardiasur.com.ar \
--cc=hverkuil-cisco@xs4all.nl \
--cc=jonas@kwiboo.se \
--cc=kernel@collabora.com \
--cc=laurent.pinchart@ideasonboard.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-media@vger.kernel.org \
--cc=linux-rockchip@lists.infradead.org \
--cc=mchehab@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).