| Message ID | HE1PR0301MB21544C6C914BF0159917C91D8F439@HE1PR0301MB2154.eurprd03.prod.outlook.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [FFmpeg-devel,01/34] avcodec/adpcmenc: Avoid copying packet data | expand |
| Context | Check | Description |
|---|---|---|
| andriy/x86_make | success | Make finished |
| andriy/x86_make_fate | fail | Make fate failed |
| andriy/PPC64_make | success | Make finished |
| andriy/PPC64_make_fate | warning | Make fate failed |
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@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 [...]
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@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
diff --git a/libavcodec/dvenc.c b/libavcodec/dvenc.c index e0679c538c..337bc326ef 100644 --- a/libavcodec/dvenc.c +++ b/libavcodec/dvenc.c @@ -1164,7 +1164,8 @@ static int dvvideo_encode_frame(AVCodecContext *c, AVPacket *pkt, DVVideoContext *s = c->priv_data; int ret; - if ((ret = ff_alloc_packet2(c, pkt, s->sys->frame_size, 0)) < 0) + if ((ret = ff_alloc_packet2(c, pkt, s->sys->frame_size, + s->sys->frame_size)) < 0) return ret; c->pix_fmt = s->sys->pix_fmt;
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@outlook.com> --- libavcodec/dvenc.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)