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

[Eoff, Ullysses A]

> -----Original Message-----
> From: ffmpeg-devel [mailto:ffmpeg-devel-bounces at ffmpeg.org] On Behalf Of Mark Thompson
> Sent: Friday, June 07, 2019 1:47 PM
> To: ffmpeg-devel at ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH] vaapi_encode_mjpeg: WA: fix bad component id bug
> 
> 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.
> 

Ah yes, that explanation makes sense.

> 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?
> 

Sure.  V2 coming up.

> >      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
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".