Gain map support

[anforowicz]

Let me try to start a future-looking discussion about support for gainmaps. I say “future-looking” because AFAIK:

• The work to add gainmaps to the PNG spec is still in progress (see Proposal: gain maps for PNG w3c/png#380).
• Support for gain maps is not a blocker for adopting Rust png in Chromium. (At least not from Chromium Security perspective, since we care mostly about parity with libpng / SkPngCodec / blink::PNGImageDecoder and none of them support gain maps today.)

But, despite the caveats above, I think it’s still desirable to start some early discussions on how Rust png crate API could potentially accommodate gain maps:

• I assume that next_frame and/or next_interlaced_row APIs would also work for decoding the hypothetical gDAT chunks.
  • IIUC the gDAT output color type may be different from that of IDAT / fDAT. I don’t know if this is a problem (e.g. if Reader.output_color_type and/or Reader.output_buffer_size APIs would need to evolve, or get gain-map-specific sibling methods).
• I assume that a method like next_frame_info can be used to seek to the start of the next sequence of IDAT / fDAT / gDAT chunks
  • IIUC next_frame_info’s PR has been merged relatively recently and so it hasn’t yet been released on crates.io (and therefore breaking changes are not a concern at this point)
    • Today next_frame_info returns &FrameControl which won't work when gDAT doesn't have a preceding fCTL.
  • There probably should be an API to indicate if the next frame is gDAT vs IDAT or fDAT:
    • Maybe we need to introduce enum ImageDataKind { /* IDAT ? */ fDAT, gDAT }?
    • Maybe next_frame_info can return ImageDataKind? (this wouldn't help with identifying the very first image, but the current spec proposal implies that IDAT would always remain the first image data kind - this is why I've tentatively commented out IDAT above)
    • Or as an alternative we could add Info.current_image_data_kind or info.next_image_data_kind? (it should work, but it feels weird that it would be sort of undefined between the end of xDAT and before the start of the next yDAT sequence)
• I assume that representing gMAP chunk in Info struct is pretty straightforward (once the spec discussion settle down and we know what this chunk actually covers).

/cc @ccameron-chromium

[fintelia]

The current API design automatically decodes all metadata in bulk (regardless of whether the user is going to want it) and provides a stateful API to access the image data in the order it appears in the file. Within those constraints, the design you're describing probably makes the most sense. The other option would be to treat the entire gain map as metadata to be held inside the Info struct, which doesn't seem ideal.

If we required a Seek bound, it would open up other options. For instance, adding a dedicated gain_map method that read and decoded the gain map on-demand. Or a method to seek to a given ImageDataKind, and then use the normal decoding methods to read it.

(I started #534 which discusses some other changes a Seek bound would enable)

[kornelski]

How about returning a separate reader instance for reading a frame of the gain map?

The current reader would remain used exclusively for SDR pixels.

fn read_gain_map(&mut self) -> Option<GainMapReader<'_, R>>

This way the current reader wouldn't have stateful multiple purposes, and the sub-reader could expose all the individual methods and properties just for the gain map.