Re: [PATCH] media: hevc: Fix dependent slice segment flags

[John Cox]

>Dependent slice segment flag for PPS control is misnamed. It should have
>"enabled" at the end. It only tells if this flag is present in slice
>header or not and not the actual value.
>
>Fix this by renaming the PPS flag and introduce another flag for slice
>control which tells actual value.
>
>Signed-off-by: Jernej Skrabec <jernej.skrabec@xxxxxxxx>
>---
> Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst | 5 ++++-
> drivers/staging/media/sunxi/cedrus/cedrus_h265.c          | 4 ++--
> include/media/hevc-ctrls.h                                | 3 ++-
> 3 files changed, 8 insertions(+), 4 deletions(-)
>
>diff --git a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
>index 7b90cb939e9d..5ed343ddd1ea 100644
>--- a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
>+++ b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
>@@ -3059,7 +3059,7 @@ enum v4l2_mpeg_video_hevc_size_of_length_field -
>     :stub-columns: 0
>     :widths:       1 1 2
> 
>-    * - ``V4L2_HEVC_PPS_FLAG_DEPENDENT_SLICE_SEGMENT``
>+    * - ``V4L2_HEVC_PPS_FLAG_DEPENDENT_SLICE_SEGMENT_ENABLED``
>       - 0x00000001
>       -
>     * - ``V4L2_HEVC_PPS_FLAG_OUTPUT_FLAG_PRESENT``
>@@ -3274,6 +3274,9 @@ enum v4l2_mpeg_video_hevc_size_of_length_field -
>     * - ``V4L2_HEVC_SLICE_PARAMS_FLAG_SLICE_LOOP_FILTER_ACROSS_SLICES_ENABLED``
>       - 0x00000100
>       -
>+    * - ``V4L2_HEVC_SLICE_PARAMS_FLAG_DEPENDENT_SLICE_SEGMENT``
>+      - 0x00000200
>+      -
> 
> .. raw:: latex
> 
>diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_h265.c b/drivers/staging/media/sunxi/cedrus/cedrus_h265.c
>index 397a4ba5df4c..6821e3d05d34 100644
>--- a/drivers/staging/media/sunxi/cedrus/cedrus_h265.c
>+++ b/drivers/staging/media/sunxi/cedrus/cedrus_h265.c
>@@ -479,8 +479,8 @@ static void cedrus_h265_setup(struct cedrus_ctx *ctx,
> 				slice_params->flags);
> 
> 	reg |= VE_DEC_H265_FLAG(VE_DEC_H265_DEC_SLICE_HDR_INFO0_FLAG_DEPENDENT_SLICE_SEGMENT,
>-				V4L2_HEVC_PPS_FLAG_DEPENDENT_SLICE_SEGMENT,
>-				pps->flags);
>+				V4L2_HEVC_SLICE_PARAMS_FLAG_DEPENDENT_SLICE_SEGMENT,
>+				slice_params->flags);
> 
> 	/* FIXME: For multi-slice support. */
> 	reg |= VE_DEC_H265_DEC_SLICE_HDR_INFO0_FLAG_FIRST_SLICE_SEGMENT_IN_PIC;
>diff --git a/include/media/hevc-ctrls.h b/include/media/hevc-ctrls.h
>index b713eeed1915..dc964ff7cd29 100644
>--- a/include/media/hevc-ctrls.h
>+++ b/include/media/hevc-ctrls.h
>@@ -83,7 +83,7 @@ struct v4l2_ctrl_hevc_sps {
> 	__u64	flags;
> };
> 
>-#define V4L2_HEVC_PPS_FLAG_DEPENDENT_SLICE_SEGMENT		(1ULL << 0)
>+#define V4L2_HEVC_PPS_FLAG_DEPENDENT_SLICE_SEGMENT_ENABLED	(1ULL << 0)
> #define V4L2_HEVC_PPS_FLAG_OUTPUT_FLAG_PRESENT			(1ULL << 1)
> #define V4L2_HEVC_PPS_FLAG_SIGN_DATA_HIDING_ENABLED		(1ULL << 2)
> #define V4L2_HEVC_PPS_FLAG_CABAC_INIT_PRESENT			(1ULL << 3)
>@@ -166,6 +166,7 @@ struct v4l2_hevc_pred_weight_table {
> #define V4L2_HEVC_SLICE_PARAMS_FLAG_USE_INTEGER_MV		(1ULL << 6)
> #define V4L2_HEVC_SLICE_PARAMS_FLAG_SLICE_DEBLOCKING_FILTER_DISABLED (1ULL << 7)
> #define V4L2_HEVC_SLICE_PARAMS_FLAG_SLICE_LOOP_FILTER_ACROSS_SLICES_ENABLED (1ULL << 8)
>+#define V4L2_HEVC_SLICE_PARAMS_FLAG_DEPENDENT_SLICE_SEGMENT	(1ULL << 9)
> 
> struct v4l2_ctrl_hevc_slice_params {
> 	__u32	bit_size;

Happy to see this flag finally making it

Many thanks

John Cox