Fix BMP header size #912
Open
Fix BMP header size #912
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Ubuntu and macOS CI failures are unrelated. |
cmb69
added a commit
to cmb69/php-src
that referenced
this pull request
Dec 24, 2024
cmb69
commented
Dec 25, 2024
|
The CI failures are unrelated again (note that there are occassional segfaults on MinGW for different tests; that might justify some further investigation). |
The `bfSize` member of `BITMAPFILEHEADER` is supposed to store the size of the BMP[1], and as such needs to be equal to the file size (for BMPs stored as files). Otherwise that may trip up readers; at least they may assume that the BMP is corrupted; e.g. ImageMagick warns about this. While for compressed BMP images we actually calculate the size as we go, for uncompressed BMP images we calculate the size in advance, but so far did not account for padding of the rows. We also fix the `biSizeImage` of the `BITMAPINFOHEADER`[2] accordingly. [1] <https://learn.microsoft.com/en-us/windows/win32/api/wingdi/ns-wingdi-bitmapfileheader> [2] <https://learn.microsoft.com/en-us/windows/win32/api/wingdi/ns-wingdi-bitmapinfoheader>
We need to cater to compressed images, too. We also augment the test case to check the `biSizeImage` field, and also do the checks for compressed images.
|
Rebased and force-pushed to get CI green (at least Ubuntu should be fixed in the meantime). |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The
bfSizemember ofBITMAPFILEHEADERis supposed to store the size of the BMP[1], and as such needs to be equal to the file size (for BMPs stored as files). Otherwise that may trip up readers; at least they may assume that the BMP is corrupted; e.g. ImageMagick warns about this.While for compressed BMP images we actually calculate the size as we go, for uncompressed BMP images we calculate the size in advance, but so far did not account for padding of the rows.
We also fix the
biSizeImageof theBITMAPINFOHEADER[2] accordingly.[1] https://learn.microsoft.com/en-us/windows/win32/api/wingdi/ns-wingdi-bitmapfileheader
[2] https://learn.microsoft.com/en-us/windows/win32/api/wingdi/ns-wingdi-bitmapinfoheader