Skip to content

Conversation

LordOfXen
Copy link
Contributor

Introducing IMG_CODER_STATUS, which is used in symmetry by both encoders and decoders for animation.

Here's a brief list:

  • Added IMG_CODER_STATUS enum.
  • Added IMG_CODER_STATUS_OK for stating that the coder was successful.
  • Added IMG_CODER_STATUS_FAILED for stating that the coder was unsuccessful.
  • Added IMG_CODER_STATUS_MAX for stating that we either hit max available frames (for decoders) or that we are not allowed to add more frames (for encoders).
  • Added IMG_CODER_STATUS_INVALID for stating when IMG_GetAnimationEncoderStatus or IMG_GetAnimationDecoderStatus is called with an invalid coder parameter. It can be used in other similar cases as well.
  • Modified IMG_AddAnimationEncoderFrame and IMG_GetAnimationDecoderFrame to first assign IMG_CODER_STATUS_OK and then, after the call to the underlying function, check if the status is still OK, but if an error has been provided by the underlying coder, we set IMG_CODER_STATUS_FAILED automatically. This smoothly handles statuses at the coder level without having to interfere with the existing SDL_SetError calls. However, calls for IMG_CODER_STATUS_MAX and such are still provided by the underlying coders.
  • Modified WEBP, AVIF, PNG, and GIF accordingly to use this new IMG_CODER_STATUS (e.g., IMG_CODER_STATUS_MAX).
  • Small other additions where brackets were forgotten, added those.

This does not interfere with the execution and can safely be merged in after an approval for the design.

@LordOfXen
Copy link
Contributor Author

Maybe we can rename IMG_CoderStatus to IMG_AnimationCoderStatus, up to you guys.

@LordOfXen
Copy link
Contributor Author

Okay if we only need this for the decoder part I'll update it all soon to only reflect this change for our decoder.
Should I rename IMG_CoderStatus to IMG_AnimationDecoderStatus?

@slouken
Copy link
Collaborator

slouken commented Aug 14, 2025

Okay if we only need this for the decoder part I'll update it all soon to only reflect this change for our decoder. Should I rename IMG_CoderStatus to IMG_AnimationDecoderStatus?

I suggested IMG_DecodeStatus above. Could be IMG_AnimationDecodeStatus?

@LordOfXen
Copy link
Contributor Author

Alright IMG_AnimationDecodeStatus it is then, will make those soon.

@LordOfXen
Copy link
Contributor Author

LordOfXen commented Aug 14, 2025

typedef enum IMG_CoderStatus
{
	IMG_DECODE_STATUS_OK, 	            /**< Decoded the frame successfully. */
	IMG_DECODE_STATUS_FAILED,           /**< Decoding the frame failed. Call SDL_GetError for more information. */
	IMG_DECODE_STATUS_COMPLETE, 	    /**< No more frames available. */

    IMG_DECODE_STATUS_INVALID,          /**< Invalid decoder status that does not represent any valid status. */
} IMG_CoderStatus;

@slouken should we keep IMG_DECODE_STATUS_INVALID? if we won't, what will it be when the user gives NULL to decoder pointer while calling the GetAnimationDecoderStatus?

Do you want function to be still named GetAnimationDecoderStatus? If not, what do you want it to be named?

Please let me know these.

(Ignore the enum name, haven't changed that yet, just check the fields)

FYI: IMG_AnimationDecodeStatus sounds grammatically incorrect; should it be IMG_AnimationDecoderStatus or IMG_AnimationFrameDecodeStatus? Or do you want to keep DecodeStatus as-is?

@slouken
Copy link
Collaborator

slouken commented Aug 14, 2025

@slouken should we keep IMG_DECODE_STATUS_INVALID? if we won't, what will it be when the user gives NULL to decoder pointer while calling the GetAnimationDecoderStatus?

Yes, keeping it seems correct.

Do you want function to be still named GetAnimationDecoderStatus? If not, what do you want it to be named?

Sure, that sounds good.

FYI: IMG_AnimationDecodeStatus sounds grammatically incorrect; should it be IMG_AnimationDecoderStatus or IMG_AnimationFrameDecodeStatus? Or do you want to keep DecodeStatus as-is?

It sounds grammatically correct to me, but IMG_AnimationDecoderStatus is probably better. It's the current status of the decoder.

@LordOfXen
Copy link
Contributor Author

Done, also fixed a bug in the encoder part where we weren't checking the result before increasing the accumulated_pts and last_delay with the given duration, it was getting increased even when the function failed, now that won't be the case.

@slouken slouken merged commit e4ee643 into libsdl-org:main Aug 14, 2025
5 checks passed
@slouken
Copy link
Collaborator

slouken commented Aug 14, 2025

Merged, thanks!

@LordOfXen
Copy link
Contributor Author

Merged, thanks!

Thanks to you as well.

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

Successfully merging this pull request may close these issues.

2 participants