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

[Ezequiel Garcia <ezequiel@collabora.com>]

Hi Jernej, Emmanuel,

Thanks for the patch.

On Tue, 2020-11-10 at 23:35 +0100, Jernej Skrabec wrote:
> 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
> 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>
> [addressed issues from reviewer]
> Signed-off-by: Emmanuel Gil Peyrot <linkmauve@linkmauve.fr>
> ---
> Changes in v3:
> - addressed comments from Ezequiel Garcia - new comments,
>   using new macros from VP8 UAPI, new function for waiting
>   on bit to be set
> Changes in v2:
> - rebased on top of current linux-media master branch
> 
> NOTE: This now depends on following patch:
> https://patchwork.linuxtv.org/project/linux-media/patch/20201108202021.4187-1-linkmauve@linkmauve.fr/
> 

The patch looks fairly good, so let's wait and see
what Hans, Paul and Maxime think about it.

FWIW, my humble Reviewed-by: Ezequiel Garcia <ezequiel@collabora.com>

It would be good to make sure this doesn't regress
v4l2-compliance, or cause some regression in decoding.

Not really a blocker to merge this, but I'm thinking
that now that we have Fluster for conformance testing,
we could add the VP8 vectors and use them against
Cedrus and Hantro:

https://chromium.googlesource.com/webm/vp8-test-vectors/+/refs/heads/master

Thanks,
Ezequiel

>  drivers/staging/media/sunxi/cedrus/Makefile   |   3 +-
>  drivers/staging/media/sunxi/cedrus/cedrus.c   |   8 +
>  drivers/staging/media/sunxi/cedrus/cedrus.h   |  24 +
>  .../staging/media/sunxi/cedrus/cedrus_dec.c   |   5 +
>  .../staging/media/sunxi/cedrus/cedrus_hw.c    |   2 +
>  .../staging/media/sunxi/cedrus/cedrus_regs.h  |  80 ++
>  .../staging/media/sunxi/cedrus/cedrus_video.c |   9 +
>  .../staging/media/sunxi/cedrus/cedrus_vp8.c   | 907 ++++++++++++++++++
>  8 files changed, 1037 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/staging/media/sunxi/cedrus/cedrus_vp8.c
> 


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel