Remove all previous hack code. Create a SDWebImageFLCoder to decode just FLAnimatedImage for GIF.

[dreampiggy]

This PR, contains a possible solution for SDWebImage/SDWebImage#2404

Reason

We previouslly, in SDWebImage 4.x, have no choice to hack and create FLAnimatedImage during setImageBlock. However, it's a wrong solution, whatever from the architecture or the task side.

The decoding process, should always happend on the Coder level, which receive a compressed image data and generate image representation. Though FLAnimatedImageView only accept FLAnimatedImage, which is not a UIImage subclass. However, we use associated object to bind it on a normal UIImage, so this can solve the problem.

Design

We should move the decoding code, into a single custom coder, called SDWebImageFLCoder. Which will produce a UIImage which has a FLAnimatedImage associated to it. Quite simple.

However, in order to not affect the normal image decoding process on normal UIImageView or SDAnimatedImageView. We should only filter that the orignal image request for GIF, is come from FLAnimatedImageView. So we need the support from SDImageCoderOption, to pass the context arg (Introduced in 5.0.0-beta) to the coder, and check it.

Another consideration, from 4.x, we should move those args to create FLAnimatedImage, like predrawEnabled, optimalCacheSize, to the context option level. Because it's related to FLAnimatedImage level, but not FLAnimatedImageView level.

Implemenatation

@interface SDWebImageFLCoder : NSObject <SDImageCoder>
@property (nonatomic, class, readonly, nonnull) SDWebImageFLCoder *sharedCoder;
@end

@implemenation SDWebImageFLCoder
- (UIImage *)decodedImageWithData:(NSData *)data options:(SDImageCoderOptions *)options {
    if (original request from FLAnimatedImageView) {
        FLAnimatedImage *animatedImage = [[FLAnimatedImage alloc] initWithAnimatedGIFData:data];
        return [UIImage sd_imageWithFLAnimatedImage:animatedImage];
    } else {
        return [SDImageGIFCoder.sharedCoder decodedImageWithData:data options:options];
    }
}

@end

What for user: Remember to add SDWebImageFLCoder, to the coders manager at the beginning, ensure its order is higher than SDImageGIFCoder. (in AppDelegate or somewhere)

[zhongwuzw]

Just some thoughts after I looked coder mechanism.
The coder mechanism is sufficient currently? Maybe we can add the url parameter and a context option which can specify the coder class per image.

[dreampiggy]

@zhongwuzw You can pass anything into context, and get the context back. This is a context pattern (The context arg pass from really top-level API to the bottom). For example, pass the url during sd_setImageWithURL:

[imageView sd_setImageWithURL:url placeholderImage:nil context:@{MyURLKey: url}];
// Then you can get the `MyURLKey` from custom coder
SDWebImageContext *context = options[SDImageCoderWebImageContext];
NSURL *url = context[MyURLKey];

Actually it's a bad design. A image coder, it's designed to solve the image decoding task, but not any business logic related to things other than decoding. However, we leave it as a hackable space, to enable some advanced user use this to do extra logic.

All built-in coders, should not touch SDWebImageContext during image decoding, if new option is requsted, update SDImageCoderOptions

[zhongwuzw]

@dreampiggy Emm, I mean bring the context ahead to - (BOOL)canDecodeFromData:(NSData *)data, which can determine wether or not can handle image in advance.

[dreampiggy]

@zhongwuzw That too much. Your image coder should only judge the image format and choose to response it or not. Like Image/IO using UTType to determine which underneath plugin (AppleJPEGPlugin ? PNGPlugin ?) should use. So it should be been passed with the SDImageCoderOptions arg.

[zhongwuzw]

@dreampiggy fine, no big difference.

[zhongwuzw]

Compare String is not sufficient, Class would be better, because user maybe subclass FLAnimatedImageView.

[zhongwuzw]

The same.

[dreampiggy]

What's the meaning ?

[zhongwuzw]

We don't support subclass.

@interface SubclassOfFLAnimatedImageView : FLAnimatedImageView
@end

[zhongwuzw]

Oops, this line don't needs to consider subclass. Just this line.

[dreampiggy]

@zhongwuzw Added one commit to support subclass.

Anyway, most of users does not use subclass of FLAnimatedImageView. However, this is possible and support on it is easy

[zhongwuzw]

Yeah, it's true, normally, maybe only big companies would do some encapsulation, they maybe would use AFNetWorking、SDWebImage...., but would not use it in code directly, the mainly reason is keep the third parties in control, e.x., if third framework changes API or definitely incompatible, they only need change the part of encapsulation, no hurt to the entire code base.

