From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-8.7 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,MENTIONS_GIT_HOSTING, SPF_HELO_NONE,SPF_PASS,UNPARSEABLE_RELAY,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 6EF84C388F7 for ; Tue, 10 Nov 2020 22:39:51 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 2C66B20781 for ; Tue, 10 Nov 2020 22:39:51 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1732233AbgKJWjt (ORCPT ); Tue, 10 Nov 2020 17:39:49 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:52842 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1731558AbgKJWjt (ORCPT ); Tue, 10 Nov 2020 17:39:49 -0500 Received: from bhuna.collabora.co.uk (bhuna.collabora.co.uk [IPv6:2a00:1098:0:82:1000:25:2eeb:e3e3]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 2FDE9C0613D1; Tue, 10 Nov 2020 14:39:49 -0800 (PST) Received: from [127.0.0.1] (localhost [127.0.0.1]) (Authenticated sender: ezequiel) with ESMTPSA id DB7A61F4554F Message-ID: Subject: Re: [PATCH v5 0/3] media: rkvdec: Add a VP9 backend From: Ezequiel Garcia To: Adrian Ratiu , Ezequiel Garcia Cc: linux-media , Mauro Carvalho Chehab , Boris Brezillon , Hans Verkuil , Laurent Pinchart , Jonas Karlman , "open list:ARM/Rockchip SoC..." , Linux Kernel Mailing List , kernel@collabora.com Date: Tue, 10 Nov 2020 19:39:38 -0300 In-Reply-To: <87y2j8hmoc.fsf@collabora.com> References: <20201102190551.1223389-1-adrian.ratiu@collabora.com> <87y2j8hmoc.fsf@collabora.com> Organization: Collabora Content-Type: text/plain; charset="UTF-8" User-Agent: Evolution 3.36.3-1 MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, 2020-11-11 at 00:28 +0200, Adrian Ratiu wrote: > Hi Ezequiel, > > On Tue, 10 Nov 2020, Ezequiel Garcia > wrote: > > On Mon, 2 Nov 2020 at 16:04, Adrian Ratiu > > 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 > > >