Skip to content

Assorted optimizations for GStreamer's image-rs plugin#2907

Draft
amyspark wants to merge 2 commits intoimage-rs:version-0.25from
amyspark:add-png-avif-extras
Draft

Assorted optimizations for GStreamer's image-rs plugin#2907
amyspark wants to merge 2 commits intoimage-rs:version-0.25from
amyspark:add-png-avif-extras

Conversation

@amyspark
Copy link
Copy Markdown

@amyspark amyspark commented Apr 9, 2026

Hi,

This is a MR to add a few things for feature parity in my GStreamer reader plugin:

  • PNG: iCCP chunk's profile name
  • AVIF: speed up the reader by delaying actual decoding until read_image

Both require upstream work, so marking as draft until those are landed: image-rs/image-png#679, mozilla/mp4parse-rust#446


/// Return the ICC profile's name as embedded in the image.
///
/// For formats that don't store embedded profiles with separate names this function should always return `Ok(None)`.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are there formats other than PNG which do store names for the embedded profiles?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's the only one. I introduced this API, however, because I didn't know how to downcast from impl ImageDecoder to PngDecoder (and so access a decoder specific API).

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We may need a novel mechanism for this. Downcast ties it to the png version which conflicts with the requirement of being able to bump it (through breaking changes). On the other hand a new API on the trait is also rather awkward. Maybe something similar to Error::provide would fit the bill in letting us translate types provided by version-encumbered crates to stable types defined in image directly (that we then only let include the 'stable' options / data from the formats). The converse holds for encoder, another unsatisfied user ask.

Copy link
Copy Markdown
Contributor

@fintelia fintelia Apr 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're trying to cast between a Box<dyn ImageDecoder> and a image::codecs::png::PngDecoder, right?

For that, I think we might just be able switch to a Box<dyn ImageDecoder + Any> or make Any a super-trait of ImageDecoder?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any does not really solve the problem. It only makes the dependency problem a runtime error but I'd consider that worse than a breaking change everytime we bump dependencies. There's no reasonable way to use Any::downcast right here for downstream—they'd have to add fallback cases for potentially unpublished versions of our dependencies preemptively to avoid functionality breaking and that just pollutes the dependency graph greatly.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, if I'm understanding correctly, the goal isn't to downcast to the decoder struct from the png crate but rather to cast to the PngDecoder struct in this crate

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants