support for image orientation

[n13]

fix for issue #316

[bpoplauschi]

@rs could you take a look at this? I'm a bit concerned about what this could break.

[rs]

Looks good

[bpoplauschi]

Ok, merging. Let's see how this goes. Thanks @n13

[n13]

awesome. fingers crossed ;)

[bpoplauschi]

@n13 could you please take a look at those crashes?

[n13]

Yes but not right now. Tomorrow.

On Thu, Jul 17, 2014 at 4:37 PM, Bogdan Poplauschi
notifications@github.com wrote:

@n13 could you please take a look at those crashes?

Reply to this email directly or view it on GitHub:
#764 (comment)

[n13]

Got a fix for this - apparently imageSource can be NULL. That's not in the Apple docs, but Apple's own demo code is checking for NULL. Sigh.
It would be great to get the image that caused the problem, and add it to the unit tests. Are there unit tests?

[n13]

Created a new pull request. Let me know if there's anything else I can do.

[bpoplauschi]

@n13 we have a limited amount of unit tests, we are working on increasing this coverage. I don't think we have images we know that can cause this crash.

[jdelStrother]

For what it's worth, I ran into this crash on 3.7.0, while rapidly scrolling a tableview containing UIImageViews, each of which was requesting a load from SDWebImage. I ended up at this stack trace :

#0  0x000000010e571f9f in CFRelease ()
#1  0x000000010a499d02 in +[UIImage(MultiFormat) sd_imageOrientationFromImageData:] at /Users/jon/Developer/iphone/Pods/SDWebImage/SDWebImage/UIImage+MultiFormat.m:63
#2  0x000000010a499b20 in +[UIImage(MultiFormat) sd_imageWithData:] at /Users/jon/Developer/iphone/Pods/SDWebImage/SDWebImage/UIImage+MultiFormat.m:34
#3  0x000000010a489960 in -[SDWebImageDownloaderOperation connectionDidFinishLoading:] at /Users/jon/Developer/iphone/Pods/SDWebImage/SDWebImage/SDWebImageDownloaderOperation.m:343
#4  0x000000010b5e536b in __65-[NSURLConnectionInternal _withConnectionAndDelegate:onlyActive:]_block_invoke ()
#5  0x000000010b498bdb in -[NSURLConnectionInternal _withConnectionAndDelegate:onlyActive:] ()
#6  0x000000010b498aec in -[NSURLConnectionInternal _withActiveConnectionAndDelegate:] ()
#7  0x000000010e381637 in ___ZN27URLConnectionClient_Classic26_delegate_didFinishLoadingEU13block_pointerFvvE_block_invoke ()
#8  0x000000010e37f802 in ___ZN27URLConnectionClient_Classic18_withDelegateAsyncEPKcU13block_pointerFvP16_CFURLConnectionPK33CFURLConnectionClientCurrent_VMaxE_block_invoke_2 ()
#9  0x000000010e590f74 in CFArrayApplyFunction ()
#10 0x000000010e2f23e7 in RunloopBlockContext::perform() ()
#11 0x000000010e2f2217 in MultiplexerSource::perform() ()
#12 0x000000010e2f203a in MultiplexerSource::_perform(void*) ()
#13 0x000000010e579d21 in __CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__ ()
#14 0x000000010e5795f2 in __CFRunLoopDoSources0 ()
#15 0x000000010e59546f in __CFRunLoopRun ()
#16 0x000000010e594d83 in CFRunLoopRunSpecific ()
#17 0x000000010e59f8c1 in CFRunLoopRun ()
#18 0x000000010a487651 in -[SDWebImageDownloaderOperation start] at /Users/jon/Developer/iphone/Pods/SDWebImage/SDWebImage/SDWebImageDownloaderOperation.m:103
#19 0x000000010b56cc0b in __NSOQSchedule_f ()
#20 0x000000010f8e072d in _dispatch_client_callout ()
#21 0x000000010f8ceeab in _dispatch_async_redirect_invoke ()
#22 0x000000010f8e072d in _dispatch_client_callout ()
#23 0x000000010f8d0b27 in _dispatch_root_queue_drain ()
#24 0x000000010f8d0d12 in _dispatch_worker_thread2 ()
#25 0x000000010fc7cef8 in _pthread_wqthread ()
#26 0x000000010fc7ffb9 in start_wqthread ()

By this point, self.imageData was nil, which is why imageSource ended up as NULL. self.expectedSize was set, however, so didReceiveResponse presumably got called.

Is it a race condition where one thread finishes downloading & calls connectionDidFinishLoading, and then another thread calls reset ?

[jdelStrother]

Actually, I can reproduce this pretty easily if I put a sleep(1) in connectionDidFinishLoading to exaggerate the race condition, and repeatedly calling sd_setImageWithURL from the main thread.

Possibly connectionDidFinishLoading should take a reference to imageData at the start (like it does with completionBlock), and bail if either are nil? Even that might not be sufficient though - is it ok calling done (and thus reset) at the end of connectionDidFinishLoading, when the main thread might have tried to start a new image loading in the middle of that method?

[bpoplauschi]

@jdelStrother could you please check with 3.7.1? This version is supposed to fix the crash. Fix was #819

[jdelStrother]

Yeah, 3.7.1 fixes the crash, though you get " ImageIO: CGImageSourceCreateWithData data parameter is nil" in the log. Still slightly concerned about race conditions in the latter half of connectionDidFinishLoading, but I can't reproduce any problems with it.

[bpoplauschi]

Hmm. You might be right, that a race condition still happens there. I'll open a new issue for that.

[dreampiggy]

It seems that this code is out of date. -[UIImage initWithData:] now can correctlly handler the EXIF orientation tag. It's no need to create another UIImage instance again. Maybe in the earily iOS version it doesn't. But now, above iOS 8, it works.

See the test for that: https://github.com/recurser/exif-orientation-examples (Just copy the image urls, uncomment that calculation of image EXIF orientation and run)

I'd suggest to remove this code to make ImageIO coder more clear and performent.

[n13]

If the code can be removed now, I am all for that.