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 > <maxime.ripard@xxxxxxxxxxx> 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
