image-rs / image

Encoding and decoding images in Rust
Apache License 2.0
4.88k stars 605 forks source link

Color space information #786

Open casey opened 6 years ago

casey commented 6 years ago

It would be nice to be able to access image color space information, which is often stored in image metadata.

Ogeon's palette crate has a good overview of color spaces as they relate to image formats.

For background, I'm writing a simple game framework, and I'd like to be able to correctly convert to a linear color space when loading assets, which I'm unable to do without knowing the image's color space.

Thanks!

HeroicKatora commented 4 years ago

Based on ffmpeg I had an idea how this could be implemented internally, based on palette but without exposing that detail directly.

For example, let's have a look at ffmpeg for reference:

  • There is a Context that describes the color transformation without any buffer involved.
  • Although it uses a completely different and dynamic Pixel so it may only be of limited use as an example.
  • Here is the ColorSpace enum for reference.

The similar interface in terms of Rust but also type safe would looks like so:

// Define an opaque converter struct, which models `Context`.
struct ColorConverter<From, To> {
    // Internals still unsure, but possibly:
    map: Box<dyn Fn(From) -> To + Send + Sync 'static>,
}

// The color space enum.
enum ColorSpace {
    // probably the same variants
}

// A trait to create a converter for those types for which we have it implemented.
trait MakeConverter<From, To> {
    fn make_converter(self) -> Result<ColorConverter<From, To>, ImageError>;
}

impl<From, To> ColorConverter<From, To> {
    pub fn new(seed: impl MakeConverter<From, To>)
        -> Result<Self, ImageError>
    {
        seed.make_converter()
    }

    fn apply(&self, from: &impl GenericImageView<Pixel=From>)
        -> ImageBuffer<To>
    { }
}

// And implement `MakeConverter` on `ColorSpace` for specific pixel types:
impl MakeConverter<Rgb<u8>, Luma<u8>> for ColorSpace {
    fn make_converter(self) -> Result<ColorConverter<From, To>, ImageError> {
        match self {
            ITU709 => /* some code to create it */
            _ => Err(ImageError::Unsupported(..)),
        }
    }
}

I know that @fintelia wanted to get rid of the strict typing in the long term, which I've also given some consideration. As long as the bounds From, To: Pixel are not introduced at the type or trait level we can introduce a Dynamic struct and use it to later operate on dynamic pixels by implementing MakeConverter<Dynamic, Dynamic> as appropriate.

fintelia commented 4 years ago

To be clear, the converter would keep the same colorspace, but convert the color type? That interface seems reasonable to me, though we should watch to make sure there aren't too many (From, To) pairs that the boilerplate gets unwieldy.

HeroicKatora commented 4 years ago

To be clear, the converter would keep the same colorspace, but convert the color type?

In this particular constructor, yes. That is sufficient for the rgb - luma pairs, which should account for a large amount of cases. It's also a simple proposal, we may want to always operate on a pair instead but that is not important to simply assign a color space to the Rgb<u8> representation in the first place.

We've also been asked about applying icc profiles. A potential solution is have an ICC struct which implements MakeConverter but for the device and source color types as From To. Details would have to be made clear in another sketch if we decide to introduce that.

HeroicKatora commented 4 years ago

Two other thoughts I had: