-
Notifications
You must be signed in to change notification settings - Fork 122
ARW: Support new LJPEG compression on ILCE-7M4 #386
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
Closed
Closed
Changes from all commits
Commits
Show all changes
3 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
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
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
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.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we use the file crop in any other modes (uncompressed and lossy), but rather leave it to the settings from XML? The way it is suggested now, there will be a difference between the modes, in both final image size, and the location of top left pixel...
There is a small problem to handle there, as width/height for lossless is wrongly specified by Sony as integer number of tiles, and it didn't have to be... One easy way to handle this was to have always an absolute positive crop in the XML for all modes, but it was rejected...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I remove it then the original under history has black bars on the bottom right, although it is cropped out later in the default module history.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, I didn't mean remove the crop completely. But we'll have to find a way to provide the same crop as uncompressed/lossy mode (defined in data/cameras.xml), and that works for APC-C/S35 files as well.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@artemist I think this tag has the information of the maximum available raw image size that you actually want, e.g. for the 7M4:
@LebedevRI It looks like cameras.xml crop for uncompressed/lossy could be too conservative and we could have it match this by changing to "-8" (will check for other recent Sonys as well;
alternatively we could start recognizing ARW 4.0 files and go directly to this tag in all modesedit: tag not available in other modes)?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the correct behaviour.
Since we don't use the camera-specified crop,
i don't really see why we care what the EXIF says,
we are just going to re-use the existing cameras.xml-specified crop in that mode too,
unless we need a different one, like some Fuji's need.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because of this new lossless mode and tile size rounding up nonsense. This is actually the one thing that correlates it to the traditional modes and cameras.xml (unless you want to bloat cameras.xml with more entries, which I don't think is a good idea).
I'll provide an example if it's not clear.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@LebedevRI Ok, let's take the 7M4 here - in the traditional uncompressed and lossy modes it has
and in (tweaked) cameras.xml we put
<Crop x="0" y="0" width="-8" height="0"/>
, so the resulting raw image size is7032 4688
.In the new lossless mode it has this nonsensical round up to integer multiple of 512x512 tiles:
that produces huge black padding on the right and at the bottom, and then this new tag
See the pattern here?
So options for the lossless mode implementation to give us the same output size as uncompressed are 1) bloat cameras.xml with more entries just so we can have different crop numbers (and not just one; we'll need extra for APS-C outputs as remainder/padding after rounding up is different of course) or 2) just use the bloody 0x7038 tag as is because it is exactly what it means - maximum usable raw area, not "camera-specified crop".
I have verified all the ILCE-1/7M4/7SM3/7RM5 samples on RPU (dump using LibRaw's
unprocessed_raw
which also works for lossless) and everything matches up (w/ minor tweaks), and it also lines up perfectly for the APS-C mode of 7RM5 and cameras.xml crop for the corresponding uncompressed.And a final thing - this is not "crop according to EXIF", as we have just established. That would in fact be further cropping according to
which is what @artemist is doing now (and matching OOC JPEG size). I agree, this is not what we generally want or care about.