1. Patchew
  2. linux
  3. [PATCH v2 00/18] media: Fix the last set of coccinelle warnings
  4. View patch

[PATCH v2 01/18] media: allegro: nal-hevc: Refactor nal_hevc_sub_layer_hrd_parameters

Ricardo Ribalda posted 18 patches 1 year, 9 months ago
There is a newer version of this series
[PATCH v2 01/18] media: allegro: nal-hevc: Refactor nal_hevc_sub_layer_hrd_parameters
Posted by Ricardo Ribalda 1 year, 9 months ago 
Replace all the single elements arrays with the element itself.

Pahole shows the same padding and alignment for x86 and arm in both
situations.

This fixes this cocci warning:
drivers/media/platform/allegro-dvt/nal-hevc.h:102:14-22: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)

Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
---
 drivers/media/platform/allegro-dvt/allegro-core.c |  6 +++---
 drivers/media/platform/allegro-dvt/nal-hevc.c     | 11 +++--------
 drivers/media/platform/allegro-dvt/nal-hevc.h     |  6 +++---
 3 files changed, 9 insertions(+), 14 deletions(-)

diff --git a/drivers/media/platform/allegro-dvt/allegro-core.c b/drivers/media/platform/allegro-dvt/allegro-core.c
index da61f9beb6b4..369bd88cc0ae 100644
--- a/drivers/media/platform/allegro-dvt/allegro-core.c
+++ b/drivers/media/platform/allegro-dvt/allegro-core.c
@@ -1852,14 +1852,14 @@ static ssize_t allegro_hevc_write_sps(struct allegro_channel *channel,
 	hrd->dpb_output_delay_length_minus1 = 30;
 
 	hrd->bit_rate_scale = ffs(channel->bitrate_peak) - 6;
-	hrd->vcl_hrd[0].bit_rate_value_minus1[0] =
+	hrd->vcl_hrd[0].bit_rate_value_minus1 =
 		(channel->bitrate_peak >> (6 + hrd->bit_rate_scale)) - 1;
 
 	cpb_size = v4l2_ctrl_g_ctrl(channel->mpeg_video_cpb_size) * 1000;
 	hrd->cpb_size_scale = ffs(cpb_size) - 4;
-	hrd->vcl_hrd[0].cpb_size_value_minus1[0] = (cpb_size >> (4 + hrd->cpb_size_scale)) - 1;
+	hrd->vcl_hrd[0].cpb_size_value_minus1 = (cpb_size >> (4 + hrd->cpb_size_scale)) - 1;
 
-	hrd->vcl_hrd[0].cbr_flag[0] = !v4l2_ctrl_g_ctrl(channel->mpeg_video_frame_rc_enable);
+	hrd->vcl_hrd[0].cbr_flag = !v4l2_ctrl_g_ctrl(channel->mpeg_video_frame_rc_enable);
 
 	size = nal_hevc_write_sps(&dev->plat_dev->dev, dest, n, sps);
 
diff --git a/drivers/media/platform/allegro-dvt/nal-hevc.c b/drivers/media/platform/allegro-dvt/nal-hevc.c
index 9cdf2756e0a3..575089522df5 100644
--- a/drivers/media/platform/allegro-dvt/nal-hevc.c
+++ b/drivers/media/platform/allegro-dvt/nal-hevc.c
@@ -210,14 +210,9 @@ static void nal_hevc_rbsp_vps(struct rbsp *rbsp, struct nal_hevc_vps *vps)
 static void nal_hevc_rbsp_sub_layer_hrd_parameters(struct rbsp *rbsp,
 						   struct nal_hevc_sub_layer_hrd_parameters *hrd)
 {
-	unsigned int i;
-	unsigned int cpb_cnt = 1;
-
-	for (i = 0; i < cpb_cnt; i++) {
-		rbsp_uev(rbsp, &hrd->bit_rate_value_minus1[i]);
-		rbsp_uev(rbsp, &hrd->cpb_size_value_minus1[i]);
-		rbsp_bit(rbsp, &hrd->cbr_flag[i]);
-	}
+	rbsp_uev(rbsp, &hrd->bit_rate_value_minus1);
+	rbsp_uev(rbsp, &hrd->cpb_size_value_minus1);
+	rbsp_bit(rbsp, &hrd->cbr_flag);
 }
 
 static void nal_hevc_rbsp_hrd_parameters(struct rbsp *rbsp,
diff --git a/drivers/media/platform/allegro-dvt/nal-hevc.h b/drivers/media/platform/allegro-dvt/nal-hevc.h
index eb46f12aae80..afa7a9d7d654 100644
--- a/drivers/media/platform/allegro-dvt/nal-hevc.h
+++ b/drivers/media/platform/allegro-dvt/nal-hevc.h
@@ -97,9 +97,9 @@ struct nal_hevc_vps {
 };
 
 struct nal_hevc_sub_layer_hrd_parameters {
-	unsigned int bit_rate_value_minus1[1];
-	unsigned int cpb_size_value_minus1[1];
-	unsigned int cbr_flag[1];
+	unsigned int bit_rate_value_minus1;
+	unsigned int cpb_size_value_minus1;
+	unsigned int cbr_flag;
 };
 
 struct nal_hevc_hrd_parameters {

-- 
2.45.0.rc1.225.g2a3ae87e7f-goog
Re: [PATCH v2 01/18] media: allegro: nal-hevc: Refactor nal_hevc_sub_layer_hrd_parameters
Posted by Michael Tretter 1 year, 9 months ago 
On Tue, 07 May 2024 16:27:06 +0000, Ricardo Ribalda wrote:
> Replace all the single elements arrays with the element itself.
> 
> Pahole shows the same padding and alignment for x86 and arm in both
> situations.
> 
> This fixes this cocci warning:
> drivers/media/platform/allegro-dvt/nal-hevc.h:102:14-22: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)

Thanks for the patch.

> 
> Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> ---
>  drivers/media/platform/allegro-dvt/allegro-core.c |  6 +++---
>  drivers/media/platform/allegro-dvt/nal-hevc.c     | 11 +++--------
>  drivers/media/platform/allegro-dvt/nal-hevc.h     |  6 +++---
>  3 files changed, 9 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/media/platform/allegro-dvt/allegro-core.c b/drivers/media/platform/allegro-dvt/allegro-core.c
> index da61f9beb6b4..369bd88cc0ae 100644
> --- a/drivers/media/platform/allegro-dvt/allegro-core.c
> +++ b/drivers/media/platform/allegro-dvt/allegro-core.c
> @@ -1852,14 +1852,14 @@ static ssize_t allegro_hevc_write_sps(struct allegro_channel *channel,
>  	hrd->dpb_output_delay_length_minus1 = 30;
>  
>  	hrd->bit_rate_scale = ffs(channel->bitrate_peak) - 6;
> -	hrd->vcl_hrd[0].bit_rate_value_minus1[0] =
> +	hrd->vcl_hrd[0].bit_rate_value_minus1 =
>  		(channel->bitrate_peak >> (6 + hrd->bit_rate_scale)) - 1;
>  
>  	cpb_size = v4l2_ctrl_g_ctrl(channel->mpeg_video_cpb_size) * 1000;
>  	hrd->cpb_size_scale = ffs(cpb_size) - 4;
> -	hrd->vcl_hrd[0].cpb_size_value_minus1[0] = (cpb_size >> (4 + hrd->cpb_size_scale)) - 1;
> +	hrd->vcl_hrd[0].cpb_size_value_minus1 = (cpb_size >> (4 + hrd->cpb_size_scale)) - 1;
>  
> -	hrd->vcl_hrd[0].cbr_flag[0] = !v4l2_ctrl_g_ctrl(channel->mpeg_video_frame_rc_enable);
> +	hrd->vcl_hrd[0].cbr_flag = !v4l2_ctrl_g_ctrl(channel->mpeg_video_frame_rc_enable);
>  
>  	size = nal_hevc_write_sps(&dev->plat_dev->dev, dest, n, sps);
>  
> diff --git a/drivers/media/platform/allegro-dvt/nal-hevc.c b/drivers/media/platform/allegro-dvt/nal-hevc.c
> index 9cdf2756e0a3..575089522df5 100644
> --- a/drivers/media/platform/allegro-dvt/nal-hevc.c
> +++ b/drivers/media/platform/allegro-dvt/nal-hevc.c
> @@ -210,14 +210,9 @@ static void nal_hevc_rbsp_vps(struct rbsp *rbsp, struct nal_hevc_vps *vps)
>  static void nal_hevc_rbsp_sub_layer_hrd_parameters(struct rbsp *rbsp,
>  						   struct nal_hevc_sub_layer_hrd_parameters *hrd)
>  {
> -	unsigned int i;
> -	unsigned int cpb_cnt = 1;
> -
> -	for (i = 0; i < cpb_cnt; i++) {
> -		rbsp_uev(rbsp, &hrd->bit_rate_value_minus1[i]);
> -		rbsp_uev(rbsp, &hrd->cpb_size_value_minus1[i]);
> -		rbsp_bit(rbsp, &hrd->cbr_flag[i]);
> -	}
> +	rbsp_uev(rbsp, &hrd->bit_rate_value_minus1);
> +	rbsp_uev(rbsp, &hrd->cpb_size_value_minus1);
> +	rbsp_bit(rbsp, &hrd->cbr_flag);
>  }
>  
>  static void nal_hevc_rbsp_hrd_parameters(struct rbsp *rbsp,
> diff --git a/drivers/media/platform/allegro-dvt/nal-hevc.h b/drivers/media/platform/allegro-dvt/nal-hevc.h
> index eb46f12aae80..afa7a9d7d654 100644
> --- a/drivers/media/platform/allegro-dvt/nal-hevc.h
> +++ b/drivers/media/platform/allegro-dvt/nal-hevc.h
> @@ -97,9 +97,9 @@ struct nal_hevc_vps {
>  };
>  
>  struct nal_hevc_sub_layer_hrd_parameters {
> -	unsigned int bit_rate_value_minus1[1];
> -	unsigned int cpb_size_value_minus1[1];
> -	unsigned int cbr_flag[1];
> +	unsigned int bit_rate_value_minus1;
> +	unsigned int cpb_size_value_minus1;
> +	unsigned int cbr_flag;

The struct is modeled after the specification in ITU-T H.265, which
defines the fields as arrays. It's a limitation of the current
implementation that only a single element is supported.

Maybe replacing the hard coded values with a constant would be more
appropriate to document this limitation.

Michael

>  };
>  
>  struct nal_hevc_hrd_parameters {
> 
> -- 
> 2.45.0.rc1.225.g2a3ae87e7f-goog
> 
>
Re: [PATCH v2 01/18] media: allegro: nal-hevc: Refactor nal_hevc_sub_layer_hrd_parameters
Posted by Ricardo Ribalda 1 year, 9 months ago 
Hi Michael

On Wed, 8 May 2024 at 10:53, Michael Tretter <m.tretter@pengutronix.de> wrote:
>
> On Tue, 07 May 2024 16:27:06 +0000, Ricardo Ribalda wrote:
> > Replace all the single elements arrays with the element itself.
> >
> > Pahole shows the same padding and alignment for x86 and arm in both
> > situations.
> >
> > This fixes this cocci warning:
> > drivers/media/platform/allegro-dvt/nal-hevc.h:102:14-22: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
>
> Thanks for the patch.
>
> >
> > Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> > ---
> >  drivers/media/platform/allegro-dvt/allegro-core.c |  6 +++---
> >  drivers/media/platform/allegro-dvt/nal-hevc.c     | 11 +++--------
> >  drivers/media/platform/allegro-dvt/nal-hevc.h     |  6 +++---
> >  3 files changed, 9 insertions(+), 14 deletions(-)
> >
> > diff --git a/drivers/media/platform/allegro-dvt/allegro-core.c b/drivers/media/platform/allegro-dvt/allegro-core.c
> > index da61f9beb6b4..369bd88cc0ae 100644
> > --- a/drivers/media/platform/allegro-dvt/allegro-core.c
> > +++ b/drivers/media/platform/allegro-dvt/allegro-core.c
> > @@ -1852,14 +1852,14 @@ static ssize_t allegro_hevc_write_sps(struct allegro_channel *channel,
> >       hrd->dpb_output_delay_length_minus1 = 30;
> >
> >       hrd->bit_rate_scale = ffs(channel->bitrate_peak) - 6;
> > -     hrd->vcl_hrd[0].bit_rate_value_minus1[0] =
> > +     hrd->vcl_hrd[0].bit_rate_value_minus1 =
> >               (channel->bitrate_peak >> (6 + hrd->bit_rate_scale)) - 1;
> >
> >       cpb_size = v4l2_ctrl_g_ctrl(channel->mpeg_video_cpb_size) * 1000;
> >       hrd->cpb_size_scale = ffs(cpb_size) - 4;
> > -     hrd->vcl_hrd[0].cpb_size_value_minus1[0] = (cpb_size >> (4 + hrd->cpb_size_scale)) - 1;
> > +     hrd->vcl_hrd[0].cpb_size_value_minus1 = (cpb_size >> (4 + hrd->cpb_size_scale)) - 1;
> >
> > -     hrd->vcl_hrd[0].cbr_flag[0] = !v4l2_ctrl_g_ctrl(channel->mpeg_video_frame_rc_enable);
> > +     hrd->vcl_hrd[0].cbr_flag = !v4l2_ctrl_g_ctrl(channel->mpeg_video_frame_rc_enable);
> >
> >       size = nal_hevc_write_sps(&dev->plat_dev->dev, dest, n, sps);
> >
> > diff --git a/drivers/media/platform/allegro-dvt/nal-hevc.c b/drivers/media/platform/allegro-dvt/nal-hevc.c
> > index 9cdf2756e0a3..575089522df5 100644
> > --- a/drivers/media/platform/allegro-dvt/nal-hevc.c
> > +++ b/drivers/media/platform/allegro-dvt/nal-hevc.c
> > @@ -210,14 +210,9 @@ static void nal_hevc_rbsp_vps(struct rbsp *rbsp, struct nal_hevc_vps *vps)
> >  static void nal_hevc_rbsp_sub_layer_hrd_parameters(struct rbsp *rbsp,
> >                                                  struct nal_hevc_sub_layer_hrd_parameters *hrd)
> >  {
> > -     unsigned int i;
> > -     unsigned int cpb_cnt = 1;
> > -
> > -     for (i = 0; i < cpb_cnt; i++) {
> > -             rbsp_uev(rbsp, &hrd->bit_rate_value_minus1[i]);
> > -             rbsp_uev(rbsp, &hrd->cpb_size_value_minus1[i]);
> > -             rbsp_bit(rbsp, &hrd->cbr_flag[i]);
> > -     }
> > +     rbsp_uev(rbsp, &hrd->bit_rate_value_minus1);
> > +     rbsp_uev(rbsp, &hrd->cpb_size_value_minus1);
> > +     rbsp_bit(rbsp, &hrd->cbr_flag);
> >  }
> >
> >  static void nal_hevc_rbsp_hrd_parameters(struct rbsp *rbsp,
> > diff --git a/drivers/media/platform/allegro-dvt/nal-hevc.h b/drivers/media/platform/allegro-dvt/nal-hevc.h
> > index eb46f12aae80..afa7a9d7d654 100644
> > --- a/drivers/media/platform/allegro-dvt/nal-hevc.h
> > +++ b/drivers/media/platform/allegro-dvt/nal-hevc.h
> > @@ -97,9 +97,9 @@ struct nal_hevc_vps {
> >  };
> >
> >  struct nal_hevc_sub_layer_hrd_parameters {
> > -     unsigned int bit_rate_value_minus1[1];
> > -     unsigned int cpb_size_value_minus1[1];
> > -     unsigned int cbr_flag[1];
> > +     unsigned int bit_rate_value_minus1;
> > +     unsigned int cpb_size_value_minus1;
> > +     unsigned int cbr_flag;
>
> The struct is modeled after the specification in ITU-T H.265, which
> defines the fields as arrays. It's a limitation of the current
> implementation that only a single element is supported.
>
> Maybe replacing the hard coded values with a constant would be more
> appropriate to document this limitation.

A define seems to convince coccinelle of our intentions :). I will
upload the fix in v3

diff --git a/drivers/media/platform/allegro-dvt/nal-hevc.h
b/drivers/media/platform/allegro-dvt/nal-hevc.h
index eb46f12aae80..361e2f55c254 100644
--- a/drivers/media/platform/allegro-dvt/nal-hevc.h
+++ b/drivers/media/platform/allegro-dvt/nal-hevc.h
@@ -96,10 +96,11 @@ struct nal_hevc_vps {
        unsigned int extension_data_flag;
 };

+#define N_HRD_PARAMS 1
 struct nal_hevc_sub_layer_hrd_parameters {
-       unsigned int bit_rate_value_minus1[1];
-       unsigned int cpb_size_value_minus1[1];
-       unsigned int cbr_flag[1];
+       unsigned int bit_rate_value_minus1[N_HRD_PARAMS];
+       unsigned int cpb_size_value_minus1[N_HRD_PARAMS];
+       unsigned int cbr_flag[N_HRD_PARAMS];
 };

 struct nal_hevc_hrd_parameters {


Thanks.


>
> Michael
>
> >  };
> >
> >  struct nal_hevc_hrd_parameters {
> >
> > --
> > 2.45.0.rc1.225.g2a3ae87e7f-goog
> >
> >



--
Ricardo Ribalda

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.