Andreas Rheinhardt: > Up until now, ff_startcode_find_candidate_c() simply casts > an uint8_t* to uint64_t*/uint32_t* to read 64/32 bits at a time > in case HAVE_FAST_UNALIGNED is true. Yet this ignores the > alignment requirement of these types as well as effective type > rules of the C standard. This commit therefore replaces these > direct accesses with AV_RN64/32; this also improves > readability. > > UBSan reported these unaligned accesses which happened in 233 > FATE-tests involving H.264 and VC-1 (this has also been reported > in tickets #8138 and #8485); these tests are fixed by this commit. > > The output of GCC with -O3 is unchanged for aarch64, alpha, arm, > loongarch, ppc and x64. There was only a slight difference for mips.
Will apply this patch in two days with the last paragraph replaced by the following unless there are objections: The output of GCC with -O3 is unchanged for aarch64, loongarch, ppc and x64 (as well as for arches like alpha for which HAVE_FAST_UNALIGNED is never true in the first place). There was only a slight difference for mips and arm. I don't know about the speed impact of them. > > Signed-off-by: Andreas Rheinhardt <andreas.rheinha...@outlook.com> > --- > This is v2 of > https://patchwork.ffmpeg.org/project/ffmpeg/patch/20200122145210.6898-1-andreas.rheinha...@gmail.com/ > > Here is the mips code before this change: > > startcode_old_O3.o: file format elf64-tradlittlemips > > > Disassembly of section .text: > > 0000000000000000 <ff_startcode_find_candidate_c>: > 0: 18a00029 blez a1,a8 <ff_startcode_find_candidate_c+0xa8> > 4: 3c08ff7f lui a4,0xff7f > 8: 3c07ff01 lui a3,0xff01 > c: 65087f7f daddiu a4,a4,32639 > 10: 64e70101 daddiu a3,a3,257 > 14: 00084438 dsll a4,a4,0x10 > 18: 00073c38 dsll a3,a3,0x10 > 1c: 65087f7f daddiu a4,a4,32639 > 20: 64e70101 daddiu a3,a3,257 > 24: 00084478 dsll a4,a4,0x11 > 28: 00073df8 dsll a3,a3,0x17 > 2c: 00803025 move a2,a0 > 30: 00001025 move v0,zero > 34: 3508feff ori a4,a4,0xfeff > 38: 10000005 b 50 <ff_startcode_find_candidate_c+0x50> > 3c: 34e78080 ori a3,a3,0x8080 > 40: 24420008 addiu v0,v0,8 > 44: 0045182a slt v1,v0,a1 > 48: 10600015 beqz v1,a0 <ff_startcode_find_candidate_c+0xa0> > 4c: 00000000 nop > 50: dcc30000 ld v1,0(a2) > 54: 0068482d daddu a5,v1,a4 > 58: 44a30000 dmtc1 v1,$f0 > 5c: 44a90800 dmtc1 a5,$f1 > 60: 4be10002 pandn $f0,$f0,$f1 > 64: 44230000 dmfc1 v1,$f0 > 68: 00671824 and v1,v1,a3 > 6c: 1060fff4 beqz v1,40 <ff_startcode_find_candidate_c+0x40> > 70: 64c60008 daddiu a2,a2,8 > 74: 0045182a slt v1,v0,a1 > 78: 10600009 beqz v1,a0 <ff_startcode_find_candidate_c+0xa0> > 7c: 0082182d daddu v1,a0,v0 > 80: 10000005 b 98 <ff_startcode_find_candidate_c+0x98> > 84: 90640000 lbu a0,0(v1) > 88: 24420001 addiu v0,v0,1 > 8c: 10a20008 beq a1,v0,b0 <ff_startcode_find_candidate_c+0xb0> > 90: 00000000 nop > 94: 90640000 lbu a0,0(v1) > 98: 1480fffb bnez a0,88 <ff_startcode_find_candidate_c+0x88> > 9c: 64630001 daddiu v1,v1,1 > a0: 03e00008 jr ra > a4: 00000000 nop > a8: 03e00008 jr ra > ac: 00001025 move v0,zero > b0: 03e00008 jr ra > b4: 00a01025 move v0,a1 > ... > > And here after this change: > > startcode_new_O3.o: file format elf64-tradlittlemips > > > Disassembly of section .text: > > 0000000000000000 <ff_startcode_find_candidate_c>: > 0: 18a0002b blez a1,b0 <ff_startcode_find_candidate_c+0xb0> > 4: 3c08ff7f lui a4,0xff7f > 8: 3c07ff01 lui a3,0xff01 > c: 65087f7f daddiu a4,a4,32639 > 10: 64e70101 daddiu a3,a3,257 > 14: 00084438 dsll a4,a4,0x10 > 18: 00073c38 dsll a3,a3,0x10 > 1c: 65087f7f daddiu a4,a4,32639 > 20: 64e70101 daddiu a3,a3,257 > 24: 00084478 dsll a4,a4,0x11 > 28: 00073df8 dsll a3,a3,0x17 > 2c: 00803025 move a2,a0 > 30: 00001025 move v0,zero > 34: 3508feff ori a4,a4,0xfeff > 38: 10000005 b 50 <ff_startcode_find_candidate_c+0x50> > 3c: 34e78080 ori a3,a3,0x8080 > 40: 24420008 addiu v0,v0,8 > 44: 0045182a slt v1,v0,a1 > 48: 10600017 beqz v1,a8 <ff_startcode_find_candidate_c+0xa8> > 4c: 00000000 nop > 50: 68c30007 ldl v1,7(a2) > 54: 6cc30000 ldr v1,0(a2) > 58: 0068482d daddu a5,v1,a4 > 5c: 44a30000 dmtc1 v1,$f0 > 60: 44a90800 dmtc1 a5,$f1 > 64: 4be10002 pandn $f0,$f0,$f1 > 68: 44230000 dmfc1 v1,$f0 > 6c: 00671824 and v1,v1,a3 > 70: 1060fff3 beqz v1,40 <ff_startcode_find_candidate_c+0x40> > 74: 64c60008 daddiu a2,a2,8 > 78: 0045182a slt v1,v0,a1 > 7c: 1060000a beqz v1,a8 <ff_startcode_find_candidate_c+0xa8> > 80: 0082182d daddu v1,a0,v0 > 84: 10000006 b a0 <ff_startcode_find_candidate_c+0xa0> > 88: 90640000 lbu a0,0(v1) > 8c: 00000000 nop > 90: 24420001 addiu v0,v0,1 > 94: 10a20008 beq a1,v0,b8 <ff_startcode_find_candidate_c+0xb8> > 98: 00000000 nop > 9c: 90640000 lbu a0,0(v1) > a0: 1480fffb bnez a0,90 <ff_startcode_find_candidate_c+0x90> > a4: 64630001 daddiu v1,v1,1 > a8: 03e00008 jr ra > ac: 00000000 nop > b0: 03e00008 jr ra > b4: 00001025 move v0,zero > b8: 03e00008 jr ra > bc: 00a01025 move v0,a1 > > As one can see, the difference is that an ld has been replaced > by a pair of ldl and ldr. I don't know the performance implications > of this. > > libavcodec/startcode.c | 9 +++++---- > 1 file changed, 5 insertions(+), 4 deletions(-) > > diff --git a/libavcodec/startcode.c b/libavcodec/startcode.c > index 9efdffe8c6..d84f326521 100644 > --- a/libavcodec/startcode.c > +++ b/libavcodec/startcode.c > @@ -25,6 +25,7 @@ > * @author Michael Niedermayer <michae...@gmx.at> > */ > > +#include "libavutil/intreadwrite.h" > #include "startcode.h" > #include "config.h" > > @@ -38,14 +39,14 @@ int ff_startcode_find_candidate_c(const uint8_t *buf, int > size) > */ > #if HAVE_FAST_64BIT > while (i < size && > - !((~*(const uint64_t *)(buf + i) & > - (*(const uint64_t *)(buf + i) - 0x0101010101010101ULL)) & > + !((~AV_RN64(buf + i) & > + (AV_RN64(buf + i) - 0x0101010101010101ULL)) & > 0x8080808080808080ULL)) > i += 8; > #else > while (i < size && > - !((~*(const uint32_t *)(buf + i) & > - (*(const uint32_t *)(buf + i) - 0x01010101U)) & > + !((~AV_RN32(buf + i) & > + (AV_RN32(buf + i) - 0x01010101U)) & > 0x80808080U)) > i += 4; > #endif _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".