Re: [FFmpeg-devel] [FFmpeg-cvslog] avcodec/asvenc: Don't waste bits encoding non-visible part

Re: [FFmpeg-devel] [FFmpeg-cvslog] avcodec/asvenc: Don't waste bits encoding non-visible part

[Martin Storsjö]

On Mon, 26 May 2025, Andreas Rheinhardt wrote:

> ffmpeg | branch: master | Andreas Rheinhardt <andreas.rheinhardt@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.rheinhardt@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.

// Martin

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

Re: [FFmpeg-devel] [FFmpeg-cvslog] avcodec/asvenc: Don't waste bits encoding non-visible part

[Andreas Rheinhardt]

Martin Storsjö:
> On Mon, 26 May 2025, Andreas Rheinhardt wrote:
> 
>> ffmpeg | branch: master | Andreas Rheinhardt
>> <andreas.rheinhardt@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.rheinhardt@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-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [FFmpeg-cvslog] avcodec/asvenc: Don't waste bits encoding non-visible part

[James Almer]

[-- Attachment #1.1.1: Type: text/plain, Size: 412 bytes --]

On 5/26/2025 10:06 AM, Andreas Rheinhardt wrote:
> 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 -

Why does that generate unaligned strides? nullsrc uses 
ff_get_video_buffer() to allocate a frame, which aligns strides with 
av_cpu_max_align().


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

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

Re: [FFmpeg-devel] [FFmpeg-cvslog] avcodec/asvenc: Don't waste bits encoding non-visible part

[Andreas Rheinhardt]

James Almer:
> On 5/26/2025 10:06 AM, Andreas Rheinhardt wrote:
>> 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 -
> 
> Why does that generate unaligned strides? nullsrc uses
> ff_get_video_buffer() to allocate a frame, which aligns strides with
> av_cpu_max_align().
> 

In this example, it is the pointers, not the strides that are unaligned.

- 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-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [FFmpeg-cvslog] avcodec/asvenc: Don't waste bits encoding non-visible part

[James Almer]

[-- Attachment #1.1.1: Type: text/plain, Size: 795 bytes --]

On 5/26/2025 10:22 AM, Andreas Rheinhardt wrote:
> James Almer:
>> On 5/26/2025 10:06 AM, Andreas Rheinhardt wrote:
>>> 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 -
>>
>> Why does that generate unaligned strides? nullsrc uses
>> ff_get_video_buffer() to allocate a frame, which aligns strides with
>> av_cpu_max_align().
>>
> 
> In this example, it is the pointers, not the strides that are unaligned.

Right, the crop filter adds offsets to the data pointers. In that case 
I'm leaning towards encoders having to assume non aligned pointers are 
possible, and the asv2 encoder fixed to take that into account.


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

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