Skip to content

Conversation

dreampiggy
Copy link
Contributor

New Pull Request Checklist

  • I have read and understood the CONTRIBUTING guide

  • I have read the Documentation

  • I have searched for a similar pull request in the project and found none

  • I have updated this branch with the latest master to avoid conflicts (via merge from master or rebase)

  • I have added the required tests to prove the fix/feature I am adding

  • I have updated the documentation (if necessary)

  • I have run the tests and they pass

  • I have run the lint and it passes (pod lib lint)

This merge request fixes / refers to the following issues: #2626

Pull Request Description

See #2626

In the 4.4.5 version, we add support to load WebP images' ICC Profile. However, current code seems does not check whether the ICC Profile color space is RGB. When using the color space other than RGB (such as Monochrome, CMYK), the CGImageCreate call will fail, so this cause the decoding failed.

So, we must do filter of the ICC Profile's color space before using. To ensure we only use RGB color mode to feed the CGImageCreate built-in API.

…r than RGB, which cause the CGImageCreate failed
@codecov-io
Copy link

Codecov Report

❗ No coverage uploaded for pull request base (master@bede907). Click here to learn what that means.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #2627   +/-   ##
=========================================
  Coverage          ?   75.19%           
=========================================
  Files             ?       37           
  Lines             ?     4072           
  Branches          ?        0           
=========================================
  Hits              ?     3062           
  Misses            ?     1010           
  Partials          ?        0
Impacted Files Coverage Δ
SDWebImage/SDWebImageWebPCoder.m 80.8% <0%> (ø)

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 bede907...8c3cfbd. Read the comment docs.

Copy link
Member

@kinarobin kinarobin left a comment

Choose a reason for hiding this comment

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

This can fix CGImageCreate failed bug, Thanks @dreampiggy.

@dreampiggy dreampiggy merged commit 46fae66 into SDWebImage:master Feb 26, 2019
@dreampiggy dreampiggy deleted the bugfix_webp_icc_monochrome branch February 26, 2019 09:50
if (colorSpaceRef) {
// `CGImageCreate` does not support colorSpace other than RGB (such as Monochrome), we must filter the colorSpace mode
CGColorSpaceModel model = CGColorSpaceGetModel(colorSpaceRef);
if (model != kCGColorSpaceModelRGB) {
Copy link
Member

Choose a reason for hiding this comment

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

I don't think CGImageCreate don't support Monochrome, two reasons leads created failed, one is we passed wrong bitsPerPixel, Monochrome has only one component, you can try CGImageRef imageRef = CGImageCreate(width, height, 8, 8, config.output.u.RGBA.stride, colorSpaceRef, bitmapInfo, provider, NULL, NO, renderingIntent); , at least, image can show proportional. Other reason, it's also why it's show proportional is we already decode it to RGB data.

image

Copy link
Contributor Author

@dreampiggy dreampiggy Feb 27, 2019

Choose a reason for hiding this comment

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

......Maybe you misunderstand something.

We ask libwebp, to output a RGB/RGBA bitmap buffer. Even it's original encoded with Monochrome(grey style) which contains 1 component. The ouput bitmap buffer is always 3/4 components (transformed by libwebp from Monochrome to RGB).

    config.output.colorspace = config.input.has_alpha ? MODE_rgbA : MODE_RGB; // <--- This cause libwebp to always ouput 3 or 4 components RGB bitmap buffer

If you supply 1 component to CGImageCreate, your dataProvider, actually is wrong. Because the bitmapBuffer is RGB(3 components), the colorSpace is Monochrome(1 component). It's also not correct..The correct way, is to supply either a Monochrome colorSpace + Monochrome bitmap data buffer, or a RGB colorSpace + RGB bitmap buffer, but not mix them together...

Actually, we can use ColorSync and vImage framework, to convert a ICC Profile from Monochrome to RGB, or even CMYK(YUV), but I really don't think it's necessary. 😓

We have never designed to be a professional image processing tools or decoding tools. If you build apps like Photoshop iOS and your customer is worry about the color sync, you have to write many many codes with professional color sync knowledge and low level APIs. This PR is just fix the problem of some rare WebP images on the modern Internet.

Copy link
Member

Choose a reason for hiding this comment

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

@dreampiggy ...Emm, maybe you don't get what I want to say 😂 , I only want to explain why CGImageCreate failed, there have two reasons. So the comments in this PR is wrong, I made a PR #2629 to update.

@ducnv2910
Copy link

Hello .

I can not play webP with this link: https://storage.googleapis.com/myscoop/scoops/webp/mtu1mdcxnju3ns40mju4ntuy_320.webp
- some : Error Domain=NSURLErrorDomain Code=-1001 "The request timed out." UserInfo={_kCFStreamErrorCodeKey=-2102, NSUnderlyingError=0x2837563d0 {Error Domain=kCFErrorDomainCFNetwork Code=-1001 "(null)" UserInfo={_kCFStreamErrorCodeKey=-2102, _kCFStreamErrorDomainKey=4}}, _NSURLErrorFailingURLSessionTaskErrorKey=LocalDataTask <18471379-5B42-4074-9C2F-18B5844EC0DF>.<14>, _NSURLErrorRelatedURLSessionTaskErrorKey=( "LocalDataTask <18471379-5B42-4074-9C2F-18B5844EC0DF>.<14>" ), NSLocalizedDescription=The request timed out., NSErrorFailingURLStringKey=https://storage.googleapis.com/myscoop/scoops/webp/mtu1mdcxnju3ns40mju4ntuy_320.webp, NSErrorFailingURLKey=https://storage.googleapis.com/myscoop/scoops/webp/mtu1mdcxnju3ns40mju4ntuy_320.webp, _kCFStreamErrorDomainKey=4}

@dreampiggy
Copy link
Contributor Author

@ducnv2910 Seems the error code is network, but not WebP decoding.

And I can successlly load this webp image url. Maybe it's something of your code that cause the issue ?

image

@ducnv2910
Copy link

@dreampiggy which version that you are using. I am using 4.4.6

And can you show me your code load webp.

Thank you so much

@dreampiggy
Copy link
Contributor Author

@ducnv2910 Using our SDWebImage repo's demo.
Change your image url in MasterViewController.m, Run "SDWebImage iOS Demo" target...

@nvduc2910
Copy link

Thanks. it works now.

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

Successfully merging this pull request may close these issues.

6 participants