Thread (24 messages) 24 messages, 6 authors, 2019-02-14

Re: [PATCH v3 0/2] media: cedrus: Add H264 decoding support

From: Tomasz Figa <tfiga@chromium.org>
Date: 2019-02-13 03:02:28
Also in: linux-media, lkml

On Wed, Feb 13, 2019 at 6:22 AM Ezequiel Garcia [off-list ref] wrote:
Hey Tomasz,

On Tue, 2019-02-12 at 21:50 +0900, Tomasz Figa wrote:
quoted
Hi Maxime,

On Mon, Feb 11, 2019 at 11:39 PM Maxime Ripard
[off-list ref] wrote:
quoted
Hi,

Here is a new version of the H264 decoding support in the cedrus
driver.
Thanks for working on this. Please see my comments below.
quoted
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/

I've tested the various ABI using this gdb script:
http://code.bulix.org/jl4se4-505620?raw

And this test script:
http://code.bulix.org/8zle4s-505623?raw

The application compiled is quite trivial:
http://code.bulix.org/e34zp8-505624?raw

The output is:
arm:    builds/arm-test-v4l2-h264-structures
        SHA1: 88cbf7485ba81831fc3b93772b215599b3b38318
x86:    builds/x86-test-v4l2-h264-structures
        SHA1: 88cbf7485ba81831fc3b93772b215599b3b38318
x64:    builds/x64-test-v4l2-h264-structures
        SHA1: 88cbf7485ba81831fc3b93772b215599b3b38318
arm64:  builds/arm64-test-v4l2-h264-structures
        SHA1: 88cbf7485ba81831fc3b93772b215599b3b38318

Let me know if there's any flaw using that test setup, or if you have
any comments on the patches.

Maxime

Changes from v2:
  - Simplified _cedrus_write_ref_list as suggested by Jernej
  - Set whether the frame is used as reference using nal_ref_idc
  - Respect chroma_format_idc
  - Fixes for the scaling list and prediction tables
  - Wrote the documentation for the flags
  - Added a bunch of defines to the driver bit fields
  - Reworded the controls and data format descriptions as suggested
    by Hans
  - Reworked the controls' structure field size to avoid padding
  - Removed the long term reference flag
This and...
Maxime has dropped this because of Ayaka's mail about long term references
not making much sense in stateless decoders.
I haven't seen any argument confirming that thesis, though. I should
have kicked in earlier, sorry.
I noticed that RK3399 TRM has a field to specify long term refs and
so was wondering about this item as well.
quoted
quoted
  - Reintroduced the neighbor info buffer
  - Removed the ref_pic_list_p0/b0/b1 arrays that are redundant with the
    one in the DPB
these are used in our Rockchip VDEC driver.

Could you elaborate on the reasons why they got removed?
If I understood correctly, there are two reference picture lists.
P-frames will populate ref_pic_list0 and B-frames will populate both.

According to this, v4l2_ctrl_h264_slice_param.ref_pic_list0 and .ref_pic_list1
should be enough and ref_pic_list_p0/b0/b1 are not needed.

What do you think?
The lists in v4l2_ctrl_h264_slice_param are expected to be past the
per-slice modification stage (which is quite complicated and better
done in userspace), while the ones in v4l2_ctrl_h264_decode_param just
in the original order. Rockchip VPU expects them in the original order
and does the modification in the hardware.

Best regards,
Tomasz

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help