Thread (14 messages) 14 messages, 3 authors, 2021-11-10

Re: [PATCH v2 0/4] media: HEVC: RPS clean up

From: Benjamin Gaignard <benjamin.gaignard@collabora.com>
Date: 2021-10-15 14:53:27
Also in: linux-media, linux-rockchip, linux-staging, linux-sunxi, lkml

Le 15/10/2021 à 16:14, Alex Bee a écrit :
Am 15.10.21 um 16:06 schrieb Benjamin Gaignard:
quoted
Le 15/10/2021 à 12:33, Alex Bee a écrit :
quoted
Hi Benjamin, Jernej
Am 12.10.21 um 18:08 schrieb Jernej Škrabec:
quoted
CC: Alex Bee

Alex, please take a look to these patches too.
These patches don't remove anything that would be need for rkvdec 
hevc - but indeed - we need some more:
https://github.com/LibreELEC/LibreELEC.tv/blob/master/projects/Rockchip/patches/linux/default/linux-2001-v4l-wip-rkvdec-hevc.patch#L242-L305 


v4l2_ctrl_hevc_sps:
__u8    video_parameter_set_id
__u8    seq_parameter_set_id

v4l2_ctrl_hevc_pps:
__u8    pic_parameter_set_id
__u16    short_term_ref_pic_set_size
__u16    long_term_ref_pic_set_size

As far as I can see, they are all part of the spec and should be 
therefore good to go in the uapi.
Do you have any plan to upstream these fields in HEVC uAPI ?
I might be upstreaming them at some point, yes.

With this I just wanted to underline Jernej said: HEVC uapi is NOT ready
to get unstaged yet.
Ok but the question is how to get it unstaged ?
Should we continue to do changes in staged uAPI ?
or send a RFC to move it to stable and review all the missing parts at that time ?

Regards,
Benjamin
quoted
Regards,
Benjamin
quoted
As you might now, even rkvdec is a frame-based decoder, it doesn't 
fully parse slice headers in HW for HEVC and we need to set 
references in SW which requires looping over the slices. Downstream 
we have a hack to give num_slices in v4l2_ctrl_hevc_sps for doing that.
That could fully go away, if V4L2_CID_MPEG_VIDEO_HEVC_SLICE_PARAMS 
could get dynamic array control support and would make upstreaming 
this a lot easier - as far as I'm concered this would be required 
for RPi HEVC decoder as well.
As a last resort we could also implement a HW specifc control à la
V4L2_CID_HANTRO_HEVC_SLICE_HEADER_SKIP - but I'd like to avoid that, 
knowing it would certainly be better from performance pov.

Alex.
quoted
Dne torek, 12. oktober 2021 ob 17:57:50 CEST je Benjamin Gaignard 
napisal(a):
quoted
Le 12/10/2021 à 17:34, Jernej Škrabec a écrit :
quoted
Hi Benjamin!

Dne torek, 12. oktober 2021 ob 16:35:48 CEST je Benjamin Gaignard
napisal(a):
quoted
quoted
quoted
This series aims to clean up Reference Picture Set usage and flags.

Long term flag was named with RPS prefix while it is not used 
for RPS
but for mark long term references in DBP. Remane it and remove 
the two
other useless RPS flags.

Clarify documentation about RPS lists content and make sure that 
Hantro
driver use them correctly (i.e without look up in DBP).

These patches are the last in my backlog impacting HEVC uAPI.
  From my point of view, once they get merged, you could start 
talking
about how move HEVC uAPI to stable.
With your changes, HEVC uAPI controls still won't be complete. 
Cedrus
needs
quoted
quoted
entry point control, which in turn needs dynamic array support. 
I'm a bit
lazy
quoted
quoted
implementing that control, but I guess I can take a look in a 
month or so.
rkvdec also needs more fields for HEVC. With patches collected here:
https://github.com/LibreELEC/LibreELEC.tv/blob/master/projects/Rockchip/ 

patches/linux/default/linux-2001-v4l-wip-rkvdec-hevc.patch
fluster HEVC test score is reportedly 121/135 (8-bit tests only).
Hi Jernej,

Thanks for your feedback, getting a list of missing items in HEVC 
uAPI
will definitively help to fill the hope.
The patch you mention for rkvdec are already merged in mainline 
kernel (at
least for uAPI part).
Are they? What about:
video_parameter_set_id
seq_parameter_set_id
pic_parameter_set_id
short_term_ref_pic_set_size
long_term_ref_pic_set_size

At least I don't see them in linux-next. Maybe that information can be
obtained in some other way?
quoted
Cedrus needs are about num_entry_point_offsets, offset_len_minus1 and
entry_point_offset_minus1[ i ]
quoted
in HEVC specifications ?
Yes, Cedrus needs to know whole list of entry points. I don't think 
we need to
worry about offset_len_minus1, list could be pre-processed - just 
number of
entry points and their values.

Best regards,
Jernej
quoted
Regards,
Benjamin
quoted
I would certainly wait with moving HEVC uAPI to stable.

Best regards,
Jernej
quoted
version 2:
- change DPB field name from rps to flags

Please note that the only purpose of commits 3 and 4 is to allow 
to test
G2 hardware block for IMX8MQ until a proper solution isuing 
power domain
can be found. Do not merge them.

GStreamer HEVC plugin merge request can be found here:
https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/1079 


With those piece of code fluster score is 77/147.

Benjamin

Benjamin Gaignard (4):
    media: hevc: Remove RPS named flags
    media: hevc: Embedded indexes in RPS
    media: hantro: Use syscon instead of 'ctrl' register
    arm64: dts: imx8mq: Add node to G2 hardware

   .../media/v4l/ext-ctrls-codec.rst             | 14 +++---
   arch/arm64/boot/dts/freescale/imx8mq.dtsi     | 43 
+++++++++++++----
   drivers/staging/media/hantro/hantro.h         |  5 +-
   .../staging/media/hantro/hantro_g2_hevc_dec.c | 27 +++--------
   drivers/staging/media/hantro/imx8m_vpu_hw.c   | 48 
++++++++++++-------
   .../staging/media/sunxi/cedrus/cedrus_h265.c  |  2 +-
   include/media/hevc-ctrls.h                    |  6 +--
   7 files changed, 84 insertions(+), 61 deletions(-)

-- 
2.30.2
_______________________________________________
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