| Message ID | 20190607202818.25099-1-ullysses.a.eoff@intel.com |
|---|---|
| State | New |
| Headers | show |
On 07.06.2019, at 22:28, "U. Artie Eoff" <ullysses.a.eoff@intel.com> 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 to me sounds like a serious enough bug that I'm tempted to suggest blacklisting gcc 9 instead... However the original code doesn't seem really proper to me. Have you tried replacing (uint8_t[3]) by (const uint8_t[3])?
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@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
> -----Original Message----- > From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On Behalf Of Reimar Döffinger > Sent: Friday, June 07, 2019 1:41 PM > To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org> > Subject: Re: [FFmpeg-devel] [PATCH] vaapi_encode_mjpeg: WA: fix bad component id bug > > On 07.06.2019, at 22:28, "U. Artie Eoff" <ullysses.a.eoff@intel.com> 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 to me sounds like a serious enough bug that I'm tempted to suggest blacklisting gcc 9 instead... > However the original code doesn't seem really proper to me. > Have you tried replacing (uint8_t[3]) by (const uint8_t[3])? Yes I tried that but it didn't fix the issue. > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
> -----Original Message----- > From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On Behalf Of Mark Thompson > Sent: Friday, June 07, 2019 1:47 PM > To: ffmpeg-devel@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@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@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
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 }; 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.
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@intel.com> --- libavcodec/vaapi_encode_mjpeg.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)