Re: [PATCH] media: cedrus: Add support for VP8 decoding

From: Jernej Åkrabec
Date: Wed May 20 2020 - 18:31:04 EST


Dne sreda, 20. maj 2020 ob 23:43:40 CEST je Nicolas Dufresne napisal(a):
> Le mercredi 20 mai 2020 à 23:01 +0200, Jernej Skrabec a écrit :
> > VP8 in Cedrus shares same engine as H264.
> >
> > Note that it seems necessary to call bitstream parsing functions,
> > to parse frame header, otherwise decoded image is garbage. This is
> > contrary to what is driver supposed to do. However, values are not
> > really used, so this might be acceptable. It's possible that bitstream
>
> Have you verified that all values passed through controls are not used
> ? To remain a stateless driver, there is no requirement for parsed data
> to be used, the only requirement is that the reference are used.
> Otherwise doing parallel decoding of two stream of different stream
> would be broken. Have you verified that parallel decoding is working as
> expected ?

I'm not sure if you understand what I meant. Although userspace app parses
frame header and fills all data in VP8 control, driver parses frame header
again, using HW bitstream parsing functionality in cedrus_read_header().
Without that second header parsing in HW, decoded image is garbage. Note that
cedrus_read_header() discards all parsed values and relies on those provided
in controls.

This parsing doesn't cause any problems with parallel decoding or anything.
It's done during frame decoding job, so it doesn't affect any state. It's just
that we shouldn't need to parse header in driver because all data is already
provided in controls. It seems that Cedrus core was never tested without that
HW frame header parsing. I found out that HEVC and H264 frames can sometimes
also be wrongly decoded if no bitstream parsing function is triggered in HW
before final decoding.

I spend a lot of time trying to avoid that header parsing, but I couldn't find
any way around it.

In another words, Cedrus VPU provides two functionalities - HW bitstream
parsing (to speed up header parsing) and video decoding. One would thought
that video decoding can be used independently, if all data from header is
already known, but it can't be.

Best regards,
Jernej

>
> > parsing functions set some internal VPU state, which is later necessary
> > for proper decoding. Biggest suspect is "VP8 probs update" trigger.
> >
> > Signed-off-by: Jernej Skrabec <jernej.skrabec@xxxxxxxx>
> > ---
> >
> > drivers/staging/media/sunxi/cedrus/Makefile | 3 +-
> > drivers/staging/media/sunxi/cedrus/cedrus.c | 8 +
> > drivers/staging/media/sunxi/cedrus/cedrus.h | 15 +
> > .../staging/media/sunxi/cedrus/cedrus_dec.c | 5 +
> > .../staging/media/sunxi/cedrus/cedrus_hw.c | 1 +
> > .../staging/media/sunxi/cedrus/cedrus_regs.h | 80 ++
> > .../staging/media/sunxi/cedrus/cedrus_video.c | 9 +
> > .../staging/media/sunxi/cedrus/cedrus_vp8.c | 699 ++++++++++++++++++
> > 8 files changed, 819 insertions(+), 1 deletion(-)
> > create mode 100644 drivers/staging/media/sunxi/cedrus/cedrus_vp8.c