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

[Eoff, Ullysses A]

> -----Original Message-----
> From: ffmpeg-devel [mailto:ffmpeg-devel-bounces at ffmpeg.org] On Behalf Of Reimar Döffinger
> Sent: Friday, June 07, 2019 11:07 PM
> To: FFmpeg development discussions and patches <ffmpeg-devel at ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH v4] vaapi_encode_mjpeg: fix bad component id bug
> 
> On 07.06.2019, at 23:45, "U. Artie Eoff" <ullysses.a.eoff at intel.com> wrote:
> 
> > The compound literals assigned to "components"
> > only exist within the scope of the if/else
> > block (thanks Mark Thompson for the better
> > explanation).
> >
> > Thus, after this if/else block, "components"
> > ends up pointing to an arbitrary/undefined
> > array.  With some compilers and depending on
> > optimization settings, these arbitrary values
> > may end up being the same value (i.e. 0 with
> > GNU GCC 9.x).  Unfortunately, the GNU GCC
> > compiler, at least, never prints any warnings
> > about this.
> >
> > This patch fixes this issue by assigning the
> > constant arrays to local variables at function
> > scope and then pointing "components" 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(-)
> >
> > 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  };
> 
> Not sure what the necessary scope is for these, but maybe safest to go for "static const"?

I don't think these need scope beyond this function since the values are just
"copied" into the frame header here.  So static would just be an [barely detectable]
optimization.  Unless I am missing something?

> _______________________________________________
> 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".