[zhongwuzw]

So I'm sensitive to these things. The class would always better than direct String compare. 😂

[zhongwuzw]

Needs to change UIImageOrientationUp to posterImage. imageOrientation?

[dreampiggy]

GIF does not support EXIF orientation. It's impossible that a GIF compressed image data, can been decoded to a rotated UIImage, until that GIF decoder contains bug.

But anyway, if you desire, I can change this with the posterImage.imageOrientation

[dreampiggy]

@zhongwuzw Any other consideration about it ? Can I squash and merge this PR ?

[zhongwuzw]

@dreampiggy TBO, I don't much like handle like this, it's a little bit non-decouple very well, my opinion is if SDWebImageFLCoder or any other coder can't create the image, just return nil, SDImageCodersManager can check if it is nil, it would enumerate next coder to check wether can decode. I can make a PR if you think it deserve. 😂
What's your opinion? 🤔

[dreampiggy]

@zhongwuzw The image coder protocol, it's not designed for SDImageCodersManager...SDImageCodersManager is just a implmentation based on that protocol to provide multiple coder system. And, these two methods we talk about, does not represent the same step during image decoding. I can describe why we need both of two.

The decodedImageWithData:options: return nil, does not means : This coder can not support this compressed image foramt. You should realize this meaning.

For example, during image decoding, you are trying to parse the information from binary data for target image format (EXIF, etc), but it failed.
For example, during allocation of large memory for bitmap buffer, the allocation failed.
For example, during animation frame looping, there are one frame can not been decoded and it failed.

These cases, should not call another coder to process the samething. Because it's a internal error happend for that coder, but not image foramt.

However, the canDecodeFromData: it's about whether this coder can have ability to decode target image format. Actually it should be named with canDecodedFromFormat:, but using a NSData * make it more extensible because we can't not provide all SDImageForamt or list all the File Signature for image format in code.

So, I still propose to keep these two protocol seperate. The decodedImageWithData can have a options arg which about the options for image decoding. However, the canDecodeFromData: should not contains the options arg. It's only things about whether this coder can support this image format, but not whether current system status and data can allow coder to successfully produce a UIImage.

[zhongwuzw]

@dreampiggy 👍 Yeah, I know your opinion. Maybe most of the user would not use multiple coder for each image format. e.x. we have SDImageGIFCoder and SDWebImageFLCoder for the same GIF format.
When I see these code, I just feel that SDWebImageFLCoder shouldn't know what SDImageCodersManager is, it just needs to decode data IMO.

My opinion for SDImageCodersManager is it would manage coder group by image format. Other words, SDWebImageFLCoder and SDImageGIFCoder is a tuple, maybe SDImageAPNGCoder and Custom APNGCoder is an another tuple. SDImageCodersManager would try image in specific image coder group, like GIF, SDImageCodersManager would try decode in SDWebImageFLCoder firstly, because it's added by user, if it return nil, would try SDImageGIFCoder next. And also we can provide options to let user re-disable this behavior.

At last. It's also good to keep the current implementation.

[zhongwuzw]

@dreampiggy I suddenly realized that maybe Apple do the same as my thought. Apple has a manager to figure out what the image format the data is, if it is JPEG, it would call JPEGPlugin to decode, if it is PNG, it would call PNGPlugin. Other than enumerate all plugins to ask them wether they can decode the data. Just like broadcast vs peer to peer.

[dreampiggy]

@zhongwuzw No. ImageIO is just a Core Foundation wrapper for the C++ framework IIOReadPlugin && IIOWritePlugin. It actually use the same design from SDWebImage's coder. They still need to enumerate each plugins.

IIOReadPlugin use a array (std::vector) to store each of plugins (Which subclass of IIOGeneric_Reader abstract class). And for each plugin, call their handle method instead. When you call CGImageSourceCreateWithXXX, it will try to bind the Core Foundation CGImageSourceRef instance to a C++ IIOReadPlugin plugin internally (Just like a proxy).

The good point it's that Apple can hard-coded all the plugins into single files because they are an entire framework, but we can't. We open the posibility to let other third-party developers to provide a coder plugin. So this is why we can't do the same thing to hold a central check of UTType and choose which coder should been used.

If you're interested in the detailed implementation, You can use Hopper to deassemble ImageIO.framework. Here is just some snippet from my previous experience.

[dreampiggy]

For example, they register all plugins during IIO_ReaderHandler::buildPluginList() lazy init method, like we recommend user to register coders to SDImageCodersManager at lauch time.

