Add streaming/incremental decoding support to BMP decoder

[telecos]

This PR implements the proposal for resumable decoding for BMP images, allowing metadata and pixel data to be loaded incrementally when the full image data isn't immediately available (e.g., network streams, async I/O).

New API (proposal document: https://hackmd.io/@QFC5jGJdR42daEpF6AzCKg/rkmGAQmXbe):

• BmpDecoder::new_streaming(): Create decoder in streaming mode
• try_read_metadata(): Incrementally load image metadata
• try_read_row(): Decode pixel data row-by-row

Key features:

Fully backward compatible - existing API unchanged
State machine preserves progress across calls when data is unavailable
Returns ImageError::InsufficientData when more data needed (retryable)
Supports all BMP formats including RLE compression and ICC profiles
Async-safe I/O handling (WouldBlock, Interrupted)
Use case: This enables the Chromium experiment (tracked in https://issues.chromium.org/issues/452666425) for switching to Rust-based BMP decoding where images are received incrementally over the network.

API documentation: https://hackmd.io/@QFC5jGJdR42daEpF6AzCKg/rkmGAQmXbe

Closes #2696

[mstoeckl]

One method to check that the streaming decoder implementation is consistent with the original is with a differential fuzzer (like https://github.com/mstoeckl/image/blob/bmp-differential-fuzz/fuzz/fuzzers/fuzzer_script_bmp_differential.rs .) I don't know if the image project maintainers would prefer to have the streaming implementation replace or stay alongside the current decoder; but while this PR is in progress, you may find it useful to find discrepancies between the implementation that could be bugs in either the old or new implementations. (Or it may prove tedious if there are too many false positives.)

The first discrepancy that I found with it is in bitmask reading: for V4 and V5 headers, it looks like the streaming decoder always loads the bitmasks directly after the full header, while the current implementation loads the bitmasks from inside the header. See

image/src/codecs/bmp/decoder.rs

Lines 860 to 879 in 256dc9d

	let mut bitmask_bytes_offset = 0;
	if self.image_type == ImageType::Bitfields16
	|| self.image_type == ImageType::Bitfields32
	{
	self.read_bitmasks()?;
	// Per https://learn.microsoft.com/en-us/windows/win32/gdi/bitmap-header-types, bitmaps
	// using the `BITMAPINFOHEADER`, `BITMAPV4HEADER`, or `BITMAPV5HEADER` structures with
	// an image type of `BI_BITFIELD` contain RGB bitfield masks immediately after the header.
	//
	// `read_bitmasks` correctly reads these from earlier in the header itself but we must
	// ensure the reader starts on the image data itself, not these extra mask bytes.
	if matches!(
	self.bmp_header_type,
	BMPHeaderType::Info | BMPHeaderType::V4 | BMPHeaderType::V5
	) {
	// This is `size_of::<u32>() * 3` (a red, green, and blue mask), but with less noise.
	bitmask_bytes_offset = 12;
	}
	};

, introduced by 140c36c ; the BMP image documentation that it refers to appears rather confusing, so I'm not entirely certain which is right. (Or if both are wrong.)

[telecos]

Thanks for the differential fuzzer suggestion, it totally makes sense to guarantee parity on behavior with the reference implementation we want to match.

Regarding having separated implementation for streamed decoding w.r.t. to current image bmp decoding, this is a conservative approach I propose because currently Chromium is using its custom BMP C++ implementation from blink, so we are expecting also some gaps/differences on current behavior for Chromium implementation w.r.t. current Image crate BMP implementation. As we will integrate the new streamed version into Chromium, having it separated may allow us to have more flexibility to converge and meet Chromium requirements without breaking current users of image crate. After the convergence with Chromium functionality is achieved, then it may make totally sense for streamed decoding implementation to replace/merge with traditional implementation, deciding which changes, may be "breaking" (if any) and thus only suitable for a major release change and which are safe to port, etc...

Also, we may want to consider in a follow up to add support for streamed/incremental decoding in a generic way from ImageDecoder (or some sister structure) if this approach may fit other codecs (for example, we also have in the backlog to bring rust ICO implementation to Chromium through image crate also, which will have also the requirement of having support for streamed decoding)

[telecos]

As a reference of the usage of new API, I created a draft CL in skia to add support for streamed bmp decoding through the new API:

https://skia-review.googlesource.com/c/skia/+/1138796

There will be another CL in Chromium making use of the streamed decoding support from skia but the user of image crate API for Chromium use case will be skia.

[telecos]

Regarding spotted issue on bitmask not being properly parsed for V4/V5, you are totally right. Current implementation validation is based mainly on the test "test_streaming_vs_traditional" which compares the decoded content for both current and new streamed decoder. This spotted a gap that we don't have a test file covering this. I will craft some sample file so we can also have coverage in this file.