Can't read some 16-bit bitmaps

[Enyium]

I tried to create a 16-bit bitmap and searched for an online converter. I found https://online-converting.com/image/convert2bmp/, which can convert to RGB565. But the file produced by it couldn't be read by tinybmp: I got ParseError::UnexpectedEndOfFile. Because of that, I later converted to RGB565 using GIMP, which worked. But since the format produced by online-converting.com seems to be valid according to my inspection of it using ImHex, which can decode and visualize the bitmap struct fields, and because XnView can display it, I'm posting this issue.

How to create an RGB565 bitmap with online-converting.com:

• Go to https://online-converting.com/image/convert2bmp/.
• Deselect the checkbox "Automatically start" at the bottom of the form before uploading any file.
• In the "Color" drop-down box, choose "16 (5:6:5, RGB Hi color)".
• Upload a file.
• Click "Convert".

Here's an image I converted: 5x1px-16bpp-rgb565.bmp.

The file uses the BITMAPINFOHEADER. For it's biCompression member, which contains BI_BITFIELDS, Microsoft documents:

Value	Meaning
BI_RGB	Uncompressed RGB.
BI_BITFIELDS	Uncompressed RGB with color masks. Valid for 16-bpp and 32-bpp bitmaps.

[...]

For 16-bpp bitmaps, if biCompression equals BI_RGB, the format is always RGB 555. If biCompression equals BI_BITFIELDS, the format is either RGB 555 or RGB 565. Use the subtype GUID in the AM_MEDIA_TYPE structure to determine the specific RGB type.

Since we're not decoding video and there's no AM_MEDIA_TYPE, this isn't very helpful. But there are hints about how to handle BI_BITFIELDS when only having BITMAPINFOHEADER:

• In an old Windows CE documentation for BITMAPINFOHEADER, it says under biCompression:

Value	Description
BI_RGB	An uncompressed format.
BI_BITFIELDS	Specifies that the bitmap is not compressed and that the color table consists of three DWORD color masks that specify the red, green, and blue components of each pixel. This is valid when used with 16- and 32-bpp bitmaps.

• In the "Color table" section of Wikipedia's "BMP file format" article, its says:

The color table (palette) occurs in the BMP image file directly after the BMP file header, the DIB header, and after the optional three or four bitmasks if the BITMAPINFOHEADER header with BI_BITFIELDS (12 bytes) or BI_ALPHABITFIELDS (16 bytes) option is used. Therefore, its offset is the size of the BITMAPFILEHEADER plus the size of the DIB header (plus optional 12-16 bytes for the three or four bit masks).

I didn't dive into your code in detail, but your src/header/dib_header.rs only knows HeaderType::Info and HeaderType::V3 and not HeaderType::V2, and it has this bit of code:

        let (_dib_header_data, channel_masks) = if header_type.is_at_least(HeaderType::V3)
            && matches!(compression_method, CompressionMethod::Bitfields)
        {
            let (dib_header_data, mask_red) = try_const!(le_u32(dib_header_data));
            let (dib_header_data, mask_green) = try_const!(le_u32(dib_header_data));
            let (dib_header_data, mask_blue) = try_const!(le_u32(dib_header_data));
            let (dib_header_data, mask_alpha) = try_const!(le_u32(dib_header_data));

            (
                dib_header_data,
                Some(ChannelMasks {
                    red: mask_red,
                    green: mask_green,
                    blue: mask_blue,
                    alpha: mask_alpha,
                }),
            )
        } else {
            (dib_header_data, None)
        };

But the "DIB header (bitmap information header)" section of the Wikipedia article also lists a BITMAPV2INFOHEADER, which only added "RGB bit masks," whereas, the BITMAPV3INFOHEADER added the single additional "alpha channel bit mask." So, it seems, tinybmp needs to differentiate between just interpreting the first 3 vs. the first 4 RGBQUADs from BITMAPINFO. Adding HeaderType::V2 seems to be incorrect, because your code reads dib_header_length, which would still be sizeof(BITMAPINFOHEADER). As a quick assessment, you'd need to remove the first part of the condition if header_type.is_at_least(HeaderType::V3) && matches!(compression_method, CompressionMethod::Bitfields) and then only retrieve mask_alpha, if header_type.is_at_least(HeaderType::V3) is true.

[rfuest]

I did take a quick look at this and I think the issue is caused by the biSizeImage field. The BMP file you provided sets that field as the file size and TinyBMP expects it to be the size of the image data without the header.

If I change this line to allow the data to be truncated instead of giving an UnexpectedEndOfFile error the file is displayed correctly:

tinybmp/src/raw_bmp.rs

Lines 63 to 65 in 4b307a8

	// Believe what the bitmap tells us rather than multiplying width by
	// height by bits-per-pixel, because the image data might be compressed.
	header.image_data_len as usize

The interpretation of the size field has caused issues in the past and there needs to be some more testing with files from other sources to ensure it's handled correctly.

[Enyium]

Oh, right; biSizeImage is incorrect. With a biCompression of BI_BITFIELDS, the file is also uncompressed and thus deterministic. So, tinybmp should also be able to calculate the image size in that case instead of trusting the field. Currently, it's only calculating the size itself when having CompressionMethod::Rgb.

But my previous statements regarding needing to differentiate between 3 and 4 channel mask u32s still seem to be relevant, given what can be researched about the BMP format. I don't know how you changed the line you referenced, but if you just inserted the length of the image data FF FF | 00 00 | FF FF | 00 00 | FF FF | 97 90, you ignored that the 3 channel masks 00 F8 00 00 | E0 07 00 00 | 1F 00 00 00 would be read as 4 by the current code, which would skip the initial image data FF FF | 00 00 (and erroneously interpret it as an alpha channel mask).

[rfuest]

Oh, right; biSizeImage is incorrect. With a biCompression of BI_BITFIELDS, the file is also uncompressed and thus deterministic. So, tinybmp should also be able to calculate the image size in that case instead of trusting the field. Currently, it's only calculating the size itself when having CompressionMethod::Rgb.

I agree. biSizeImage should be ignored if header.compression_method.is_compressed() (from #55) is false.

But my previous statements regarding needing to differentiate between 3 and 4 channel mask u32s still seem to be relevant, given what can be researched about the BMP format. I don't know how you changed the line you referenced, but if you just inserted the length of the image data FF FF | 00 00 | FF FF | 00 00 | FF FF | 97 90, you ignored that the 3 channel masks 00 F8 00 00 | E0 07 00 00 | 1F 00 00 00 would be read as 4 by the current code, which would skip the initial image data FF FF | 00 00 (and erroneously interpret it as an alpha channel mask).

I only changed data_length in RawBmp::from_slice to not overflow the available data. This worked because the location of the image data in the BMP file is based on the bfOffBits field in the BITMAPFILEHEADER and not on the length of the header. But the channel masks would be ignored and a proper fix should read the 3 masks for header versions less than 3.