int __ZN17IIO_ReaderHandler15buildPluginListEv() {
    r13 = rdi;
    if (*_gIIODebugFlagsInitializer != 0xffffffffffffffff) {
            dispatch_once(_gIIODebugFlagsInitializer, ^ {/* block implemented at ____ZN17IIO_ReaderHandler15buildPluginListEv_block_invoke */ } });
    }
    r14 = 0x0;
    var_38 = r14;
    var_34 = r14;
    if (dyld_process_is_restricted() == 0x0) {
            r14 = getenv("RAWCAMERA_BUNDLE_PATH");
    }
    if ((*(int8_t *)0x539f42 & 0x4) == 0x0) {
            if (r14 != 0x0) {
                    r15 = dlopen(r14, 0x1);
                    if (r15 == 0x0) {
                            r15 = dlopen("/System/Library/CoreServices/RawCamera.bundle/RawCamera", 0x1);
                            if (r15 != 0x0) {
                                    rcx = dlsym(r15, "GetRawPluginsInfo");
                                    if (rcx != 0x0) {
                                            rax = (rcx)(&var_38, &var_34);
                                    }
                                    else {
                                            rax = 0x0;
                                    }
                                    var_40 = rax;
                                    *_gReadMakerNoteProps = dlsym(r15, "ReadMakerNoteProps");
                            }
                            else {
                                    var_40 = 0x0;
                            }
                    }
                    else {
                            rcx = dlsym(r15, "GetRawPluginsInfo");
                            if (rcx != 0x0) {
                                    rax = (rcx)(&var_38, &var_34);
                            }
                            else {
                                    rax = 0x0;
                            }
                            var_40 = rax;
                            *_gReadMakerNoteProps = dlsym(r15, "ReadMakerNoteProps");
                    }
            }
            else {
                    r15 = dlopen("/System/Library/CoreServices/RawCamera.bundle/RawCamera", 0x1);
                    if (r15 != 0x0) {
                            rcx = dlsym(r15, "GetRawPluginsInfo");
                            if (rcx != 0x0) {
                                    rax = (rcx)(&var_38, &var_34);
                            }
                            else {
                                    rax = 0x0;
                            }
                            var_40 = rax;
                            *_gReadMakerNoteProps = dlsym(r15, "ReadMakerNoteProps");
                    }
                    else {
                            var_40 = 0x0;
                    }
            }
    }
    else {
            var_40 = 0x0;
    }
    if (*IIO_ReaderHandler::UseAppleJPEG()::appleJPEGCheck != 0xffffffffffffffff) {
            dispatch_once(IIO_ReaderHandler::UseAppleJPEG()::appleJPEGCheck, ^ {/* block implemented at ____ZN17IIO_ReaderHandler12UseAppleJPEGEv_block_invoke */ } });
    }
    if (*(int8_t *)IIO_ReaderHandler::UseAppleJPEG()::gUseAppleJPEGPlugin != 0x0) {
            rax = CreateReader_AppleJPEG();
    }
    else {
            rax = CreateReader_JPEG();
    }
    var_30 = rax;
    if (rax != 0x0) {
            rcx = *(r13 + 0x18);
            if (rcx != *(r13 + 0x20)) {
                    *rcx = rax;
                    *(r13 + 0x18) = *(r13 + 0x18) + 0x8;
            }
            else {
                    void std::__1::vector<IIO_Reader*, std::__1::allocator<IIO_Reader*> >::__push_back_slow_path<IIO_Reader* const>(r13 + 0x10);
            }
    }
    rax = CreateReader_PNG();
    var_30 = rax;
    if (rax != 0x0) {
            rcx = *(r13 + 0x18);
            if (rcx != *(r13 + 0x20)) {
                    *rcx = rax;
                    *(r13 + 0x18) = *(r13 + 0x18) + 0x8;
            }
            else {
                    void std::__1::vector<IIO_Reader*, std::__1::allocator<IIO_Reader*> >::__push_back_slow_path<IIO_Reader* const>(r13 + 0x10);
            }
    }
    rax = CreateReader_GIF();
    var_30 = rax;
    if (rax != 0x0) {
            rcx = *(r13 + 0x18);
            if (rcx != *(r13 + 0x20)) {
                    *rcx = rax;
                    *(r13 + 0x18) = *(r13 + 0x18) + 0x8;
            }
            else {
                    void std::__1::vector<IIO_Reader*, std::__1::allocator<IIO_Reader*> >::__push_back_slow_path<IIO_Reader* const>(r13 + 0x10);
            }
    }
    r12 = var_34;
    if (r12 > 0x0) {
            r15 = r13 + 0x10;
            rbx = 0x0;
            do {
                    rax = operator new(0x60);
                    rcx = *(var_40 + rbx * 0x8);
                    xmm0 = intrinsic_movups(xmm0, *(int128_t *)rcx);
                    rdx = *(rcx + 0x10);
                    rsi = *(rcx + 0x60);
                    *(int128_t *)(rax + 0x8) = intrinsic_movups(*(int128_t *)(rax + 0x8), xmm0);
                    *(rax + 0x18) = rdx;
                    *(rax + 0x20) = rsi;
                    *(int8_t *)(rax + 0x28) = 0x1;
                    *rax = 0x527c18;
                    xmm0 = intrinsic_movups(xmm0, *(int128_t *)(rcx + 0x18));
                    *(int128_t *)(rax + 0x30) = intrinsic_movups(*(int128_t *)(rax + 0x30), xmm0);
                    xmm0 = intrinsic_movups(xmm0, *(int128_t *)(rcx + 0x28));
                    *(int128_t *)(rax + 0x40) = intrinsic_movups(*(int128_t *)(rax + 0x40), xmm0);
                    xmm0 = intrinsic_movups(xmm0, *(int128_t *)(rcx + 0x38));
                    *(int128_t *)(rax + 0x50) = intrinsic_movups(*(int128_t *)(rax + 0x50), xmm0);
                    var_30 = rax;
                    rcx = *(r13 + 0x18);
                    if (rcx != *(r13 + 0x20)) {
                            *rcx = rax;
                            *(r13 + 0x18) = *(r13 + 0x18) + 0x8;
                    }
                    else {
                            void std::__1::vector<IIO_Reader*, std::__1::allocator<IIO_Reader*> >::__push_back_slow_path<IIO_Reader* const>(r15);
                            r12 = var_34;
                    }
                    rbx = rbx + 0x1;
            } while (rbx < sign_extend_64(r12));
    }
    rax = CreateReader_TIFF();
    var_30 = rax;
    if (rax != 0x0) {
            rcx = *(r13 + 0x18);
            if (rcx != *(r13 + 0x20)) {
                    *rcx = rax;
                    *(r13 + 0x18) = *(r13 + 0x18) + 0x8;
            }
            else {
                    void std::__1::vector<IIO_Reader*, std::__1::allocator<IIO_Reader*> >::__push_back_slow_path<IIO_Reader* const>(r13 + 0x10);
            }
    }
    rax = CreateReader_JP2();
    var_30 = rax;
    if (rax != 0x0) {
            rcx = *(r13 + 0x18);
            if (rcx != *(r13 + 0x20)) {
                    *rcx = rax;
                    *(r13 + 0x18) = *(r13 + 0x18) + 0x8;
            }
            else {
                    void std::__1::vector<IIO_Reader*, std::__1::allocator<IIO_Reader*> >::__push_back_slow_path<IIO_Reader* const>(r13 + 0x10);
            }
    }
    // many lines of add plugin
    // many lines of add plugin
    // many lines of add plugin
    rax = *(r13 + 0x10);
    rcx = *(r13 + 0x18);
    if (rax != rcx) {
            rdx = *(r13 + 0x8);
            do {
                    rsi = rdx;
                    rdx = *rax;
                    rdx = *(rdx + 0x18);
                    CMP(rsi, rdx);
                    asm { cmova      rdx, rsi };
                    rax = rax + 0x8;
            } while (rcx != rax);
            *(r13 + 0x8) = rdx;
    }
    return rax;
}

