ImageSharp
ImageSharp copied to clipboard
Add ANI decoder support
Prerequisites
- [x] I have written a descriptive pull-request title
- [x] I have verified that there are no overlapping pull-requests open
- [x] I have verified that I am following the existing coding patterns and practice as demonstrated in the repository. These follow strict Stylecop rules :cop:.
- [x] I have provided test coverage for my change (where applicable)
Description
implements AniDecoder for ImageSharp, and encoder if the pr approved
Comment
Since the ANI file may contain a two-dimensional array of ImageFrames, I flattened the original ImageFrames as follows
I have kept all Metadata as much as possible, but this may result in a structure that is not intuitive, and may require further discussion with you) reserved the possibility of future modifications
ANI is also a RIFF file, so I referenced RiffHelper, perhaps we should move it out of the WEBP namespace
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.
:white_check_mark: Poker-sang
:x: zxbmmmmmmmmm
You have signed the CLA already but the status is still pending? Let us recheck it.
Thanks @Poker-sang I’ll have a deep look at this asap.
Thank you for the review ❤ I fixed some simple review comments, but there is a lot of work that needs to continue to be discussed before we can move forward.
Based on your suggestions I'm guessing it may need to be improved in this way:
-
The rate block section could probably be included into
AniMetadata -
Include all members of
Ico/Cur/BmpMetadatainAniFrameMetadataand indicate which fields to use via flag. bmp is simpler, but forIco/Curframes could be specified usingSequenceNumberfor encode.
Thank you for the review ❤ I fixed some simple review comments, but there is a lot of work that needs to continue to be discussed before we can move forward.
Based on your suggestions I'm guessing it may need to be improved in this way:
- The rate block section could probably be included into
AniMetadata- Include all members of
Ico/Cur/BmpMetadatainAniFrameMetadataand indicate which fields to use via flag. bmp is simpler, but forIco/Curframes could be specified usingSequenceNumberfor encode.
I don't think we should store the rate block in the AniMetadata as we can have much more granular control when stored as FrameDelay in individual AniFrameMetadata instances. When encoding we can check if there is any variation in those values and use the header for all rates; otherise individual rate chunks.
Thinking about it some more I believe a good plan would be to have an enum in AniFrameMetadata of type AniFrameFormat which states whether the frame is Ico/Cur/Bmp.
We can then use nullable sub properties for IcoFrameMetadata, and CurFrameMetadata to AniFrameMetadata. which will save some effort. We can add NotNullWhen attributes to the properties based on the value of our AniFrameFormat property.
I make every ImageFrame has its own AniFrameMetadata, and where should we place Bmp/Ico/CurMetadata (not FrameMetadata)?
@Poker-sang I'll pull this down and have another deep look tonight.
New structure diagram for your reference
@Poker-sang Haven't forgotten this, just focusing on closing some issues first.
Would like to thank you and say that this PR is super useful to me as I'm currently working on some MacOS mouse cursor software. A lot of cursors online are made for Windows and come in .ani format, so having to support it is unavoidable and I'm very excited to try this out in the future.