Page MenuHomePhabricator

Support WebP Animation Image Files
ClosedPublic

Authored by herb on May 25 2020, 12:06 AM.

Details

Summary

Support WebP Animate Format Imaeg Files.
To support webp animation, apply webp animation decoder.

Test Plan
  1. compile src/exmaple/elementary/image_webp_example_01.c and 02.c
  2. run the samples

Diff Detail

Repository
rEFL core/efl
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
There are a very large number of changes, so older changes are hidden. Show Older Changes
herb requested review of this revision.May 25 2020, 12:06 AM
herb updated this revision to Diff 30376.May 25 2020, 12:09 AM

update codes

herb updated this revision to Diff 30377.May 25 2020, 12:33 AM

update sample codes

herb updated this revision to Diff 30378.May 25 2020, 1:25 AM

update files

herb updated this revision to Diff 30379.May 25 2020, 1:34 AM

update codes

herb updated this revision to Diff 30380.May 25 2020, 1:38 AM

clean up the codes

kimcinoo requested changes to this revision.May 25 2020, 3:31 PM
kimcinoo added inline comments.
src/modules/evas/image_loaders/webp/evas_image_load_webp.c
13

What is this Frame_Info for?

32

Why do't you use same style for typedef?
i.e.
typedef struct _Image_Frame Image_Frame;
or
typedef struct _Loader_Info
{
...
} Loader_Info

113–138

Please remove EINA_UNUSED

163

Please free memory of frame before return here.

168

Would you please add some comments why we need to use 0.1? What is the meaning of 0.1?

179

:-p
단수형은 datum이지만 전문 용어로는 흔히 복수형으로 쓰임
https://en.dict.naver.com/#/entry/enko/dca3a30f55114aa79876939b06b0f769

193

*error = ?;
Please set error before return.

200

*error = ?;
Please set error before return.

208

Please remove empty line.

This revision now requires changes to proceed.May 25 2020, 3:31 PM
herb updated this revision to Diff 30395.May 25 2020, 7:04 PM

modified code for comments

+1024 Looks good to me!

Hermet requested changes to this revision.May 25 2020, 9:31 PM

Please check comments.

src/examples/elementary/image_webp_example_01.c
2

You missed adding this examples in meson.build.

src/lib/evas/meson.build
13

webp = dependency('libwebpdemux', required: get_option('evas-loaders-disabler').contains('webp') == false)

src/modules/evas/image_loaders/webp/evas_image_load_webp.c
18

Does File_Info struct necessary? I think you can put the void* map into Loader_Info

144

EINA_LIST_FREE()?
where this frames are freed?

161

looks unnecessary.

177

Always animated??

190

q) what case frame->dealy == 0.0? and where DEFAULT_DURATION value come from?

204

I think you can use eina_array so that you can find the frame in O(1)

212

This is not good for loading time....
Is it possible to load frames on the demand?

This revision now requires changes to proceed.May 25 2020, 9:31 PM
herb updated this revision to Diff 30445.May 27 2020, 5:56 AM

update codes

Hermet requested changes to this revision.May 27 2020, 6:39 PM

Please check one more comment.

src/modules/evas/image_loaders/webp/evas_image_load_webp.c
109

When we get frame_count, we can reserve array with the size in advance.

This revision now requires changes to proceed.May 27 2020, 6:39 PM
jsuya added inline comments.May 27 2020, 6:41 PM
src/modules/evas/image_loaders/webp/evas_image_load_webp.c
109

I have simple question. Why step size is 20?

109

The number of frames is fixed. You can consider using eina_inarray.

herb updated this revision to Diff 30452.May 27 2020, 9:53 PM

update codes

Looks good to me.

I think this module also consider to flush the stored frame images to retain the spare memory , Please check a gif loader - flush_older_frames(), how it does.

Hermet accepted this revision.May 27 2020, 10:09 PM
kimcinoo requested changes to this revision.May 28 2020, 12:13 AM
kimcinoo added inline comments.
src/modules/evas/image_loaders/webp/evas_image_load_webp.c
223

Why don't you check return value of this WebPAnimDecoderGetNext?
No need to initialize buf?

This revision now requires changes to proceed.May 28 2020, 12:13 AM
herb updated this revision to Diff 30460.EditedMay 28 2020, 12:25 AM

Add checking return value for getting next frame

buf owner is dec, so we don't need to free for buf
// ... (Do NOT free 'buf', as it is owned by 'dec').

herb updated this revision to Diff 30461.May 28 2020, 12:45 AM

update codes

kimcinoo accepted this revision.May 28 2020, 12:57 AM

Verified as well. Thank you for your effort.

This revision is now accepted and ready to land.May 28 2020, 12:57 AM
jsuya accepted this revision.May 28 2020, 3:41 AM

LGTM :)

@bu5hm4n any comments of the meson build?

Only this one thing :)

src/lib/evas/meson.build
10–11

This line can go, as we overwrite the result anyways.

Hermet requested changes to this revision.May 28 2020, 6:57 PM

Please check one more comment.

src/lib/evas/meson.build
10–11

@herb Could you please remove libwebp dependency?

This revision now requires changes to proceed.May 28 2020, 6:57 PM
herb updated this revision to Diff 30478.May 28 2020, 7:32 PM

remove libweb dependency for libwebpdemux

Hermet accepted this revision.May 28 2020, 7:40 PM
This revision is now accepted and ready to land.May 28 2020, 7:40 PM
This revision was automatically updated to reflect the committed changes.

I think there should be libwebp version check, because you will get compilation errors on ubuntu 16.04 when libwebp in installed using apt install