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

Fix decoding of RGB JPEG with Adobe APP14 marker #1444

Merged
merged 3 commits into from Jul 20, 2021

Conversation

hackermd
Copy link
Contributor

@hackermd hackermd commented Jul 17, 2021

Describe the changes

DICOM allows JPEG steams where the image has been directly compressed in RGB color space without first transforming the image into YCbCr color space. Pillow doesn't (and cannot!) decode the pixel data correctly, because it doesn't know whether a color transformation has been applied (in DICOM specified via Photometric Interpretation). We previously addressed this issue via #878.

In the meantime, pillow has become capable of detecting the Adobe APP14 marker segment in the JPEG header (which specifies the color transformation via the transform flag) and can now decode the JPEG stream correctly if the marker segment is present (see python-pillow/Pillow#5408). The change in decoding behavior now conflicts with the decoding logic implemented in pydicom.

This PR addressed this conflict. Specifically, if the Adobe APP14 marker segment is detected in the JPEG header by Pillow, pydicom lets Pillow handle the decoding and color transformation. If the marker is absent, pydicom will provide further information to Pillow to avoid an incorrect/unnecessary color transformation.

Tasks

  • Unit tests added that reproduce the issue or prove feature is working
  • Fix or feature added
  • Code typed and mypy shows no errors
  • Documentation updated (if relevant)
    • No warnings during build
    • Preview link (CircleCI -> Artifacts -> doc/_build/html/index.html)
  • Unit tests passing and overall coverage the same or better

@pep8speaks
Copy link

pep8speaks commented Jul 17, 2021

Hello @hackermd! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2021-07-18 15:08:34 UTC

@codecov
Copy link

codecov bot commented Jul 17, 2021

Codecov Report

Merging #1444 (3ed375a) into master (506ecea) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1444   +/-   ##
=======================================
  Coverage   96.97%   96.97%           
=======================================
  Files          65       65           
  Lines       10184    10185    +1     
=======================================
+ Hits         9876     9877    +1     
  Misses        308      308           
Impacted Files Coverage Δ
pydicom/pixel_data_handlers/pillow_handler.py 96.22% <100.00%> (+0.03%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 506ecea...3ed375a. Read the comment docs.

@scaramallion
Copy link
Member

Looks good, another thing I've been intending to do is check to see if the component names are 'R' 'G' and 'B'.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants