[PATCH 0/9] media: cedrus: Add H264 decoding support

Maxime Ripard
2018-06-14T16:37:17Z

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@bootlin.com> 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)