Feature request: parsing sBIT chunks

[anforowicz]

The PNG spec describes an sBIT chunk, but AFAICT the png crate ignores this kind of chunks today. Chromium/Blink integration doesn't have a hard dependency on support for sBIT chunks, but it would help for fully matching the behavior of legacy decoders - see https://crbug.com/359279096 for more details.

I think that adding sBIT support would involve something like:

• Figure out / design how to expose the sBIT information through the Info struct (this is a bit tricky, because depending on the color type, only bitness of some channels may be provided in the sBIT chunk). Once this is done, then add representation of the chunk to the Info struct in src/common.rs
• Add sBIT-specific definitions to src/chunk.rs and add fn parse_sbit to src/decoder/stream.rs (calling it from fn parse_chunk)
• Add unit tests

[HeroicKatora]

Would this interface work?

struct SbitChunk {
    // copied from the IHDR
    default_depth: u8,
    color_type: ColorType,
    // Filled from the chunk in order.
    parsed_depths: [u8; 4],
}

impl SbitChunk {
    pub fn red(&self) -> Option<u8> {
        if let ColorType::Rgb | ColorType::Rgba = self.color_type {
            Some(self.parsed_depths[0])
        } else {
            None
        }
    }

    pub fn alpha(&self) -> Option<u8> {
        match self.color_type {
            ColorType::GrayscaleAlpha => Some(self.parsed_depths[1]),
            ColorType::Rgba => Some(self.parsed_depths[3]),
           _ => None,
        }
    }

    pub fn greyscale(&self) -> Option<u8> {
        match self.color_type {
            ColorType::Grayscale | ColorType::GrayscaleAlpha => Some(self.parsed_depths[0]),
           _ => None,
        }
    }
}

impl Info<'_> {
    // Return the sBIT chunk or a synthesized one with default values derived from the IHDR.
    pub fn sbit_or_default(&self) -> SBitChunk { … }
}

[fintelia]

That interface seems fine to me, but I'd also be OK with something simpler like just having a sbit: Option<[u8; 4]> field on Info and adding note that any elements beyond the number expected in Info::color_type are set to zero.

(If the sBIT chunk itself is invalid, the field would be left as None in accordance with PNG's recommended behavior for corrupt chunks)

[kornelski]

BTW the implementation you've linked to checks for 565 depth exactly, instead of <=5 <=6 <=5 so it will miss out on optimizing for 555 and lower depths.

However, in practice I don't think there are any PNGs using sBIT on the Web.

[thirumurugan-git]

Is anyone currently working on this issue? If not, may I take it on to fix?

[Shnatsel]

@thirumurugan-git nobody is currently working on this, so please feel free to open a PR.

[Shnatsel]

Fixed by #524