[FFmpeg-devel] [PATCH] vaapi_encode_mjpeg: WA: fix bad component id bug

[Mark Thompson]

On 07/06/2019 21:28, U. Artie Eoff wrote:
> 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 array.  This is probably a bug in GCC
> 9.x.
> 
> This patch works around this issue by assigning
> the constant arrays to local variables and then
> pointing "component" to those as necessary.
> 
> Fixes #7915
> 
> Signed-off-by: U. Artie Eoff <ullysses.a.eoff at intel.com>
> ---
>  libavcodec/vaapi_encode_mjpeg.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)

Huh.  No, the compiler is right - the compound literals only exist in the tiny scope of the branches of the if.  I'm somewhat disappointed it isn't warning you about those variables having worked out enough to eliminate them, though - it clearly knows that it's going to generate something undefined.

Good catch, in any case!

> diff --git a/libavcodec/vaapi_encode_mjpeg.c b/libavcodec/vaapi_encode_mjpeg.c
> index 4dcdc3d16bb0..3e8cbb7c9bf9 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_123[3] = {  1,   2,   3  };

It would probably make more sense to call this components_yuv?

>      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_123;
>  
>      // Frame header.
>  
> 

Thanks,

- Mark