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=-0.8 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,URIBL_BLOCKED autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by aws-us-west-2-korg-lkml-1.web.codeaurora.org (Postfix) with ESMTP id 576B5C433EF for ; Thu, 14 Jun 2018 16:37:24 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 0D77A208B2 for ; Thu, 14 Jun 2018 16:37:24 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 0D77A208B2 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=bootlin.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755330AbeFNQhV convert rfc822-to-8bit (ORCPT ); Thu, 14 Jun 2018 12:37:21 -0400 Received: from mail.bootlin.com ([62.4.15.54]:56728 "EHLO mail.bootlin.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755227AbeFNQhU (ORCPT ); Thu, 14 Jun 2018 12:37:20 -0400 Received: by mail.bootlin.com (Postfix, from userid 110) id C15FD206A6; Thu, 14 Jun 2018 18:37:18 +0200 (CEST) Received: from localhost (LFbn-TOU-1-32-230.w86-201.abo.wanadoo.fr [86.201.126.230]) by mail.bootlin.com (Postfix) with ESMTPSA id 89AA4203D9; Thu, 14 Jun 2018 18:37:18 +0200 (CEST) Date: Thu, 14 Jun 2018 18:37:17 +0200 From: Maxime Ripard To: Tomasz Figa Cc: Hans Verkuil , Alexandre Courbot , Sakari Ailus , Laurent Pinchart , Pawel Osciak , Paul Kocialkowski , Chen-Yu Tsai , Linux Kernel Mailing List , "list@263.net:IOMMU DRIVERS , Joerg Roedel ," , Linux Media Mailing List , nicolas.dufresne@collabora.com, jenskuske@gmail.com, linux-sunxi@googlegroups.com, thomas.petazzoni@bootlin.com Subject: Re: [PATCH 0/9] media: cedrus: Add H264 decoding support Message-ID: <20180614163717.hqxeiwn63vvnk5xg@flea> References: <20180613140714.1686-1-maxime.ripard@bootlin.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: 8BIT In-Reply-To: User-Agent: NeoMutt/20180512 Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Tomasz, On Thu, Jun 14, 2018 at 10:00:43PM +0900, Tomasz Figa wrote: > Hi Maxime, > > On Wed, Jun 13, 2018 at 11:07 PM Maxime Ripard > wrote: > > > > Hi, > > > > Here is a preliminary version of the H264 decoding support in the > > cedrus driver. > > Thanks for the series! Let me reply inline to some of the points raised here. > > > As you might already know, the cedrus driver relies on the Request > > API, and is a reverse engineered driver for the video decoding engine > > found on the Allwinner SoCs. > > > > This work has been possible thanks to the work done by the people > > behind libvdpau-sunxi found here: > > https://github.com/linux-sunxi/libvdpau-sunxi/ > > > > This driver is based on the last version of the cedrus driver sent by > > Paul, based on Request API v13 sent by Hans: > > https://lkml.org/lkml/2018/5/7/316 > > Just FYI, there is v15 already. :) Yeah, we know, Paul is currently working on rebasing to that version :) > > This driver has been tested only with baseline profile videos, and is > > missing a few key features to decode videos with higher profiles. > > This has been tested using our cedrus-frame-test tool, which should be > > a quite generic v4l2-to-drm decoder using the request API to > > demonstrate the video decoding: > > https://github.com/free-electrons/cedrus-frame-test/, branch h264 > > > > However, sending this preliminary version, I'd really like to start a > > discussion and get some feedback on the user-space API for the H264 > > controls exposed through the request API. > > > > I've been using the controls currently integrated into ChromeOS that > > have a working version of this particular setup. However, these > > controls have a number of shortcomings and inconsistencies with other > > decoding API. I've worked with libva so far, but I've noticed already > > that: > > Note that these controls are supposed to be defined exactly like the > bitstream headers deserialized into C structs in memory. I believe > Pawel (on CC) defined them based on the actual H264 specification. > > > - The kernel UAPI expects to have the nal_ref_idc variable, while > > libva only exposes whether that frame is a reference frame or > > not. I've looked at the rockchip driver in the ChromeOS tree, and > > our own driver, and they both need only the information about > > whether the frame is a reference one or not, so maybe we should > > change this? > > The fact that 2 drivers only need partial information doesn't mean > that we should ignore the data being already in the bitstream. IMHO > this API should to provide all the metadata available in the stream to > the kernel driver, as a replacement for bitstream parsing in firmware > (or in kernel... yuck). The point is more that libva will only pass the result of (nal_ref_idc != 0). So in the libva plugin, you won't be able to fill the proper value to the kernel, since you don't have access to it. > > - The H264 bitstream exposes the picture default reference list (for > > both list 0 and list 1), the slice reference list and an override > > flag. The libva will only pass the reference list to be used (so > > either the picture default's or the slice's) depending on the > > override flag. The kernel UAPI wants the picture default reference > > list and the slice reference list, but doesn't expose the override > > flag, which prevents us from configuring properly the > > hardware. Our video decoding engine needs the three information, > > but we can easily adapt to having only one. However, having two > > doesn't really work for us. > > Where does the override flag come from? If it's in the bitstream, then > I guess it was just missed when creating the structures. It's in the bitstream yeah. I'll add it then. Maxime -- Maxime Ripard, Bootlin (formerly Free Electrons) Embedded Linux and Kernel engineering https://bootlin.com From mboxrd@z Thu Jan 1 00:00:00 1970 From: maxime.ripard@bootlin.com (Maxime Ripard) Date: Thu, 14 Jun 2018 18:37:17 +0200 Subject: [PATCH 0/9] media: cedrus: Add H264 decoding support In-Reply-To: References: <20180613140714.1686-1-maxime.ripard@bootlin.com> Message-ID: <20180614163717.hqxeiwn63vvnk5xg@flea> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org Hi Tomasz, On Thu, Jun 14, 2018 at 10:00:43PM +0900, Tomasz Figa wrote: > Hi Maxime, > > On Wed, Jun 13, 2018 at 11:07 PM Maxime Ripard > wrote: > > > > Hi, > > > > Here is a preliminary version of the H264 decoding support in the > > cedrus driver. > > Thanks for the series! Let me reply inline to some of the points raised here. > > > As you might already know, the cedrus driver relies on the Request > > API, and is a reverse engineered driver for the video decoding engine > > found on the Allwinner SoCs. > > > > This work has been possible thanks to the work done by the people > > behind libvdpau-sunxi found here: > > https://github.com/linux-sunxi/libvdpau-sunxi/ > > > > This driver is based on the last version of the cedrus driver sent by > > Paul, based on Request API v13 sent by Hans: > > https://lkml.org/lkml/2018/5/7/316 > > Just FYI, there is v15 already. :) Yeah, we know, Paul is currently working on rebasing to that version :) > > This driver has been tested only with baseline profile videos, and is > > missing a few key features to decode videos with higher profiles. > > This has been tested using our cedrus-frame-test tool, which should be > > a quite generic v4l2-to-drm decoder using the request API to > > demonstrate the video decoding: > > https://github.com/free-electrons/cedrus-frame-test/, branch h264 > > > > However, sending this preliminary version, I'd really like to start a > > discussion and get some feedback on the user-space API for the H264 > > controls exposed through the request API. > > > > I've been using the controls currently integrated into ChromeOS that > > have a working version of this particular setup. However, these > > controls have a number of shortcomings and inconsistencies with other > > decoding API. I've worked with libva so far, but I've noticed already > > that: > > Note that these controls are supposed to be defined exactly like the > bitstream headers deserialized into C structs in memory. I believe > Pawel (on CC) defined them based on the actual H264 specification. > > > - The kernel UAPI expects to have the nal_ref_idc variable, while > > libva only exposes whether that frame is a reference frame or > > not. I've looked at the rockchip driver in the ChromeOS tree, and > > our own driver, and they both need only the information about > > whether the frame is a reference one or not, so maybe we should > > change this? > > The fact that 2 drivers only need partial information doesn't mean > that we should ignore the data being already in the bitstream. IMHO > this API should to provide all the metadata available in the stream to > the kernel driver, as a replacement for bitstream parsing in firmware > (or in kernel... yuck). The point is more that libva will only pass the result of (nal_ref_idc != 0). So in the libva plugin, you won't be able to fill the proper value to the kernel, since you don't have access to it. > > - The H264 bitstream exposes the picture default reference list (for > > both list 0 and list 1), the slice reference list and an override > > flag. The libva will only pass the reference list to be used (so > > either the picture default's or the slice's) depending on the > > override flag. The kernel UAPI wants the picture default reference > > list and the slice reference list, but doesn't expose the override > > flag, which prevents us from configuring properly the > > hardware. Our video decoding engine needs the three information, > > but we can easily adapt to having only one. However, having two > > doesn't really work for us. > > Where does the override flag come from? If it's in the bitstream, then > I guess it was just missed when creating the structures. It's in the bitstream yeah. I'll add it then. Maxime -- Maxime Ripard, Bootlin (formerly Free Electrons) Embedded Linux and Kernel engineering https://bootlin.com