MCD: calculate channel count based on metadata, not pixel bytes #255
Conversation
This file contains 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
In some cases, the number of pixel bytes stored for a particular acquisition may not match the expected number of bytes, so calculating the channel count based on the stored number of pixel bytes will cause problems. Instead of throwing an exception, this sets the channel count to the number of channels linked to the acquisition, and logs that some image data may be blank. This more closely matches the behavior of MCDViewer (the vendor software).
This prevents incorrect data from being read when the number of stored pixel bytes is smaller than expected.
chris-allan
approved these changes
Aug 7, 2024
| for (int c=0; c<channels.size(); c++) { | ||
| Channel channel = channels.get(c); | ||
| if (channel.acqID == acq.id) { | ||
| acq.channelIndexes[channel.index] = c; | ||
| while (channel.index >= acq.channelIndexes.size()) { | ||
| acq.channelIndexes.add(-1); |
There was a problem hiding this comment.
Is this meant to signify anything different than null? Are we checking this value anywhere?
There was a problem hiding this comment.
That will get checked in line 291/292. If null is preferable, it would be easy enough to rework that check.
There was a problem hiding this comment.
A ha, gotcha. No, that's fine.
mabruce
approved these changes
Aug 7, 2024
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.
In some cases, the number of pixel bytes stored for a particular acquisition may not match the expected number of bytes, so calculating the channel count based on the stored number of pixel bytes will cause problems.
Instead of throwing an exception, this sets the channel count to the number of channels linked to the acquisition, and logs that some image data may be blank. This more closely matches the behavior of MCDViewer (the vendor software).
I have checked a few older datasets that do not exhibit the missing pixel bytes problem and have found no difference in behavior with this change, but that should be double-checked in addition to testing the broken file.
Adding @mabruce for functional review and @chris-allan for code review, but can reassign if time runs out this week.
As mentioned separately, #253 is the only thing merged since 0.9.3, so we should be OK to do a 0.9.4 as soon as this is merged.