Le 13/02/2022 à 12:33, Jernej Škrabec a écrit :
Hi Benjamin,
CC: Alex, John
Sorry for late response, but I've been very busy last week.
First of all, thank you for doing this! It's about time that HEVC moves
forward.
Dne torek, 01. februar 2022 ob 13:34:39 CET je Benjamin Gaignard napisal(a):
The HEVC stateless 'uAPI' was staging and marked explicitly in the
V4L2 specification that it will change and is unstable.
Note that these control IDs were never exported as a public API,
they were only defined in kernel-local headers (hevc-ctrls.h).
While moving the controls out of staging they are renamed and
control IDs get new numbers.
Drivers (Hantro, Cedrus) and Documentation are updated accordaly.
accordaly -> accordingly
Additional structures fields has been added for RKVDEC driver usage.
You should do separate patch for that, preceding this one. One patch should
only do one thing.
I will do that in v2
I also suggest that you add additional patch for removing bit_size field in
struct v4l2_ctrl_hevc_slice_params. Similar fields were already removed from
MPEG2 and H264 structures. Bit size can be deduced from output buffer size and
it doesn't hurt if bit size in Cedrus is set to bigger value than actual slice
bit size.
ok
Hantro dedicated control is moving to hantro-media.h
Since hevc-ctrls.h content has been dispatched in others file, remove it.
fluster tests results on IMX8MQ is 77/147 for HEVC codec.
Signed-off-by: Benjamin Gaignard <benjamin.gaignard@xxxxxxxxxxxxx>
Note that Cedrus still needs additional information in order to decode some
HEVC videos. Missing info is num_entry_point_offsets and list of all
entry_point_offset_minus1 (obviously, num_entry_point_offsets in size).
I suggest that this is represented in a new control, which would use dynamic
array feature, written by Hans. While Cedrus supports max. 256 entries, it can
be much bigger in theory, but in reality, it's much smaller (like 4-8
entries).
I haven't seen yet any user for these fields but I will create a new control like
#define V4L2_CID_STATELESS_HEVC_ENTRY_POINT (V4L2_CID_CODEC_STATELESS_BASE + 407)
struct v4l2_ctrl_hevc_entry_point_offset {
__u32 entry_point_offset_minus1;
};
and add it in the documentation:
``V4L2_CID_STATELESS_HEVC_ENTRY_POINT (struct)``
Specifies the i-th entry point offset in bytes and is represented by
offset_len_minus1 plus 1 bits.
This control is a dynamically sized array. The number of entry point
offsets is reported by the ``elems`` field.
This bitstream parameter is defined according to :ref:`hevc`.
They are described in section 7.4.7.1 "General slice segment header
semantics" of the specification.
Last but not least, data_bit_offset should be better defined. Currently it
points right after last header bit, just like Cedrus needs it. However, there
is padding after that, at least 1 bit and 8 bits at most, so slice data always
starts from byte aligned address. It probably make sense to rework that field
to be byte offset, not bit, just like in VA-API. Note that RPi HEVC driver also
uses byte aligned address directly. Cedrus would need some kind of workaround
and only one that works is this one:
https://github.com/bootlin/libva-v4l2-request/blob/master/src/h265.c#L191-L209
If Cedrus driver is happy with this definition I will keep it like that.
When providing offset in bit is more accurate and any driver can align the value
if needed, the reverse (byte -> bit) isn't possible.
Regards,
Benjamin
Best regards,
Jernej
