Opened 3 years ago

Closed 3 years ago

#9179 closed defect (fixed)

webp muxer fails to write out loop count

Reported by: octop Owned by:
Priority: important Component: avformat
Version: git-master Keywords: webp regression
Cc: Blocked By:
Blocking: Reproduced by developer: yes
Analyzed by developer: no

Description

Summary of the bug:
The WebP muxer (libavformat/webpenc.c) fails to write out the loop count configured via the loop option.

How to reproduce:

% ffmpeg -f lavfi -i testsrc2 -t 1 -loop 3 out.webp
ffmpeg version 2021-04-07-git-c06465a70b-essentials_build-www.gyan.dev Copyright (c) 2000-2021 the FFmpeg developers
  built with gcc 10.2.0 (Rev6, Built by MSYS2 project)
  configuration: --enable-gpl --enable-version3 --enable-static --disable-w32threads --disable-autodetect --enable-fontconfig --enable-iconv --enable-gnutls --enable-libxml2 --enable-gmp --enable-lzma --enable-zlib --enable-libsrt --enable-libssh --enable-libzmq --enable-avisynth --enable-sdl2 --enable-libwebp --enable-libx264 --enable-libx265 --enable-libxvid --enable-libaom --enable-libopenjpeg --enable-libvpx --enable-libass --enable-libfreetype --enable-libfribidi --enable-libvidstab --enable-libvmaf --enable-libzimg --enable-amf --enable-cuda-llvm --enable-cuvid --enable-ffnvcodec --enable-nvdec --enable-nvenc --enable-d3d11va --enable-dxva2 --enable-libmfx --enable-libgme --enable-libopenmpt --enable-libopencore-amrwb --enable-libmp3lame --enable-libtheora --enable-libvo-amrwbenc --enable-libgsm --enable-libopencore-amrnb --enable-libopus --enable-libspeex --enable-libvorbis --enable-librubberband
  libavutil      56. 72.100 / 56. 72.100
  libavcodec     58.135.100 / 58.135.100
  libavformat    58. 77.100 / 58. 77.100
  libavdevice    58. 14.100 / 58. 14.100
  libavfilter     7.111.100 /  7.111.100
  libswscale      5. 10.100 /  5. 10.100
  libswresample   3. 10.100 /  3. 10.100
  libpostproc    55. 10.100 / 55. 10.100
Input #0, lavfi, from 'testsrc2':
  Duration: N/A, start: 0.000000, bitrate: N/A
  Stream #0:0: Video: rawvideo (I420 / 0x30323449), yuv420p, 320x240 [SAR 1:1 DAR 4:3], 25 tbr, 25 tbn, 25 tbc
Stream mapping:
  Stream #0:0 -> #0:0 (rawvideo (native) -> webp (libwebp_anim))
Press [q] to stop, [?] for help
Output #0, webp, to 'out.webp':
  Metadata:
    encoder         : Lavf58.77.100
  Stream #0:0: Video: webp, yuv420p(progressive), 320x240 [SAR 1:1 DAR 4:3], q=2-31, 200 kb/s, 25 fps, 1k tbn
    Metadata:
      encoder         : Lavc58.135.100 libwebp_anim
frame=   25 fps=0.0 q=-0.0 Lsize=     111kB time=00:00:01.00 bitrate= 910.7kbits/s speed= 5.2x
video:111kB audio:0kB subtitle:0kB other streams:0kB global headers:0kB muxing overhead: 0.000000%

The loop count should appear in the resulting file at offset 0x2A.
Expected value: 03 00,
Actual value: 00 00.
This works correctly in 4.3.2 and appears to have been broken by the recent changes introduced in March.

Additional information:
Initially, I meant to report that the loop count isn't written out if the output is not seekable (e.g. a pipe), as that is broken in 4.3.2 as well. But since I tried reproducing this with a more recent build, I noticed that now it no longer works correctly even with seekable outputs.
As the loop count is written out in two places, flush and webp_write_trailer, I suppose the code in flush doesn't retrieve the correct option value for some reason, as the relevant section appears to write out

41 4E 49 4D 06 00 00 00 FF FF FF FF 00 00

instead of the expected

41 4E 49 4D 06 00 00 00 FF FF FF FF 03 00

and it would've been a problem which has existed for a long time now, but was obstructed by webp_write_trailer which seeks to the relevant offset in the output file and writes out the loop count again. In case of a non-seekable output, that seek would fail, and the loop count would be written out to whatever the current position in the output file happened to be at the moment. With 4.3.2, if I output to stdout, like so:

ffmpeg -f lavfi -i testsrc2 -t 1 -loop 3 -f webp - > out.webp

then the loop count is appended to the very end of the output file (essentially garbage, from the container point of view).

Change History (5)

comment:1 by octop, 3 years ago

Keywords: regression added

comment:2 by Carl Eugen Hoyos, 3 years ago

Priority: normalimportant
Reproduced by developer: set
Status: newopen
Version: unspecifiedgit-master

comment:3 by mkver, 3 years ago

The reason for this is that with the newer version only one packet ever reaches the muxer, so the check for frame_count > 1 kicks in and ensures that the loop counter won't be written.

With both the old and the new version there is a massive memleak because the encoder is apparently never properly closed; with the old version there are also memleaks that would appear if zero-sized packets reach the muxer (this is the kind of memleak that my patch was about).

comment:5 by James, 3 years ago

Resolution: fixed
Status: openclosed
Note: See TracTickets for help on using tickets.