[dreampiggy]

For example, they do for-loop to check first 16-bytes of File Signature (See above), to determine which plugin should be binded for current CGImageSource during IIO_ReaderHandler::readerForBytes(). Each plugins contains their UTType, so they just need to parse the bytes to UTType, and then enumerate the array to check which plugin should be binded.

There are also some other check based on the UTType string in IIO_ReaderHandler::readerForUTType(), or even based on the filename extension like IIO_ReaderHandler::readerForPathExtension()

[zhongwuzw]

@dreampiggy Maybe you are more familiar than me at ImageIO, I just see it start last hours before 😂 . Am I missed something? I profile using Instruments first, the result is below, IIOImageSource::createImageAtIndex would use CGImageSource to create CGImageRef, then I trace the steps, I didn't find any for loop to find the plugin. I can step into it more detail carefully, may I miss anything?

I use CGImageSourceCreateWithData and CGImageSourceCreateImageAtIndex to create image.

303.00 ms   22.2%	303.00 ms	 	 aj_icol_mcurow_cmyk
303.00 ms   22.2%	0 s	 	  aj_bufferproc_crop
303.00 ms   22.2%	0 s	 	   aj_decode_all
303.00 ms   22.2%	0 s	 	    aj_decode_all_mt
303.00 ms   22.2%	0 s	 	     applejpeg_decode_image_all
303.00 ms   22.2%	0 s	 	      AppleJPEGReadPlugin::copyImageBlockSet(InfoRec*, CGImageProvider*, CGRect, CGSize, __CFDictionary const*)
303.00 ms   22.2%	0 s	 	       AppleJPEGReadPlugin::CopyImageBlockSetProc(void*, CGImageProvider*, CGRect, CGSize, __CFDictionary const*)
303.00 ms   22.2%	0 s	 	        IIOImageProviderInfo::copyImageBlockSetWithOptions(CGImageProvider*, CGRect, CGSize, __CFDictionary const*)
303.00 ms   22.2%	0 s	 	         IIOImageProviderInfo::CopyImageBlockSetWithOptions(void*, CGImageProvider*, CGRect, CGSize, __CFDictionary const*)
303.00 ms   22.2%	0 s	 	          CGImageProviderCopyImageBlockSetWithOptions
159.00 ms   11.6%	0 s	 	           invocation function for block in IIOReadPlugin::cacheImmediately(__CFDictionary const*, CGImage*)
159.00 ms   11.6%	0 s	 	            _dispatch_client_callout
159.00 ms   11.6%	0 s	 	             _dispatch_queue_barrier_sync_invoke_and_complete
159.00 ms   11.6%	0 s	 	              IIOReadPlugin::cacheImmediately(__CFDictionary const*, CGImage*)
159.00 ms   11.6%	0 s	 	               IIOImageSource::createImageAtIndex(unsigned long, IIODictionary*)
159.00 ms   11.6%	0 s	 	                CGImageSourceCreateImageAtIndex

