| CARVIEW |
Navigation Menu
-
-
Notifications
You must be signed in to change notification settings - Fork 56.2k
Animated PNG Support #25715
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
Animated PNG Support #25715
Conversation
|
@vrabaud i am willing to modify |
|
You probably do not want to modify that file as it will make it harder to track changes upstream. Have it be its own file (apng.cpp? or whatever it was called in the first place). And then have your own |
|
@sturkmen72, did you get my mail? You should really first design an API that will handle animations. |
|
hi @vrabaud linux ci server windows ci server |
|
@asmorkalov is it possible to update versions of libpng and libwebp on linux ci servers. |
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.
Mostly, please split the code into what was taken from upstream, and what you added, this will make it easier to update files to upstream.
modules/imgcodecs/src/grfmt_apng.cpp
Outdated
| m_height = height; | ||
| m_color_type = PNG_COLOR_MASK_COLOR; | ||
|
|
||
| m_pixels = new uchar[m_height * rowbytes]; |
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 use C++ to make it easier to track memory (std::vector here)
modules/imgcodecs/src/grfmt_apng.cpp
Outdated
|
|
||
| /****************************************************************************\ | ||
| * | ||
| * this file includes some modified part of apngasm and APNG Optimizer 1.4 |
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.
Can't the relevant code simply be copied from upstream? It will make it much easier to track upstream bug fixes. Which files did you take from upstream?
|
@vrabaud thank you for review. the PR is still a draft and not yet completely functional. i will take your remarks in account. |
modules/imgcodecs/src/apngframe.hpp
Outdated
| * The next generation of apngasm, the APNG Assembler. | ||
| * The apngasm CLI tool and library can assemble and disassemble APNG image files. | ||
| * | ||
| * https://github.com/apngasm/apngasm |
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.
How about mentioning which file got copied? Ideally, the first commit would be about adding the pristine file from upstream, and you would then have a commit to modify it to make sure we know what got modified.
modules/imgcodecs/src/grfmt_png.cpp
Outdated
| if (pChunk->size > PNG_USER_CHUNK_MALLOC_MAX) | ||
| return 0; | ||
| pChunk->size += 12; | ||
| pChunk->p = new uchar[pChunk->size]; |
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 do not use new as it makes its hard to track memory. Use std::vector if you can.
sturkmen72
force-pushed
the
apng_support
branch
2 times, most recently
from
ba23302 to
2ec4b67
Compare
sturkmen72
force-pushed
the
apng_support
branch
3 times, most recently
from
8a5311c to
946a110
Compare
sturkmen72
force-pushed
the
apng_support
branch
from
32dd3ec to
4ea9b1c
Compare
sturkmen72
force-pushed
the
apng_support
branch
2 times, most recently
from
2daf9da to
5f716bd
Compare
sturkmen72
force-pushed
the
apng_support
branch
from
f22861e to
227c56f
Compare
|
@sturkmen72 Looks like Obj-C / Swift bindings generator does not handle |
sturkmen72
force-pushed
the
apng_support
branch
from
a3e42bc to
20083ab
Compare
|
@sturkmen72 I merged your patch for webp/avif. Please rebase and fix conflicts. |
|
@asmorkalov there is 3 issues i am working on hopefully be completed soon.
|
|
@asmorkalov @vrabaud i believe the PR is ready for final review. |
sturkmen72
force-pushed
the
apng_support
branch
from
d1c78d5 to
3e2f078
Compare
Animated PNG Support opencv#25715 Continues opencv#25608 ### Pull Request Readiness Checklist See details at https://github.com/opencv/opencv/wiki/How_to_contribute#making-a-good-pull-request - [x] I agree to contribute to the project under Apache 2 License. - [x] To the best of my knowledge, the proposed patch is not based on a code under GPL or another license that is incompatible with OpenCV - [x] The PR is proposed to the proper branch - [x] There is a reference to the original bug report and related work - [x] There is accuracy test, performance test and test data in opencv_extra repository, if applicable Patch to opencv_extra has the same branch name. - [x] The feature is well documented and sample code can be built with the project CMake
Animated PNG Support opencv#25715 Continues opencv#25608 ### Pull Request Readiness Checklist See details at https://github.com/opencv/opencv/wiki/How_to_contribute#making-a-good-pull-request - [x] I agree to contribute to the project under Apache 2 License. - [x] To the best of my knowledge, the proposed patch is not based on a code under GPL or another license that is incompatible with OpenCV - [x] The PR is proposed to the proper branch - [x] There is a reference to the original bug report and related work - [x] There is accuracy test, performance test and test data in opencv_extra repository, if applicable Patch to opencv_extra has the same branch name. - [x] The feature is well documented and sample code can be built with the project CMake
Resolves opencv#26913 Related(?): opencv#25715 opencv#26832
Continues #25608
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.