Martin Storsjö: > On Mon, 26 May 2025, Andreas Rheinhardt wrote: > >> ffmpeg | branch: master | Andreas Rheinhardt >> <andreas.rheinha...@outlook.com> | Thu May 22 15:57:13 2025 +0200| >> [0401ca714a2714743573e27c384ffa810fd31a92] | committer: Andreas >> Rheinhardt >> >> avcodec/asvenc: Don't waste bits encoding non-visible part >> >> Up until now, the encoder replicated all the border pixels >> for incomplete 16x16 macroblocks. In case the available width >> or height is <= 8, some of the luma blocks of the MB >> do not correspond to actual input, so that we should encode >> them using the least amount of bits. Zeroing the block coefficients >> (as this commit does) achieves this, replicating the pixels >> and performing an FDCT does not. >> >> This commit also removes the frame copying code for insufficiently >> aligned dimensions. >> >> The vsynth3-asv[12] FATE tests use a 34x34 input file and are >> therefore affected by this. As the ref updates show, the size >> and checksum of the encoded changes, yet the decoded output >> stays the same. >> >> Signed-off-by: Andreas Rheinhardt <andreas.rheinha...@outlook.com> >> >>> http://git.videolan.org/gitweb.cgi/ffmpeg.git/? >>> a=commit;h=0401ca714a2714743573e27c384ffa810fd31a92 >> --- >> >> libavcodec/asvenc.c | 131 +++++++++++++++++++++++++ >> +---------------- >> tests/ref/vsynth/vsynth3-asv1 | 4 +- >> tests/ref/vsynth/vsynth3-asv2 | 4 +- >> 3 files changed, 84 insertions(+), 55 deletions(-) > > As noted on irc, this broke the fate-vsynth3-asv1 and fate-vsynth3-asv2 > tests on arm. > > The issue is not AV_COPY128 as Andreas wondered on irc; the reason is a- >>pdsp.get_pixels. We have a backtrace like this: > > #0 0x00cfdd2a in ff_get_pixels_neon () > at src/libavcodec/arm/pixblockdsp_neon.S:47 > #1 0x0082ea54 in dct_get (mb_y=0, mb_x=0, frame=0xf6202f40, a=0x297f450) > at src/libavcodec/asvenc.c:222 > #2 encode_frame (avctx=0x297f150, pkt=0xf6202ec0, pict=0xf6202f40, > got_packet=0xf7df4af0) at src/libavcodec/asvenc.c:304 > > The arguments to a->pdsp.get_pixels are this on entry: > > (gdb) info registers r0 r1 r2 > r0 0x297f4a0 43512992 > r1 0x297caa0 43502240 > r2 0x22 34 > > After one iteration, it hits the bus error due to unaligned reads from > the source, which was expected to stay aligned after incrementing by the > stride. >
This means that the stride is unaligned. Which as I remember is an API violation on the user's (FFmpeg CLI) side. The alignment requirements for the frames passed to encoders are not really properly documented (in contrast to the requirements for decoders). The best I could find is the documentation of linesize in lavu/frame.h: "For video the linesizes should be multiples of the CPUs alignment preference, this is 16 or 32 for modern desktop CPUs. Some code requires such alignment other code can be slower without correct alignment, for yet other it makes no difference." I could not find an explicit requirement that the data pointer need to be sufficiently aligned. It is possible to hit this bug even before my patch with a commandline like this (tested with qemu): ffmpeg -filter_complex nullsrc=s=740x576:r=25,format=yuv420p,crop=w=720:x=2 -c:v asv2 -f null - The same input also gives a bus error with dvvideo. dnxhd is similarly affected by this (yet not the mpegvideo family, as it already checks the alignment and makes a copy if necessary (it needs the frames to be padded on all sides, so it is common for it to copy even properly aligned input). So do we consider this a bug in the asv encoders (and in dvvideo and dnxhd)? Or is it an API violation on part of the caller? In the former case, I could fix this by checking on a per-frame basis whether the frame is properly aligned and switching between get_pixels_unaligned and get_pixels on that basis. - Andreas _______________________________________________ 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".
