| CARVIEW |
Navigation Menu
-
-
Notifications
You must be signed in to change notification settings - Fork 56.2k
imgcodecs: gif: support animated gif without loop #26971
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
Conversation
|
@sturkmen72 Is the Looping option relevant to APNG, AVIF, etc? M.b. make it generic. |
it is possible adding enums and add relevant code to other encoders |
|
We have |
|
imho will be useful if these properties wanted to be write with up to you dropping |
|
there is also |
|
i think IMWRITE_GIF_SPEED is unfunctional now ( overlooked when adopted gif encoder to imwriteanimation ) |
|
IMO,
|
|
i vote for |
|
I dropped Currently Do we have to remove them and reassign other enums ? |
|
@Kumataro I restored constants and added warning. |
|
@asmorkalov Thank you to resolve my worry about |
| CV_PROP_RW int loop_count; | ||
| //! Background color of the animation in BGRA format. | ||
| CV_PROP_RW Scalar bgcolor; | ||
| //! Duration for each frame in milliseconds. | ||
| /*! @note (GIF) Due to file format limitation |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please consider
/*! @note (GIF) Due to file format limitation
* - Durations must be multiples of 10 milliseconds. Any provided value will be rounded down to the nearest 10ms (e.g., 88ms → 80ms).
* - 0ms(or smaller than expected in user application) duration may cause undefined behavior, e.g. it is handled with default duration.
* - Over 65535 * 10 milliseconds duration is not supported.
*/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This example is helpful to know about duration about 10ms unit, thank you for your comment !
- Over 65535 milliseconds duration is not supported.
For GIF89a specification, I think it seems to have to change from 655350.
https://www.w3.org/Graphics/GIF/spec-gif89a.txt
- Graphic Control Extension.
vii) Delay Time - If not 0, this field specifies the number of
hundredths (1/100) of a second to wait before continuing with the
processing of the Data Stream. The clock starts ticking immediately
after the graphic is rendered. This field may be used in
conjunction with the User Input Flag field.
We can storeDelay Time value as 16 bit unsigned integer with 10ms unit, so a range from 10ms to 655350ms are valid.
// GIF file stores duration in 10ms unit.
const int frameDelay10ms = cvRound(frameDelay / 10);
CV_LOG_IF_WARNING(NULL, (frameDelay10ms == 0),
cv::format("frameDelay(%d) is rounded to 0ms, its behaviour is user application depended.", frameDelay));
CV_CheckLE(frameDelay10ms, 65535, "It requires to be stored in WORD");Both 65535ms(about 1min) and 655350ms(about 11min) are a very long durations as animation frames.
So this restriction is not expected to affect user experience.
To make compatibility with other codecs, should I limit with CV_CheckLE(frameDelay, 65535, "Unsupported duration") ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Both 65535ms(about 1min) and 655350ms(about 11min) are a very long durations as animation frames.
So this restriction is not expected to affect user experience.
To make compatibility with other codecs, should I limit with CV_CheckLE(frameDelay, 65535, "Unsupported duration") >>?
i think the PR as is good.
up to you
using in documentation - Durations must be multiples of 10 milliseconds. Any provided value will be rounded down to the nearest 10ms (e.g., 88ms → 80ms).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for reply ! I fixed it and to use "millisecondsinstead ofmillseconds`.
|
@vrabaud Do you have any notes? |
Close #26970
Pull Request Readiness Checklist
See details at https://github.com/opencv/opencv/wiki/How_to_contribute#making-a-good-pull-request
Patch to opencv_extra has the same branch name.