| Message ID | AS8P250MB074443D60CB45900F2ACBA418F2A2@AS8P250MB0744.EURP250.PROD.OUTLOOK.COM |
|---|---|
| State | Accepted |
| Commit | 8e207bf463618c14a6e5cad882ee7622a303a320 |
| Headers | show |
| Series | [FFmpeg-devel] avcodec/mpeg4videodec: Align idct-block appropriately | expand |
| Context | Check | Description |
|---|---|---|
| yinshiyou/make_loongarch64 | success | Make finished |
| yinshiyou/make_fate_loongarch64 | success | Make fate finished |
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
On Wed, 13 Mar 2024 at 00:53, Andreas Rheinhardt < andreas.rheinhardt@outlook.com> wrote: > It is accessed via AV_RN64() in ff_simple_idct_put_int32_10bit(). > Should fix the UBSan failures in the mpeg4-simple-studio-profile > test here: > > https://fate.ffmpeg.org/report.cgi?time=20240312011016&slot=ppc-linux-gcc-13.2-ubsan-altivec-qemu > > Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com> > LGTM
Andreas: On Tue, Mar 12, 2024 at 8:53 PM Andreas Rheinhardt <andreas.rheinhardt@outlook.com> wrote: > > It is accessed via AV_RN64() in ff_simple_idct_put_int32_10bit(). > Should fix the UBSan failures in the mpeg4-simple-studio-profile > test here: > https://fate.ffmpeg.org/report.cgi?time=20240312011016&slot=ppc-linux-gcc-13.2-ubsan-altivec-qemu > > Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com> > --- > libavcodec/mpeg4videodec.h | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/libavcodec/mpeg4videodec.h b/libavcodec/mpeg4videodec.h > index c0e6ec6592..4a26d18987 100644 > --- a/libavcodec/mpeg4videodec.h > +++ b/libavcodec/mpeg4videodec.h > @@ -29,6 +29,7 @@ > #include "mpegvideo.h" > #include "mpeg4videodsp.h" > > +#include "libavutil/mem_internal.h" > > typedef struct Mpeg4DecContext { > MpegEncContext m; > @@ -83,7 +84,7 @@ typedef struct Mpeg4DecContext { > > Mpeg4VideoDSPContext mdsp; > > - int32_t block32[12][64]; > + DECLARE_ALIGNED(8, int32_t, block32)[12][64]; > // 0 = DCT, 1 = DPCM top to bottom scan, -1 = DPCM bottom to top scan > int dpcm_direction; > int16_t dpcm_macroblock[3][256]; > -- > 2.40.1 > > _______________________________________________ > 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". Confirming that this fixes fate-mpeg4-simple-studio-profile on PowerPC QEMU with GCC UBsan as noted. It was not failing on POWER7 (ppc64) or POWER9 (ppc64le). Thanks, Sean McGovern
Sean McGovern: > Andreas: > > On Tue, Mar 12, 2024 at 8:53 PM Andreas Rheinhardt > <andreas.rheinhardt@outlook.com> wrote: >> >> It is accessed via AV_RN64() in ff_simple_idct_put_int32_10bit(). >> Should fix the UBSan failures in the mpeg4-simple-studio-profile >> test here: >> https://fate.ffmpeg.org/report.cgi?time=20240312011016&slot=ppc-linux-gcc-13.2-ubsan-altivec-qemu >> >> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com> >> --- >> libavcodec/mpeg4videodec.h | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/libavcodec/mpeg4videodec.h b/libavcodec/mpeg4videodec.h >> index c0e6ec6592..4a26d18987 100644 >> --- a/libavcodec/mpeg4videodec.h >> +++ b/libavcodec/mpeg4videodec.h >> @@ -29,6 +29,7 @@ >> #include "mpegvideo.h" >> #include "mpeg4videodsp.h" >> >> +#include "libavutil/mem_internal.h" >> >> typedef struct Mpeg4DecContext { >> MpegEncContext m; >> @@ -83,7 +84,7 @@ typedef struct Mpeg4DecContext { >> >> Mpeg4VideoDSPContext mdsp; >> >> - int32_t block32[12][64]; >> + DECLARE_ALIGNED(8, int32_t, block32)[12][64]; >> // 0 = DCT, 1 = DPCM top to bottom scan, -1 = DPCM bottom to top scan >> int dpcm_direction; >> int16_t dpcm_macroblock[3][256]; >> -- >> 2.40.1 >> >> _______________________________________________ >> 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". > > Confirming that this fixes fate-mpeg4-simple-studio-profile on PowerPC > QEMU with GCC UBsan as noted. > > It was not failing on POWER7 (ppc64) or POWER9 (ppc64le). > Not surprising: The Mpeg4VideoDSPContext immediately before block32 contains (function) pointers and therefore has alignment 8 on 64bit machines, so that block32 happens to be properly aligned already. - Andreas
diff --git a/libavcodec/mpeg4videodec.h b/libavcodec/mpeg4videodec.h index c0e6ec6592..4a26d18987 100644 --- a/libavcodec/mpeg4videodec.h +++ b/libavcodec/mpeg4videodec.h @@ -29,6 +29,7 @@ #include "mpegvideo.h" #include "mpeg4videodsp.h" +#include "libavutil/mem_internal.h" typedef struct Mpeg4DecContext { MpegEncContext m; @@ -83,7 +84,7 @@ typedef struct Mpeg4DecContext { Mpeg4VideoDSPContext mdsp; - int32_t block32[12][64]; + DECLARE_ALIGNED(8, int32_t, block32)[12][64]; // 0 = DCT, 1 = DPCM top to bottom scan, -1 = DPCM bottom to top scan int dpcm_direction; int16_t dpcm_macroblock[3][256];
It is accessed via AV_RN64() in ff_simple_idct_put_int32_10bit(). Should fix the UBSan failures in the mpeg4-simple-studio-profile test here: https://fate.ffmpeg.org/report.cgi?time=20240312011016&slot=ppc-linux-gcc-13.2-ubsan-altivec-qemu Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com> --- libavcodec/mpeg4videodec.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)