[FFmpeg-devel,v2] vaapi_encode_mjpeg: fix bad component id bug

Submitted by U. Artie Eoff on June 7, 2019, 9:11 p.m.

Details

Message ID 20190607211119.19466-1-ullysses.a.eoff@intel.com
State Accepted
Commit f70c397456c7eba78a22e9318bce634e1d9079d6
Headers show

Commit Message

U. Artie Eoff June 7, 2019, 9:11 p.m.
When compile time optimizations are enabled and
compiling with GNU GCC 9.x, the pointer assignment
to the inline brace-enclosed list initialized
array does not work and "component" ends up pointing
to an empty/undefined array.  Thus, all of the
frame header component id's get assigned the
same value of 0.

As Mark Thompson described, this is caused by
"...the compound literals only exist in the tiny
scope of the if/else branches..." which strangely
is not producing a compiler warning.

This patch fixes this issue by assigning the constant
arrays to local variables at function scope and then
pointing "component" to those as necessary.

Fixes #7915

Signed-off-by: U. Artie Eoff <ullysses.a.eoff@intel.com>
---
 libavcodec/vaapi_encode_mjpeg.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

Comments

U. Artie Eoff June 7, 2019, 9:35 p.m.
> -----Original Message-----
> From: Eoff, Ullysses A
> Sent: Friday, June 07, 2019 2:11 PM
> To: ffmpeg-devel@ffmpeg.org
> Cc: Eoff, Ullysses A <ullysses.a.eoff@intel.com>
> Subject: [PATCH v2] vaapi_encode_mjpeg: fix bad component id bug
> 
> When compile time optimizations are enabled and
> compiling with GNU GCC 9.x, the pointer assignment
> to the inline brace-enclosed list initialized
> array does not work and "component" ends up pointing
> to an empty/undefined array.  Thus, all of the
> frame header component id's get assigned the
> same value of 0.
> 
> As Mark Thompson described, this is caused by
> "...the compound literals only exist in the tiny
> scope of the if/else branches..." which strangely
> is not producing a compiler warning.
> 
> This patch fixes this issue by assigning the constant
> arrays to local variables at function scope and then
> pointing "component" to those as necessary.
> 
> Fixes #7915
> 

Hmmm... now I don't like this commit message.  Let me revise in a V3 ;)

> Signed-off-by: U. Artie Eoff <ullysses.a.eoff@intel.com>
> ---
>  libavcodec/vaapi_encode_mjpeg.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/libavcodec/vaapi_encode_mjpeg.c b/libavcodec/vaapi_encode_mjpeg.c
> index 4dcdc3d16bb0..bd029cc90315 100644
> --- a/libavcodec/vaapi_encode_mjpeg.c
> +++ b/libavcodec/vaapi_encode_mjpeg.c
> @@ -227,6 +227,8 @@ static int vaapi_encode_mjpeg_init_picture_params(AVCodecContext *avctx,
>      JPEGRawScanHeader                 *sh = &priv->scan.header;
>      VAEncPictureParameterBufferJPEG *vpic = pic->codec_picture_params;
>      const AVPixFmtDescriptor *desc;
> +    const uint8_t components_rgb[3] = { 'R', 'G', 'B' };
> +    const uint8_t components_yuv[3] = {  1,   2,   3  };
>      const uint8_t *components;
>      int t, i, quant_scale, len;
> 
> @@ -235,9 +237,9 @@ static int vaapi_encode_mjpeg_init_picture_params(AVCodecContext *avctx,
>      desc = av_pix_fmt_desc_get(priv->common.input_frames->sw_format);
>      av_assert0(desc);
>      if (desc->flags & AV_PIX_FMT_FLAG_RGB)
> -        components = (uint8_t[3]) { 'R', 'G', 'B' };
> +        components = components_rgb;
>      else
> -        components = (uint8_t[3]) {  1,   2,   3  };
> +        components = components_yuv;
> 
>      // Frame header.
> 
> --
> 2.20.1

Patch hide | download patch | download mbox

diff --git a/libavcodec/vaapi_encode_mjpeg.c b/libavcodec/vaapi_encode_mjpeg.c
index 4dcdc3d16bb0..bd029cc90315 100644
--- a/libavcodec/vaapi_encode_mjpeg.c
+++ b/libavcodec/vaapi_encode_mjpeg.c
@@ -227,6 +227,8 @@  static int vaapi_encode_mjpeg_init_picture_params(AVCodecContext *avctx,
     JPEGRawScanHeader                 *sh = &priv->scan.header;
     VAEncPictureParameterBufferJPEG *vpic = pic->codec_picture_params;
     const AVPixFmtDescriptor *desc;
+    const uint8_t components_rgb[3] = { 'R', 'G', 'B' };
+    const uint8_t components_yuv[3] = {  1,   2,   3  };
     const uint8_t *components;
     int t, i, quant_scale, len;
 
@@ -235,9 +237,9 @@  static int vaapi_encode_mjpeg_init_picture_params(AVCodecContext *avctx,
     desc = av_pix_fmt_desc_get(priv->common.input_frames->sw_format);
     av_assert0(desc);
     if (desc->flags & AV_PIX_FMT_FLAG_RGB)
-        components = (uint8_t[3]) { 'R', 'G', 'B' };
+        components = components_rgb;
     else
-        components = (uint8_t[3]) {  1,   2,   3  };
+        components = components_yuv;
 
     // Frame header.