-
-
Notifications
You must be signed in to change notification settings - Fork 244
JPEG: added support for extra data in APP0 section #417
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Apple Multi-Picture Format JPEGs often have 4 extra bytes in the APP0 section. The pattern now skips any extra bytes beyond the fixed APP0 data.
Well, technically it is not skipping the extra bytes. It creates a variable that ends in the pattern data window. To really skip bytes use the padding built-in data type. So if, for a given segment, its size must always be the value of |
No it is not skipping the bytes, it stores them in the xtra_data so it can be inspected. The extra data will be 0 length mostly (so no padding) and if so, the xtra_data data field will not be present. The proposed changes work just fine as it is. And APP0 is the only section currently known where a vendor added data, so there is no need to change more than that. If 'extensions' to other sections of the JPEG standard happen in the future, we should be aware of them and investigate what they mean instead of just ignore them. We'll have to addapt the pattern then. |
I interpreted this as meaning that your changes now skip the extra bytes because before they were not being read before hence my comment. It seems that you meant something else and that you actually want to display the values for inspection which seems like a contradiction. |
I edited the phrasing. It is actually really both. Previously the pattern failed to parse anything correctly beyond the APP0, so at least it needs to skip the extra bytes to parse the file correctly. The extra bytes have no special meaning except for certain Apple software probably. But it does not hurt to be able to inspect this "Apple was here" marker anyway. And I have encoutered Multi-Picture format JPEGs from Samsung mobile cameras as well. They did not have the extra APP0 data, so they parsed without an issue. But someday Samsung may get inspired by Apple 😉, so it is good to have that data inspectable. |
Whether we skip or display the extra bytes is really not that important considering we don't know what the extra data means. What is important, at least it seems to me, is that the jpeg pattern failing to parse the apple format is not because the pattern is lacking and needs patching, but because the format does not adhere to the jpeg specifications. In that light we should consider creating new patterns for formats that are jpeg like that are specifically designed for the format so for example we can visualize all the images contained in the file instead of just one which I think is all the jpeg format can visualize currently. Did the samsung mobile camera file showed all images as being visualizable when ImHex ran the jpeg pattern on it? Now that we can import patterns as modules, it is possible to import the jpeg pattern and use it to define a new pattern that has jpegs images included in it. I understand this would take more work than just reading the extra bytes so maybe this is something to think about in the long run if we want to implement more extensions. For now just adding the extra bytes should be a good start. |
Your assesment is wrong. The file is a JPEG, not an "Apple format". The JPEG APPn segments are explicitly for application specific information and the length field is there for a reason. By ignoring that field, it is the pattern that fails. The propoosed change is 100% backward compatible; it only improves parsing in certain circumstances where it is needed to comply with the JPEG structure. I don't get why you are so reluctant to improve the pattern. If so, forget it. I am perfectly happy with my local modification. |
how do you interpret ". For now just adding the extra bytes should be a good start." as reluctance to make changes? |
OK, time to pick this back up after my holidays. First of all:
You keep blaming the extension i.o. the pattern. The info page you mention states literally
So is the Apple Multiple Picture file. But the JPEG pattern fails to parse the file. That is why the pattern needs fixing. And that is what this PR does. The PR does not claim to parse the extra data added by the Apple Multi Picture JPEG extension, nor any other extension. |
Thanks a lot! |
Apple Multi-Picture Format JPEGs often have 4 extra bytes in the APP0 section. The pattern now parses any extra bytes beyond the fixed APP0 data.