Refactoring & fix a bug & add tests to increase code coverage.

[ledyba-z]

Hi.

I separate SDWebImageAVIFCoder.m to 3 files. And also, I write some new tests to increase code coverage. After that, I found a crash bug about ICC profile and fix it.

Could you take a look?

[codecov]

Codecov Report

Merging #18 into master will increase coverage by 18.87%.
The diff coverage is 86.74%.

@@             Coverage Diff             @@
##           master      #18       +/-   ##
===========================================
+ Coverage   67.13%   86.01%   +18.87%     
===========================================
  Files           4        6        +2     
  Lines        1494     1494               
===========================================
+ Hits         1003     1285      +282     
+ Misses        491      209      -282     

Impacted Files	Coverage Δ
SDWebImageAVIFCoder/Classes/ColorSpace.m	84.97% <84.97%> (ø)
SDWebImageAVIFCoder/Classes/Conversion.m	87.62% <87.62%> (ø)
SDWebImageAVIFCoder/Classes/SDImageAVIFCoder.m	81.75% <100.00%> (+14.85%)	⬆️

---

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 501b702...27372a5. Read the comment docs.

[dreampiggy]

Wait me for review...

Does these method should be public APIs ? If not, you should move them into a private folder (for example, make folder strcuture looks SDWebImage/Classes/Private and update it for 3 different Packages:

• CocoaPods:

Modify Podspec and use s.private_header_files = 'SDWebImageAVIFCoder/Classes/Private/*.{h,m}'

• Carthage:

Update the Xcode's File Attribute and mark the ColorSpace.h into Project or Private.

• SwiftPM:

Update the Package.swift to only expose the publicHeadersPath arg point to the SDWebImageAVIFCoder/Classes/Public/*.

[dreampiggy]

Or I can do this myself after merging, since I'm more familiar with all these suck Package Manager :)

[ledyba-z]

Yeah, ColorSpace.h and Conversion.h must be private.

Or I can do this myself after merging, since I'm more familiar with all these suck Package Manager :)

Haha, thanks! I tried to do it myself... Could you check it?

[ledyba-z]

I think I can make ColorSpace.h and Conversion.h private, but tests became to fail because of it...

How should I do?

[dreampiggy]

Because you access the private header outside framework. In the test case:

You should use some tricks on the Test Case target, update the HEADER_SEARCH_PATH in SDWebImageAVIFCoder_Tests target (Open that Example/SDWebImageAVIFCoder.xcworkspace and choose the Test Target):

HEADER_SEARCH_PATH = $(inherited) ${PODS_ROOT}/Headers/Private

I can merge it and check each Package Manager and Project Settings again, to avoid some extra issues.

[dreampiggy]

These C function share the global symbol, which may cause conflict. Can we add some prefix here, like AVIFCreateColorSpaceMono ? Or use SDCreateAVIFColorSpaceMono this sounds better ?

And the visibility should be marked as __attribute__((visibility("hidden"))).

The same applies to others C function.

[ledyba-z]

Thanks, I renamed functions and added __attribute__((visibility("hidden"))), _Nonnull , _Nullable.

[dreampiggy]

@ledyba-z This final function name now is SDCreateAVIFCreateColorSpaceRGB, note there are two Create 🤣

Can this be SDCreateAVIFColorSpaceRGB ? Or like SDCreateColorSpaceRGBAVIF ?

If we follows the Core Foundation Naming, the Create words is important because this means the caller should call release or free the the function call result.

Actually I prefer the prefix one SDCreateAVIFXXXX but not the suffix one SDCreateXXXXAVIF, because it's more clear for the usage, like Apple's C funtion naming (they don't usually use suffix). But since this is a private API, actually it's OK...

[ledyba-z]

Oh! 😆
I missed two "Create" (it might be too long function names for me!)

I renamed the functions.

I see... thanks. > Core Foundation Naming

[ledyba-z]

Thanks to approve!

I will merge after CI passed.