Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Check return values of WebP API functions. #723

Merged
merged 2 commits into from
Nov 7, 2023

Conversation

vrabaud
Copy link
Contributor

@vrabaud vrabaud commented Nov 6, 2023

Those will be made nodiscard at some point.

Those will be made nodiscard at some point.
@DanBloomberg
Copy link
Owner

Thank you for this!

Two questions:
(1) Does "nodiscard" mean that the return value must be caught? Used? Are we talking about compiler warnings? I've never heard of this for an application library -- how does that get implemented? Enforced?
(2) Won't there be a leak from the malloc'd encoder structure with each of these new returns? if so, you would need to call WebPAnimEncoderDelete(enc) on each of them.

@vrabaud
Copy link
Contributor Author

vrabaud commented Nov 7, 2023

Thx for your responsiveness. I updated for animations and did some cosmetic changes
(1) This is C23/C++17 feature: https://en.cppreference.com/w/c/language/attributes/nodiscard. WebP will enable it if the compiler supports it. This just generates a warning if the result is not used (in an if statement, or an assignment). We thought it would be a good thing for users to check allocations.
(2) You are totally right, I fixed it.

@DanBloomberg
Copy link
Owner

I see that you're working on WebP and OpenCV at Google. Nice that Google is supporting this! I was an early supporter of WebP.

Perhaps you can tell me why Google doesn't support webpanim in Gmail. If you attach an animated gif to gmail, it will play. But not animated webp. That was one reason I never implemented reading webpanim.

Also, if you feel like it and have the time, you might enjoy implementing a function that reads webanim file data into a pixa:

       PIXA  *pixaReadMemWebPAnim(const l_uint8 *filedata, size_t filesize);

in analogy to the function pixReadMemWebP() in webpio.c

@DanBloomberg DanBloomberg merged commit c1ee729 into DanBloomberg:master Nov 7, 2023
10 of 11 checks passed
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