[FFmpeg-devel] [PATCH 07/34] avcodec/dvenc: Avoid copying packet data

[Andreas Rheinhardt]

Michael Niedermayer:
> On Mon, Apr 26, 2021 at 12:34:21AM +0200, Andreas Rheinhardt wrote:
>> When the packet size is known in advance like here, one can avoid
>> an intermediate buffer for the packet data.
>>
>> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at outlook.com>
>> ---
>>  libavcodec/dvenc.c | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> this seems to break fate here:
> not sure why or if i did something wrong
> 
> TEST    vsynth1-dv-hd
> --- ./tests/ref/vsynth/vsynth1-dv-hd	2021-04-25 11:26:39.215771845 +0200
> +++ tests/data/fate/vsynth1-dv-hd	2021-04-26 15:02:16.645954326 +0200
> @@ -1,4 +1,4 @@
> -b2bcafc74dec5f9ca516cb25dd07459b *tests/data/fate/vsynth1-dv-hd.dv
> +382fbccaf3b3071895b30d9360f96c11 *tests/data/fate/vsynth1-dv-hd.dv
>  14400000 tests/data/fate/vsynth1-dv-hd.dv
>  34b78cf725346c7f819c9d6209b8299a *tests/data/fate/vsynth1-dv-hd.out.rawvideo
>  stddev:    4.30 PSNR: 35.45 MAXDIFF:   74 bytes:  7603200/  7603200
> Test vsynth1-dv-hd failed. Look at tests/data/fate/vsynth1-dv-hd.err for details.
> tests/Makefile:255: recipe for target 'fate-vsynth1-dv-hd' failed
> make: *** [fate-vsynth1-dv-hd] Error 1
> 
> [...]
> 
I tested this whole patchset with ASAN and somehow all the memory
ff_alloc_packet2() returned was initially zeroed, as is the case with
av_fast_padded_malloc() (which is currently used here). This encoder
seems to rely on this. I have not done an in-depth investigation, but
the diff has lots of 80-byte blocks of which only the first three bytes
are actually written:

< 00046460: 96bf 8500 0000 0000 0000 0000 0000 0000  ................
< 00046470: 0000 0000 0000 0000 0000 0000 0000 0000  ................
< 00046480: 0000 0000 0000 0000 0000 0000 0000 0000  ................
< 00046490: 0000 0000 0000 0000 0000 0000 0000 0000  ................
< 000464a0: 0000 0000 0000 0000 0000 0000 0000 0000  ................
< 000464b0: 96bf 8600 0000 0000 0000 0000 0000 0000  ................
< 000464c0: 0000 0000 0000 0000 0000 0000 0000 0000  ................
< 000464d0: 0000 0000 0000 0000 0000 0000 0000 0000  ................
< 000464e0: 0000 0000 0000 0000 0000 0000 0000 0000  ................
< 000464f0: 0000 0000 0000 0000 0000 0000 0000 0000  ................

> 00046460: 96bf 852a 2a2a 2a2a 2a2a 2a2a 2a2a 2a2a  ...*************
> 00046470: 2a2a 2a2a 2a2a 2a2a 2a2a 2a2a 2a2a 2a2a  ****************
> 00046480: 2a2a 2a2a 2a2a 2a2a 2a2a 2a2a 2a2a 2a2a  ****************
> 00046490: 2a2a 2a2a 2a2a 2a2a 2a2a 2a2a 2a2a 2a2a  ****************
> 000464a0: 2a2a 2a2a 2a2a 2a2a 2a2a 2a2a 2a2a 2a2a  ****************
> 000464b0: 96bf 862a 2a2a 2a2a 2a2a 2a2a 2a2a 2a2a  ...*************
> 000464c0: 2a2a 2a2a 2a2a 2a2a 2a2a 2a2a 2a2a 2a2a  ****************
> 000464d0: 2a2a 2a2a 2a2a 2a2a 2a2a 2a2a 2a2a 2a2a  ****************
> 000464e0: 2a2a 2a2a 2a2a 2a2a 2a2a 2a2a 2a2a 2a2a  ****************
> 000464f0: 2a2a 2a2a 2a2a 2a2a 2a2a 2a2a 2a2a 2a2a  ****************

(This is the output after I made av_realloc poison the returned memory
in case the original pointer was NULL, i.e. if it is a new allocation.
Maybe this should be added to CONFIG_MEMORY_POISON? I would have found
this issue directly if it were so.)

- Andreas