[dreampiggy]

@zhongwuzw ...Paste many deassemble code here seems no use, and here on GitHub, maybe it's not a good place to talk about it. You can just use Hopper, search ImageIO.framework with that symbols.

Each plugins, in IIO_ReaderHandler::buildPluginList() (See above), call their create method, like CreateReader_HEIC(). In the CreateReader_HEIC(), it allocate a new plugin which is subclass of IIOGeneric_Reader. Then register the plugin with their desired UTType (For HEIC, it's the _kCGImageTypeIdentifierHEIC), and finally push back into the global std::vector.

During CGImageSourceGetCount / CGImageSourceCreateImageAtIndex and all the process method, it will lazy check whether a plugin has been binded to CGImageSource using IIOImageSource::bindToReader(). If not, it will call IIO_ReaderHandler::readerForBytes() once (For non-progressive decoding) to check File Signature, convert to UTType, and finally bind the plugin that match the UTType.

I don't know why you care about the loop. It just use a CFStringCompare and 16 Bytes hex check. But use a switch case will cause the code harder to maintain. (Consider a switch case contains 100 cases ?). Don't assume anything on performance until you do actual profile.

And anyway, it's just a personal experience, and deassemble code may not been considered a legal way of understanding the internal behavior for their framework. If you're interested in, just do yourself. Don't need to append more un-related things under this issue.

int __ZN17IIO_ReaderHandler15readerForUTTypeEPK10__CFString(void * arg0) {
    r15 = rsi;
    r12 = arg0;
    rbx = *(r12 + 0x10); 
    if (rbx == *(r12 + 0x18)) goto loc_10cc0e;

loc_10cbe5:
    r14 = 0x0;
    goto loc_10cbe8;

loc_10cbe8:
    if (CFStringCompare(**(*rbx + 0x8), r15, 0x0) == 0x0) goto loc_10cc13;

loc_10cc01:
    rbx = rbx + 0x8;
    if (rbx != *(r12 + 0x18)) goto loc_10cbe8;

loc_10cc16:
    rax = r14;
    return rax;

loc_10cc13:
    r14 = *rbx;
    goto loc_10cc16;

loc_10cc0e:
    r14 = 0x0;
    goto loc_10cc16;
}

[zhongwuzw]

@dreampiggy Yeah, final words 😂 ,thanks for your comments, I looked the disassembling code already, you said system would enumerate all plugins to try to decode each image data. I think it's not , maybe from your recent comment, it's exactly what I want, it has the bind mechanism, just like a Dictionary, one-to-one relationship, TBO, I don't much care the performance of for loop in our coder manager, because it's trivial, what I cared is the pattern, that is manager needs to know to call which coder, other than in some coder, it asks manager to try to decode again.

Fine, I already learn too much. Thanks again ! ❤️

[zhongwuzw]

LGTM