Le 28/02/2022 à 10:47, John Cox a écrit :
On Sat, 26 Feb 2022 19:17:46 +0100, you wrote:
Dne sobota, 26. februar 2022 ob 18:59:18 CET je Jernej Škrabec napisal(a):
Hi!
Dne petek, 25. februar 2022 ob 20:30:20 CET je Jernej Škrabec napisal(a):
Hi!
Dne petek, 25. februar 2022 ob 17:45:55 CET je Benjamin Gaignard
napisal(a):
The number of 'entry point offset' could be very variable.
Rather than use a large static array define a v4l2 dynamic array
of integer control.
I suggest we should be more specific and say U32 (V4L2_CTRL_TYPE_U32).
The number of entry point offsets is reported by the elems field.
I did few more tests and these are my findings:
1. dynamic array can't be set over size, specified in .dims array
2. entry point offsets are per slice, so if we make slices dynamic array,
then
entry points would become two dimensional dynamic array
3. num_entry_point_offsets must be part of slice control, because it can be
zero, but size of array can't be
Clarification: num_entry_point_offsets is needed in any case, even without entry
point list.
I thought that at least one decoder wanted the entry points or have we
decided that we don't? Whilst they aren't required for decode, what they
give you, if your h/w supports it, is the ability to decode multiple
lines of CTBs in parallel (with a 2 CTB offset between lines), so they
aren't useless.
If we do decided we want the entry point offset control, now or later,
then num_entry_points in the slice_param allows you to determine which
offsets apply to which slices without the complexity of a variable array
of variable arrays.
But whether or not we end up supporting entry point offsets, the Pi h/w
requires me to generate 1 table entry for each CTB line and
num_entry_points is the parameter I need for that, so I really want it
in the slice_params.
I'm adding a control (dynamic array) and num_entry_points field in slice
parameters in the next version.
See here:
https://gitlab.collabora.com/benjamin.gaignard/for-upstream/-/commit/6b4efa4dc54de069893f5203d885fea2c1347916
Regards,
Benjamin
Regards
John Cox
4. fortunately, not setting entry points doesn't impact decoding
correctness.
This is in line what John told me about them.
Hans, can you comment points 1-3? I might misunderstand point 1.
In short, it seems like we don't really need entry points, even if they are
used in BSP library. In both cases, I got fluster score 119/138 (10-bit
excluded), so we can just drop this patch, although I'm a bit uneasy not
setting entry points...
Best regards,
Jernej
Signed-off-by: Benjamin Gaignard <benjamin.gaignard@xxxxxxxxxxxxx>
---
.../userspace-api/media/v4l/ext-ctrls-codec.rst | 9 +++++++++
include/media/hevc-ctrls.h | 1 +
2 files changed, 10 insertions(+)
diff --git a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst b/
Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
index 44a268a948c0..71f7dc1c1ccd 100644
--- a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
+++ b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
@@ -3128,6 +3128,15 @@ enum v4l2_mpeg_video_hevc_size_of_length_field -
\normalsize
+``V4L2_CID_STATELESS_HEVC_ENTRY_POINT_OFFSETS (integer)``
Here you have OFFSETS (plural) ...
+ Specifies the i-th entry point offset in bytes and is represented by
+ offset_len_minus1 plus 1 bits.
You probably mean entry_point_offset_minus1? offset_len_minus1 just tells
how
much bits need to be read for each element and it's not important for
actual
decoding.
+ 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.
+
``V4L2_CID_STATELESS_HEVC_SCALING_MATRIX (struct)``
Specifies the HEVC scaling matrix parameters used for the scaling
process
for transform coefficients.
diff --git a/include/media/hevc-ctrls.h b/include/media/hevc-ctrls.h
index 3016c1abb1d0..3f8a67924df3 100644
--- a/include/media/hevc-ctrls.h
+++ b/include/media/hevc-ctrls.h
@@ -20,6 +20,7 @@
#define V4L2_CID_STATELESS_HEVC_DECODE_PARAMS (V4L2_CID_CODEC_BASE +
1012)
#define V4L2_CID_STATELESS_HEVC_DECODE_MODE (V4L2_CID_CODEC_BASE +
1015)
#define V4L2_CID_STATELESS_HEVC_START_CODE (V4L2_CID_CODEC_BASE
+
1016)
+#define V4L2_CID_STATELESS_HEVC_ENTRY_POINT_OFFSET (V4L2_CID_CODEC_BASE
+
1017)
... and here you have OFFSET (singlular). I suggest plural form to be used
in
all places, including subject line of this commit.
Additionally, it would be nice if control is initialized, like so:
https://github.com/jernejsk/linux-1/commit/
f938e162cd8dd77c9f6f1b248d80144840a37bce
Best regards,
Jernej
/* enum v4l2_ctrl_type type values */
#define V4L2_CTRL_TYPE_HEVC_SPS 0x0120
--
2.32.0
