From: "Jernej Škrabec" <jernej.skrabec@siol.net>
To: mripard@kernel.org, paul.kocialkowski@bootlin.com,
Nicolas Dufresne <nicolas@ndufresne.ca>
Cc: devel@driverdev.osuosl.org, gregkh@linuxfoundation.org,
linux-kernel@vger.kernel.org, wens@csie.org,
hverkuil-cisco@xs4all.nl, mchehab@kernel.org,
linux-arm-kernel@lists.infradead.org,
linux-media@vger.kernel.org
Subject: Re: [PATCH] media: cedrus: Add support for VP8 decoding
Date: Thu, 21 May 2020 00:30:56 +0200 [thread overview]
Message-ID: <2875977.BS6FNRR2HQ@jernej-laptop> (raw)
In-Reply-To: <ee0aa12fdf1655c4e563b8fc9753a5ab5e52f4cf.camel@ndufresne.ca>
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@siol.net>
> > ---
> >
> > 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
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
prev parent reply other threads:[~2020-05-20 22:31 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-05-20 21:01 [PATCH] media: cedrus: Add support for VP8 decoding Jernej Skrabec
2020-05-20 21:43 ` Nicolas Dufresne
2020-05-20 22:30 ` Jernej Škrabec [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=2875977.BS6FNRR2HQ@jernej-laptop \
--to=jernej.skrabec@siol.net \
--cc=devel@driverdev.osuosl.org \
--cc=gregkh@linuxfoundation.org \
--cc=hverkuil-cisco@xs4all.nl \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-media@vger.kernel.org \
--cc=mchehab@kernel.org \
--cc=mripard@kernel.org \
--cc=nicolas@ndufresne.ca \
--cc=paul.kocialkowski@bootlin.com \
--cc=wens@csie.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).