Skip to content
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

Correctly account for Cr and Cb subsampling for native YBR_FULL_422 #242

Merged
merged 2 commits into from
Nov 9, 2023

Conversation

CPBridge
Copy link
Collaborator

Addresses #235

Occasionally the YBR_FULL_422 photometric interpretation is used for native encoded pixels. When this occurs, the calculation of bytes per frame needs to account for the subsampling of the colour (Cr and Cb) channels. The logic is given here as:

Rows (0028,0010) * Columns (0028,0011) * Number of Frames (0028,0008) * 2

This seems to work correctly with the sample file shared in #235.

@CPBridge
Copy link
Collaborator Author

Note that tests are currently failing for reasons unrelated to this PR, see #243 . We should wait on #244 before proceeding with this PR

@CPBridge
Copy link
Collaborator Author

Note that issues #235 and #241 seem to be resolved by this change. However, there is more to think through before merging:

  • Dealing with the other subsampled photometric interpretations
  • Adding tests
  • Performing colour space conversion to RGB when colour correction is requested

@CPBridge CPBridge marked this pull request as ready for review November 9, 2023 13:41
@CPBridge
Copy link
Collaborator Author

CPBridge commented Nov 9, 2023

Added a test and pushing this out so that it makes it into the next release (0.22.0). I will try to fix the colour space issue soon, but for now users can retrieve the frames without colour correction and then use eg pillow to convert YBR to RGB

@CPBridge
Copy link
Collaborator Author

CPBridge commented Nov 9, 2023

Merging, since tests are failing for reasons unrelated to this PR

@CPBridge CPBridge merged commit 3bf759d into master Nov 9, 2023
2 of 11 checks passed
@CPBridge CPBridge deleted the bug/read_ybr_full_422 branch November 9, 2023 14:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant