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):

quoted

Hi!

Dne petek, 25. februar 2022 ob 17:45:55 CET je Benjamin Gaignard 

napisal(a):

quoted

quoted

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).

quoted

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.

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

quoted

quoted

Signed-off-by: Benjamin Gaignard <benjamin.gaignard@collabora.com>
---
 .../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

quoted

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) ...

quoted

+    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 

quoted

much bits need to be read for each element and it's not important for 

actual 

quoted

decoding.

quoted

+    This control is a dynamically sized array. The number of entry 

point

quoted

quoted

+    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

quoted

     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)

quoted

 #define V4L2_CID_STATELESS_HEVC_DECODE_MODE	(V4L2_CID_CODEC_BASE + 

1015)

quoted

 #define V4L2_CID_STATELESS_HEVC_START_CODE	(V4L2_CID_CODEC_BASE 

+ 

1016)

quoted

quoted

+#define V4L2_CID_STATELESS_HEVC_ENTRY_POINT_OFFSET (V4L2_CID_CODEC_BASE 

+ 

quoted

1017)

... and here you have OFFSET (singlular). I suggest plural form to be used 

in 

quoted

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

quoted

 
 /* enum v4l2_ctrl_type type values */
 #define V4L2_CTRL_TYPE_HEVC_SPS 0x0120
-- 
2.32.0

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