| Message ID | 20230725085846.93593-1-thilo.borgmann@mail.de |
|---|---|
| Headers | show |
| Series | webp: add support for animated WebP decoding | expand |
On Tue, Jul 25, 2023 at 1:58 AM Thilo Borgmann <thilo.borgmann@mail.de> wrote: > > Still images fixed from v2. Now includes a fate test for animated webp. > > Patch 5/7 is still there for making changes in lavc/webp reviewable but > shall be stashed when pushing. > > -Thilo > > > Josef Zlomek (2): > libavcodec/webp: add support for animated WebP decoding > libavformat/webp: add WebP demuxer > > Thilo Borgmann (5): > avcodec/webp: move definitions into header > avcodec/webp: remove unused definitions > avcodec/webp_parser: parse each frame into one packet > avcodec/webp: make init_canvas_frame static > fate: add test for animated WebP > > Changelog | 2 + > doc/demuxers.texi | 28 + > libavcodec/codec_desc.c | 3 +- > libavcodec/version.h | 2 +- > libavcodec/webp.c | 715 +++++++++++++++++-- > libavcodec/webp.h | 38 + > libavcodec/webp_parser.c | 130 ++-- > libavformat/Makefile | 1 + > libavformat/allformats.c | 1 + > libavformat/version.h | 2 +- > libavformat/webpdec.c | 733 ++++++++++++++++++++ > tests/fate/image.mak | 3 + > tests/ref/fate/exif-image-webp | 12 +- > tests/ref/fate/webp-anim | 22 + > tests/ref/fate/webp-rgb-lena-lossless | 2 +- > tests/ref/fate/webp-rgb-lena-lossless-rgb24 | 2 +- > tests/ref/fate/webp-rgb-lossless | 2 +- > tests/ref/fate/webp-rgb-lossy-q80 | 2 +- > tests/ref/fate/webp-rgba-lossless | 2 +- > tests/ref/fate/webp-rgba-lossy-q80 | 2 +- > 20 files changed, 1589 insertions(+), 115 deletions(-) > create mode 100644 libavcodec/webp.h > create mode 100644 libavformat/webpdec.c > create mode 100644 tests/ref/fate/webp-anim > This series is lgtm. There are still a few edge cases where 1) the 'Canvas change detected' warning will be triggered with valid files, 2) corrupt / truncated files will produce output where they would fail with libwebp and 3) I see quite a few "[webp @ 0x7f5530008c00] Multiple ff_thread_finish_setup() calls", not sure if that's expected.
Am 25.07.23 um 22:14 schrieb James Zern: > On Tue, Jul 25, 2023 at 1:58 AM Thilo Borgmann <thilo.borgmann@mail.de> wrote: >> >> Still images fixed from v2. Now includes a fate test for animated webp. >> >> Patch 5/7 is still there for making changes in lavc/webp reviewable but >> shall be stashed when pushing. >> >> -Thilo >> >> >> Josef Zlomek (2): >> libavcodec/webp: add support for animated WebP decoding >> libavformat/webp: add WebP demuxer >> >> Thilo Borgmann (5): >> avcodec/webp: move definitions into header >> avcodec/webp: remove unused definitions >> avcodec/webp_parser: parse each frame into one packet >> avcodec/webp: make init_canvas_frame static >> fate: add test for animated WebP >> >> Changelog | 2 + >> doc/demuxers.texi | 28 + >> libavcodec/codec_desc.c | 3 +- >> libavcodec/version.h | 2 +- >> libavcodec/webp.c | 715 +++++++++++++++++-- >> libavcodec/webp.h | 38 + >> libavcodec/webp_parser.c | 130 ++-- >> libavformat/Makefile | 1 + >> libavformat/allformats.c | 1 + >> libavformat/version.h | 2 +- >> libavformat/webpdec.c | 733 ++++++++++++++++++++ >> tests/fate/image.mak | 3 + >> tests/ref/fate/exif-image-webp | 12 +- >> tests/ref/fate/webp-anim | 22 + >> tests/ref/fate/webp-rgb-lena-lossless | 2 +- >> tests/ref/fate/webp-rgb-lena-lossless-rgb24 | 2 +- >> tests/ref/fate/webp-rgb-lossless | 2 +- >> tests/ref/fate/webp-rgb-lossy-q80 | 2 +- >> tests/ref/fate/webp-rgba-lossless | 2 +- >> tests/ref/fate/webp-rgba-lossy-q80 | 2 +- >> 20 files changed, 1589 insertions(+), 115 deletions(-) >> create mode 100644 libavcodec/webp.h >> create mode 100644 libavformat/webpdec.c >> create mode 100644 tests/ref/fate/webp-anim >> > > This series is lgtm. There are still a few edge cases where > 1) the > 'Canvas change detected' warning will be triggered with valid files, As long as the canvas in frame threading is bound to a ThreadFrame, we can't reallocate for changes. Wouldn't want to touch that until the new threading is all done. > 2) corrupt / truncated files will produce output where they would fail > with libwebp and We might bail out as well though AFAICT we usually try to decode whatever might be possible. > 3) I see quite a few "[webp @ 0x7f5530008c00] > Multiple ff_thread_finish_setup() calls", not sure if that's expected. Which sample you're looking at? If I can reproduce this, I'll look at it. The other things I'd keep as they are. Thanks, Thilo
On Thu, Jul 27, 2023 at 4:29 AM Thilo Borgmann <thilo.borgmann@mail.de> wrote: > > Am 25.07.23 um 22:14 schrieb James Zern: > > On Tue, Jul 25, 2023 at 1:58 AM Thilo Borgmann <thilo.borgmann@mail.de> wrote: > >> > >> Still images fixed from v2. Now includes a fate test for animated webp. > >> > >> Patch 5/7 is still there for making changes in lavc/webp reviewable but > >> shall be stashed when pushing. > >> > >> -Thilo > >> > >> > >> Josef Zlomek (2): > >> libavcodec/webp: add support for animated WebP decoding > >> libavformat/webp: add WebP demuxer > >> > >> Thilo Borgmann (5): > >> avcodec/webp: move definitions into header > >> avcodec/webp: remove unused definitions > >> avcodec/webp_parser: parse each frame into one packet > >> avcodec/webp: make init_canvas_frame static > >> fate: add test for animated WebP > >> > >> Changelog | 2 + > >> doc/demuxers.texi | 28 + > >> libavcodec/codec_desc.c | 3 +- > >> libavcodec/version.h | 2 +- > >> libavcodec/webp.c | 715 +++++++++++++++++-- > >> libavcodec/webp.h | 38 + > >> libavcodec/webp_parser.c | 130 ++-- > >> libavformat/Makefile | 1 + > >> libavformat/allformats.c | 1 + > >> libavformat/version.h | 2 +- > >> libavformat/webpdec.c | 733 ++++++++++++++++++++ > >> tests/fate/image.mak | 3 + > >> tests/ref/fate/exif-image-webp | 12 +- > >> tests/ref/fate/webp-anim | 22 + > >> tests/ref/fate/webp-rgb-lena-lossless | 2 +- > >> tests/ref/fate/webp-rgb-lena-lossless-rgb24 | 2 +- > >> tests/ref/fate/webp-rgb-lossless | 2 +- > >> tests/ref/fate/webp-rgb-lossy-q80 | 2 +- > >> tests/ref/fate/webp-rgba-lossless | 2 +- > >> tests/ref/fate/webp-rgba-lossy-q80 | 2 +- > >> 20 files changed, 1589 insertions(+), 115 deletions(-) > >> create mode 100644 libavcodec/webp.h > >> create mode 100644 libavformat/webpdec.c > >> create mode 100644 tests/ref/fate/webp-anim > >> > > > > This series is lgtm. There are still a few edge cases where > > > 1) the > > 'Canvas change detected' warning will be triggered with valid files, > > As long as the canvas in frame threading is bound to a ThreadFrame, we can't reallocate for changes. > Wouldn't want to touch that until the new threading is all done. > > > > 2) corrupt / truncated files will produce output where they would fail > > with libwebp and > > We might bail out as well though AFAICT we usually try to decode whatever might be possible. > > > > 3) I see quite a few "[webp @ 0x7f5530008c00] > > Multiple ff_thread_finish_setup() calls", not sure if that's expected. > > Which sample you're looking at? > > If I can reproduce this, I'll look at it. The other things I'd keep as they are. > I see this with: https://www.gstatic.com/webp/gallery3/1_webp_a.webp Using an animation it seems to be output once per frame.