From 1f2c4409ef78c158586a058acb5fcf8bdfd13e4c Mon Sep 17 00:00:00 2001 From: Lukasz Anforowicz Date: Wed, 19 Feb 2025 21:23:05 +0000 Subject: [PATCH] [rust png] Add a new `SkCodec::isAnimated` API. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The new `SkCodec::isAnimated` API helps to disambiguate what is meant by `codec->getRepetitionCount()` returning a zero. This in turn supports https://crrev.com/c/6271944 which tweaks Chromium's `blink::SkiaImageDecoderBase::RepetitionCount` to correctly return `kAnimationNone` for partial input of a static PNG image (rather than returning `kAnimationLoopOnce`). https://crbug.com/396192872#comment24 and https://crbug.com/396192872#comment26 describe why adding the new API seems like the best way forward. Bug: chromium:396192872 Change-Id: If425788aaafbdb2f4683390eccc2e08c40e9ea3e Reviewed-on: https://skia-review.googlesource.com/c/skia/+/952050 Commit-Queue: Łukasz Anforowicz Reviewed-by: Florin Malita Auto-Submit: Łukasz Anforowicz --- .../rust_png/decoder/impl/SkPngRustCodec.cpp | 7 ++++ .../rust_png/decoder/impl/SkPngRustCodec.h | 1 + include/codec/SkCodec.h | 40 ++++++++++++++++++- relnotes/codec_is_animated_api.md | 2 + src/codec/SkAvifCodec.cpp | 7 ++++ src/codec/SkAvifCodec.h | 1 + src/codec/SkCrabbyAvifCodec.cpp | 7 ++++ src/codec/SkCrabbyAvifCodec.h | 1 + src/codec/SkHeifCodec.cpp | 14 +++++++ src/codec/SkHeifCodec.h | 1 + src/codec/SkJpegxlCodec.cpp | 5 +++ src/codec/SkJpegxlCodec.h | 1 + src/codec/SkWebpCodec.cpp | 5 +++ src/codec/SkWebpCodec.h | 1 + src/codec/SkWuffsCodec.cpp | 11 +++++ tests/CodecAnimTest.cpp | 10 +++++ 16 files changed, 113 insertions(+), 1 deletion(-) create mode 100644 relnotes/codec_is_animated_api.md diff --git a/experimental/rust_png/decoder/impl/SkPngRustCodec.cpp b/experimental/rust_png/decoder/impl/SkPngRustCodec.cpp index 64a0ab3ec7f9..c4523e5887fe 100644 --- a/experimental/rust_png/decoder/impl/SkPngRustCodec.cpp +++ b/experimental/rust_png/decoder/impl/SkPngRustCodec.cpp @@ -758,6 +758,13 @@ int SkPngRustCodec::onGetRepetitionCount() { return numPlays - 1; } +SkCodec::IsAnimated SkPngRustCodec::onIsAnimated() { + if (fReader->has_actl_chunk() && fReader->get_actl_num_frames() > 1) { + return IsAnimated::kYes; + } + return IsAnimated::kNo; +} + std::optional> SkPngRustCodec::onTryGetPlteChunk() { if (fReader->output_color_type() != rust_png::ColorType::Indexed) { return std::nullopt; diff --git a/experimental/rust_png/decoder/impl/SkPngRustCodec.h b/experimental/rust_png/decoder/impl/SkPngRustCodec.h index 2bab2e077bc5..26ab442924a7 100644 --- a/experimental/rust_png/decoder/impl/SkPngRustCodec.h +++ b/experimental/rust_png/decoder/impl/SkPngRustCodec.h @@ -110,6 +110,7 @@ class SkPngRustCodec final : public SkPngCodecBase { int onGetFrameCount() override; bool onGetFrameInfo(int, FrameInfo*) const override; int onGetRepetitionCount() override; + IsAnimated onIsAnimated() override; const SkFrameHolder* getFrameHolder() const override; std::unique_ptr getEncodedData() const override; diff --git a/include/codec/SkCodec.h b/include/codec/SkCodec.h index d179e54d4177..dc5f202da250 100644 --- a/include/codec/SkCodec.h +++ b/include/codec/SkCodec.h @@ -784,12 +784,46 @@ class SK_API SkCodec : SkNoncopyable { * * As such, future decoding calls may require a rewind. * - * For still (non-animated) image codecs, this will return 0. + * `getRepetitionCount` will return `0` in two cases: + * 1. Still (non-animated) images. + * 2. Animated images that only play the animation once (i.e. that don't + * repeat the animation) + * `isAnimated` can be used to disambiguate between these two cases. */ int getRepetitionCount() { return this->onGetRepetitionCount(); } + /** + * `isAnimated` returns whether the full input is expected to contain an + * animated image (i.e. more than 1 image frame). This can be used to + * disambiguate the meaning of `getRepetitionCount` returning `0` (see + * `getRepetitionCount`'s doc comment for more details). + * + * Note that in some codecs `getFrameCount()` only returns the number of + * frames for which all the metadata has been already successfully decoded. + * Therefore for a partial input `isAnimated()` may return "yes", even + * though `getFrameCount()` may temporarily return `1` until more of the + * input is available. + * + * When handling partial input, some codecs may not know until later (e.g. + * until encountering additional image frames) whether the given image has + * more than one frame. Such codecs may initially return + * `IsAnimated::kUnknown` and only later give a definitive "yes" or "no" + * answer. GIF format is one example where this may happen. + * + * Other codecs may be able to decode the information from the metadata + * present before the first image frame. Such codecs should be able to give + * a definitive "yes" or "no" answer as soon as they are constructed. PNG + * format is one example where this happens. + */ + enum class IsAnimated { + kYes, + kNo, + kUnknown, + }; + IsAnimated isAnimated() { return this->onIsAnimated(); } + // Register a decoder at runtime by passing two function pointers: // - peek() to return true if the span of bytes appears to be your encoded format; // - make() to attempt to create an SkCodec from the given stream. @@ -937,6 +971,10 @@ class SK_API SkCodec : SkNoncopyable { return 0; } + virtual IsAnimated onIsAnimated() { + return IsAnimated::kNo; + } + private: const SkEncodedInfo fEncodedInfo; XformFormat fSrcXformFormat; diff --git a/relnotes/codec_is_animated_api.md b/relnotes/codec_is_animated_api.md new file mode 100644 index 000000000000..91c12b847b66 --- /dev/null +++ b/relnotes/codec_is_animated_api.md @@ -0,0 +1,2 @@ +The `SkCodec` class has a new `isAnimated` method which helps to disambiguate +the meaning of `codec->getRepetitionCount()` returning `0`. diff --git a/src/codec/SkAvifCodec.cpp b/src/codec/SkAvifCodec.cpp index 272c7b9022eb..534ec8203189 100644 --- a/src/codec/SkAvifCodec.cpp +++ b/src/codec/SkAvifCodec.cpp @@ -188,6 +188,13 @@ bool SkAvifCodec::onGetFrameInfo(int i, FrameInfo* frameInfo) const { int SkAvifCodec::onGetRepetitionCount() { return kRepetitionCountInfinite; } +SkCodec::IsAnimated SkAvifCodec::onIsAnimated() { + if (!fUseAnimation || fAvifDecoder->imageCount <= 1) { + return IsAnimated::kNo; + } + return IsAnimated::kYes; +} + SkCodec::Result SkAvifCodec::onGetPixels(const SkImageInfo& dstInfo, void* dst, size_t dstRowBytes, diff --git a/src/codec/SkAvifCodec.h b/src/codec/SkAvifCodec.h index c738553756cd..87164a4e884b 100644 --- a/src/codec/SkAvifCodec.h +++ b/src/codec/SkAvifCodec.h @@ -53,6 +53,7 @@ class SkAvifCodec : public SkScalingCodec { int onGetFrameCount() override; bool onGetFrameInfo(int, FrameInfo*) const override; int onGetRepetitionCount() override; + IsAnimated onIsAnimated() override; const SkFrameHolder* getFrameHolder() const override { return &fFrameHolder; } private: diff --git a/src/codec/SkCrabbyAvifCodec.cpp b/src/codec/SkCrabbyAvifCodec.cpp index d1ee110dc6f4..b31e746512fb 100644 --- a/src/codec/SkCrabbyAvifCodec.cpp +++ b/src/codec/SkCrabbyAvifCodec.cpp @@ -340,6 +340,13 @@ int SkCrabbyAvifCodec::onGetRepetitionCount() { : fAvifDecoder->repetitionCount; } +SkCodec::IsAnimated SkCrabbyAvifCodec::onIsAnimated() { + if (!fUseAnimation || fAvifDecoder->imageCount <= 1) { + return IsAnimated::kNo; + } + return IsAnimated::kYes; +} + bool SkCrabbyAvifCodec::conversionSupported(const SkImageInfo& dstInfo, bool srcIsOpaque, bool needsColorXform) { diff --git a/src/codec/SkCrabbyAvifCodec.h b/src/codec/SkCrabbyAvifCodec.h index 56ecf5638d8a..90b463a41baf 100644 --- a/src/codec/SkCrabbyAvifCodec.h +++ b/src/codec/SkCrabbyAvifCodec.h @@ -60,6 +60,7 @@ class SkCrabbyAvifCodec : public SkScalingCodec { int onGetFrameCount() override; bool onGetFrameInfo(int, FrameInfo*) const override; int onGetRepetitionCount() override; + IsAnimated onIsAnimated() override; const SkFrameHolder* getFrameHolder() const override { return &fFrameHolder; } bool conversionSupported(const SkImageInfo&, bool, bool) override; bool onGetGainmapCodec(SkGainmapInfo* info, std::unique_ptr* gainmapCodec) override; diff --git a/src/codec/SkHeifCodec.cpp b/src/codec/SkHeifCodec.cpp index 81e016ff0811..31c1cb9e1520 100644 --- a/src/codec/SkHeifCodec.cpp +++ b/src/codec/SkHeifCodec.cpp @@ -401,6 +401,20 @@ int SkHeifCodec::onGetRepetitionCount() { return kRepetitionCountInfinite; } +SkCodec::IsAnimated SkHeifCodec::onIsAnimated() { + if (!fUseAnimation) { + return IsAnimated::kNo; + } + + size_t frameCount; + HeifFrameInfo frameInfo; + if (!fHeifDecoder->getSequenceInfo(&frameInfo, &frameCount)) { + return IsAnimated::kUnknown; + } + + return (frameCount > 1) ? IsAnimated::kYes : IsAnimated::kNo; +} + /* * Performs the heif decode */ diff --git a/src/codec/SkHeifCodec.h b/src/codec/SkHeifCodec.h index 5bca89dfb078..f5acee3d7961 100644 --- a/src/codec/SkHeifCodec.h +++ b/src/codec/SkHeifCodec.h @@ -53,6 +53,7 @@ class SkHeifCodec : public SkCodec { int onGetFrameCount() override; bool onGetFrameInfo(int, FrameInfo*) const override; int onGetRepetitionCount() override; + IsAnimated onIsAnimated() override; const SkFrameHolder* getFrameHolder() const override { return &fFrameHolder; } diff --git a/src/codec/SkJpegxlCodec.cpp b/src/codec/SkJpegxlCodec.cpp index 87a54f7090dc..adc62f80a16e 100644 --- a/src/codec/SkJpegxlCodec.cpp +++ b/src/codec/SkJpegxlCodec.cpp @@ -441,6 +441,11 @@ int SkJpegxlCodec::onGetRepetitionCount() { return std::numeric_limits::max(); } +SkCodec::IsAnimated SkJpegxlCodec::onIsAnimated() { + JxlBasicInfo& info = fCodec->fInfo; + return info.have_animation ? IsAnimated::kYes : IsAnimated::kNo; +} + const SkFrameHolder* SkJpegxlCodec::getFrameHolder() const { return fCodec.get(); } diff --git a/src/codec/SkJpegxlCodec.h b/src/codec/SkJpegxlCodec.h index c8c5a68ead19..6cef8842966c 100644 --- a/src/codec/SkJpegxlCodec.h +++ b/src/codec/SkJpegxlCodec.h @@ -75,6 +75,7 @@ class SkJpegxlCodec : public SkScalingCodec { bool onGetFrameInfo(int, FrameInfo*) const override; int onGetRepetitionCount() override; + IsAnimated onIsAnimated() override; private: const SkFrameHolder* getFrameHolder() const override; diff --git a/src/codec/SkWebpCodec.cpp b/src/codec/SkWebpCodec.cpp index c194074309fc..0d20aedbad67 100644 --- a/src/codec/SkWebpCodec.cpp +++ b/src/codec/SkWebpCodec.cpp @@ -251,6 +251,11 @@ int SkWebpCodec::onGetRepetitionCount() { return loopCount; } +SkCodec::IsAnimated SkWebpCodec::onIsAnimated() { + auto flags = WebPDemuxGetI(fDemux.get(), WEBP_FF_FORMAT_FLAGS); + return (flags & ANIMATION_FLAG) != 0 ? IsAnimated::kYes : IsAnimated::kNo; +} + int SkWebpCodec::onGetFrameCount() { auto flags = WebPDemuxGetI(fDemux.get(), WEBP_FF_FORMAT_FLAGS); if (!(flags & ANIMATION_FLAG)) { diff --git a/src/codec/SkWebpCodec.h b/src/codec/SkWebpCodec.h index 2390eb372cba..4551c19537bd 100644 --- a/src/codec/SkWebpCodec.h +++ b/src/codec/SkWebpCodec.h @@ -46,6 +46,7 @@ class SkWebpCodec final : public SkScalingCodec { int onGetFrameCount() override; bool onGetFrameInfo(int, FrameInfo*) const override; int onGetRepetitionCount() override; + IsAnimated onIsAnimated() override; const SkFrameHolder* getFrameHolder() const override { return &fFrameHolder; diff --git a/src/codec/SkWuffsCodec.cpp b/src/codec/SkWuffsCodec.cpp index 732aaecd9b47..81c4dc64f2c8 100644 --- a/src/codec/SkWuffsCodec.cpp +++ b/src/codec/SkWuffsCodec.cpp @@ -262,6 +262,7 @@ class SkWuffsCodec final : public SkScalingCodec { int onGetFrameCount() override; bool onGetFrameInfo(int, FrameInfo*) const override; int onGetRepetitionCount() override; + IsAnimated onIsAnimated() override; // Two separate implementations of onStartIncrementalDecode and // onIncrementalDecode, named "one pass" and "two pass" decoding. One pass @@ -881,6 +882,16 @@ int SkWuffsCodec::onGetRepetitionCount() { return n < INT_MAX ? n : INT_MAX; } +SkCodec::IsAnimated SkWuffsCodec::onIsAnimated() { + if (fFrames.size() > 1) { + return IsAnimated::kYes; + } + + // If we only have encounted a single image frame so far, then we have an + // ambiguous situation - maybe more frames will come, but maybe not. + return fFramesComplete ? IsAnimated::kNo : IsAnimated::kUnknown; +} + SkCodec::Result SkWuffsCodec::seekFrame(int frameIndex) { if (fDecoderIsSuspended) { SkCodec::Result res = this->resetDecoder(); diff --git a/tests/CodecAnimTest.cpp b/tests/CodecAnimTest.cpp index ef93a8b2ec66..015299f7837b 100644 --- a/tests/CodecAnimTest.cpp +++ b/tests/CodecAnimTest.cpp @@ -345,6 +345,16 @@ DEF_TEST(Codec_frames, r) { rec.fName, rec.fRepetitionCount, repetitionCount); } + // When decoding the full, non-partial input, `isAnimated()` will + // just be a proxy for "is there just a single frame?". + const SkCodec::IsAnimated expectedIsAnimated = + rec.fFrameCount == 1 ? SkCodec::IsAnimated::kNo : SkCodec::IsAnimated::kYes; + const SkCodec::IsAnimated actualIsAnimated = codec->isAnimated(); + if (expectedIsAnimated != actualIsAnimated) { + ERRORF(r, "%s isAnimated does not match! expected: %i\tactual: %i", rec.fName, + static_cast(expectedIsAnimated), static_cast(actualIsAnimated)); + } + // From here on, we are only concerned with animated images. if (1 == frameCount) { continue;