Skip to content

If component id's are R, G, B in ASCII the color space should be RGB - #1732

Merged
brianpopow merged 6 commits into
masterfrom
bp/deduceColorSpaceFix
Aug 13, 2021
Merged

brianpopow merged 6 commits into
masterfrom
bp/deduceColorSpaceFix

Conversation

@brianpopow

@brianpopow brianpopow commented Aug 10, 2021 •

Copy link
Copy Markdown
Collaborator

Prerequisites

  • I have written a descriptive pull-request title
  • I have verified that there are no overlapping pull-requests open
  • I have verified that I am following the existing coding patterns and practice as demonstrated in the repository. These follow strict Stylecop rules 👮.
  • I have provided test coverage for my change (where applicable)

Description

I have noticed that the colorspace is determined wrongly for some jpeg. For example:
82846232-1929ea00-9ee8-11ea-8f25-53a72b14160c

is decoded as:
wrong_color

When the frame component Id's are RGB in ASCII, the color space should be RGB not YCbCr.

@brianpopow brianpopow changed the title WIP: If component id's are R, G, B in ASCII the color space should be RGB If component id's are R, G, B in ASCII the color space should be RGB Aug 10, 2021
@codecov

codecov Bot commented Aug 10, 2021 •

Copy link
Copy Markdown

Codecov Report

Merging #1732 (7e7dbbb) into master (d105ab4) will decrease coverage by 0.01%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1732      +/-   ##
==========================================
- Coverage   84.47%   84.46%   -0.02%     
==========================================
  Files         836      836              
  Lines       36735    36737       +2     
  Branches     4305     4306       +1     
==========================================
- Hits        31031    31029       -2     
- Misses       4873     4876       +3     
- Partials      831      832       +1     
Flag Coverage Δ
unittests 84.46% <100.00%> (-0.02%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs 91.37% <100.00%> (-1.04%) ⬇️

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 d105ab4...7e7dbbb. Read the comment docs.

@JimBobSquarePants

Copy link
Copy Markdown
Member

TIL! I did not know about this feature at all!

@JimBobSquarePants JimBobSquarePants added this to the 2.0.0 milestone Aug 13, 2021
}

// If the component Id's are R, G, B in ASCII the colorspace is RGB and not YCbCr.
if (components[0].Id == 82 && components[1].Id == 71 && components[2].Id == 66)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you switch the order of these checks I believe we avoid the bounds checks for 2 out of 3 of them.

@brianpopow
brianpopow merged commit 9b09775 into master Aug 13, 2021
@brianpopow
brianpopow deleted the bp/deduceColorSpaceFix branch August 13, 2021 14:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants