[guilib] Add animated image/texture support via ffmpeg. #8937

ace20022 · 2016-01-21T16:31:48Z

Warning: In its current state, this pr can cause memleaks and out of memory crashes.

ace20022 · 2016-01-21T16:36:34Z

@garbear @notspiff fyi. The same as for gifs applies here: As long as we don't load them in a job and each/some frames on demand, i's really only usable for small apngs.

@fritsch It really is a quick "hack". Are you in the mood to have a look?

fritsch · 2016-01-21T19:55:20Z

Hi @ace20022 thx much. Currently ill and not that fit. Can check over the weekend. Android keeps me busy currently.

ace20022 · 2016-01-21T19:58:46Z

@fritsch np. Get well soon!
Nothing urgent, I was just happy that it works (more or less) ;)

ace20022 · 2016-01-22T10:05:52Z

😢 libgif can die now 😢

mgonzales71 · 2016-01-26T15:09:57Z

personally want to thank you guys for finally bringing ANPG support to Kodi. Willing to test this if you create a test build - either way look forward to enjoying this at some future point in Kodi. Many thanks again for all your hard work! 😄

fritsch · 2016-01-26T16:59:51Z

Which platform do you need @mgonzales71 ?

@ace20022 i have not forgotten to review but still busy with AudioTrack - sorry for that!

mgonzales71 · 2016-01-26T17:11:42Z

@fritsch using Kodi on Win 10 x64 Pro mainly here, also occassionally on Android (ARM) so can test both if needed.

ace20022 · 2016-01-26T17:14:29Z

np, I'm still hunting a mem leak, hoping that it's not in ffmepg's png decoder (Valgrind indicates that)

@fritsch @mgonzales71 don't use a build of the pr in this state! A build is only useful without the two leak check commits.

mgonzales71 · 2016-01-26T17:53:17Z

@ace20022 I am fine with waiting once you squash mem leak(s)

FYI - a great collection of APNG resource info - http://littlesvr.ca/apng/ for the interested
(note the Developer's resources section)

ace20022 · 2016-01-28T14:01:16Z

@fritsch thx for the review, found two leaks, an old one and a new one ;)
I've not implemented a memory restriction...
@MilhouseVH could you please test what happens on a pi with the images from: https://www.reddit.com/r/apng ?
Note the files must have the extension apng (not png).

jenkins build this please

MilhouseVH · 2016-01-28T17:05:15Z

could you please test what happens on a pi with the images from: https://www.reddit.com/r/apng ?

On Pi (RPi2) and also x86, with this PR the pngs (*.apng) will be listed in Pictures - without the PR they wouldn't even be listed - and they will display, but there is no animation, presumably just the first frame is being displayed.

Were you expecting animation? It could be an OpenELEC issue, although I added --enable-encoder=apng to ffmpeg, but it had no effect.

Looking in the texture cache, the cached version of the 20MB+ apng files is being created as 400KB .png files, which might be right - I doubt we'd want the apng files to hit the cache, would we?

fritsch · 2016-01-28T17:08:13Z

@ace20022 could you show me the leaks that you fixed?

xbmc/guilib/FFmpegImage.cpp

@@ -395,6 +474,7 @@ bool CFFmpegImage::Decode(unsigned char * const pixels, unsigned int width, unsi
  }
  pictureRGB->data[0] = nullptr;
  avpicture_free(pictureRGB);
+  av_free(pictureRGB);


ace20022 · 2016-01-28T18:00:11Z

@MilhouseVH sorry, should have been more precise. They work as (skin) textures, like posters, background, etc. Simplest test is to set it as background in confluence (somewhere in the skin settings).

MilhouseVH · 2016-01-29T00:38:11Z

Heh, seems to be working on the RPi2. I've set the Confluence "custom background" path to one of the apng's and I now have animated skiing ostriches as my backdrop...

Total CPU load is pretty reasonable, about 4-5% on a 1GHz ARM RPi2 (~20% on one core) for the skiing ostriches apng. It has used a fairly large chunk of GPU memory to load the apng - from 240MB free to only 140MB free. The size of this file is 15MB.

Falling snow consumes only about 10MB GPU RAM, presumably as this is a much smaller apng (fewer frames? lower resolution?) weighing in at only 637KB.

However HQ animated pixel art landscape, a 17MB apng, consumes all available GPU memory (from 240MB free to 1MB free) yet almost 600MB of non-GPU RAM remains available. Unsurprisingly Kodi becomes unusable with this png as a background.

Animated pngs seem to be working, but are likely to cause memory issues on systems with limited GPU memory. Is there any reason why GPU memory is being used in favour of non-GPU RAM - if it's loading as a texture that might explain it.

mgonzales71 · 2016-01-29T03:53:39Z

Sorry for my ignorance with the workings of Jenkins but from what I can gather from http://jenkins.kodi.tv/job/BuildMulti-PR/4321/ the Windows test build of the PR failed? Well marked "unstable" actually... but regardless - not sure of where to download a test binary other then mirrors.kodi.tv /nightlies or /test-builds when one is available to download and test. Apologies if I missed an obvious download location for the Kodi Win Setup that the PR test build console output says is located at c:\jenkins\slave\workspace\WIN-32\project\Win32BuildSetup\KodiSetup-20160128-df84916-HEAD.exe 😀

Thanks :)

afedchin · 2016-01-29T06:10:43Z

@mgonzales71 here you go http://mirrors.kodi.tv/test-builds/win32/

fritsch · 2016-01-29T06:16:26Z

Nearly :-) Currently the whole mirror is down.

2016-01-29 7:10 GMT+01:00 Anton Fedchin notifications@github.com:

@mgonzales71 https://github.com/mgonzales71 here you go
http://mirrors.kodi.tv/test-builds/win32/

—
Reply to this email directly or view it on GitHub
#8937 (comment).

               Key-ID:     0x1A995A9B
               keyserver: pgp.mit.edu

Fingerprint: 4606 DA19 EC2E 9A0B 0157 C81B DA07 CF63 1A99 5A9B

afedchin · 2016-01-29T06:55:50Z

@fritsch it works for me.

fritsch · 2016-01-29T08:02:04Z

Nice - @kib also fixed it for the whole world some minutes ago.

2016-01-29 7:55 GMT+01:00 Anton Fedchin notifications@github.com:

@fritsch https://github.com/fritsch it works for me.

—
Reply to this email directly or view it on GitHub
#8937 (comment).

               Key-ID:     0x1A995A9B
               keyserver: pgp.mit.edu

Fingerprint: 4606 DA19 EC2E 9A0B 0157 C81B DA07 CF63 1A99 5A9B

ace20022 · 2016-01-29T13:06:31Z

@mgonzales71 http://mirrors.kodi.tv/test-builds/win32/KodiSetup-20160128-134a3dc-ffmpeg_apng.exe
Some addons failed to build: http://jenkins.kodi.tv/view/Windows/job/WIN-32/7626/

ace20022 · 2016-01-29T13:30:15Z

@MilhouseVH that one https://i.imgur.com/q2SSe9S.png (probably the biggest one) needs almost 3gb ram for decoding, after some seconds it's only 16 mb or so for the kodi process, overall is still high. I have no clue how windows handles that. The gpu mem is almost completely used.

This apng has 91 frames á 3840x2160 pixels, i.e. 91*33mb = 3003 mb.

The only thing I could do is to add an upper bound for the memory to be used and limit the number of frames accordingly. Until I'm in the mood to implement on demand frame extraction/caching.
@fritsch @MilhouseVH @popcornmix thoughts?

popcornmix · 2016-01-29T16:35:25Z

It seems quite easy for an animated png to exhaust texture memory and potentially kill kodi (if it doesn't crash immediately any subsequent gui elements will fail to allocate and so not be visible making kodi pretty much unusable).
So some sort of sanity check on size seems useful. But I don't see an easy way of choosing a size that will be optimal across many devices with different amounts of texture memory.

On demand extraction sounds like a better solution. q2SSe9S.png is only 10MB as a png, but seems to expand 30 times when decoded. On-demand extraction may increase CPU usage significantly, but that still seems like a better solution (at least for files larger than some threshold). High cpu seems better than exhausted memory and unusable kodi.

mgonzales71 · 2016-01-29T20:33:28Z

@ace20022 Thanks for the link - also the png ( https://i.imgur.com/q2SSe9S.png ) brings Firefox to it knees even on Windows but like others have noted apngs seems to eat up texture mem quickly.

For what it's worth the use I see for apng's in Kodi are obviously for skin's - add-on icons, clearart, clearlogo, character art, home tile/menu graphics and various skin elements (progress, buttons, weather icon, fanart, controls, etc) I would expect these uses to be small to modest in size but could be numerous. The other use would be backgrounds and that is where things could get ugly - depends on the apng.. badly created/pooly optimized images being the most likely flame for that fire.

silly question as I am sure that all the options were looked into but is ffmpeg's the best implementation of apng support for Kodi?

I will keep testing and provide feedback of any problems I run into on Windows.. thanks for finally adding apng support - kudos to all the devs for all your hard works. it's apprecited very much.

fritsch · 2016-01-29T20:37:23Z

silly question as I am sure that all the options were looked into but is ffmpeg's the best implementation of apng support for Kodi?

Do you know a better solution that works on all platforms we run? Adding no overhead to the code we maintain? If you find one, let us know.

mgonzales71 · 2016-01-29T20:56:00Z

@fritsch meant no disrespect I hope that wasn't how that Q came across. Apologies if I did. I have nothing but praise for all your hard work.

My go to link for APNG info has been http://littlesvr.ca/apng/ and that has been mostly from a user standpoint - they do have a dev section but I am not qualified to determine suitable criteria for potential use in Kodi.

fritsch · 2016-01-29T21:13:47Z

The problem is not ffmpeg, but the raw decoded buffers we keep in memory at once, when decoding the whole apng.

zag2me · 2016-01-30T13:11:33Z

APNG's(like animated gifs) are absolutely not designed for large backgrounds or textures. Those kind of effects should be done using the video player just like machines like the PS3 do. Far better suited to small images like buttons or thumbnails.

Saying that, it would be nice for it to fail gracefully.

ace20022 · 2016-01-30T13:18:29Z

@popcornmix are you okay, with a limit of 12 full hd frames, approx. 8 mb each, till I have the motivation to implement the on demand loading? btw currently I have limited gifs to 10 full hd frames.

zag2me · 2016-01-30T13:43:08Z

To prevent confusion in things like the animated thumbnail posters thread, could it be made equal to gifs?

http://forum.kodi.tv/showthread.php?tid=215727&page=23

…coded list for image selection.

…via FFmpegImage.

…100mb memory.

ace20022 · 2016-02-04T12:34:53Z

Added a limit of 12 full hd frames, memory-wise.

Any objections?

jenkins build this please

ace20022 · 2016-02-04T13:45:48Z

standard win32 addon failure. I'll merge it tonight.

fritsch · 2016-02-04T13:47:15Z

Jep, go for it. Btw. we can sit together afterwards and see how we can adjust the work to the upcoming ffmpeg API, which deprecates a whole lot of methods we actively use. Some easy to replace others a bit more problematic.

stefansaraev · 2016-02-04T18:29:23Z

maybe stupid question. giflib is still (conditionaly) used in imagefactory. is this supposed to be cleaned up ? (I'll do if you say so @ace20022)

EDIT: yep. if giflib is present, the ffmpeg code path is not used. here it is: stefansaraev@b8c6876 (force pushed)

ace20022 · 2016-02-04T19:04:11Z

Thx, I already had that, https://github.com/ace20022/xbmc/commits/remove_giflib . It' s still needed by the texture packer...

stefansaraev · 2016-02-04T19:14:57Z

yep, texturepacker needs giflib-native. dont bother picking if it breaks build, I'll take care of it later.

[guilib] Add animated image/texture support via ffmpeg.

ace20022 added the WIP PR that is still being worked on label Jan 21, 2016

ace20022 force-pushed the ffmpeg_apng branch 3 times, most recently from 7d1b960 to fe7af97 Compare January 24, 2016 23:33

ace20022 force-pushed the ffmpeg_apng branch from fe7af97 to 134a3dc Compare January 28, 2016 13:56

ace20022 added Type: Feature non-breaking change which adds functionality v17 Krypton Component: Picture and removed WIP PR that is still being worked on labels Jan 28, 2016

ace20022 reviewed Jan 28, 2016
View reviewed changes

xbmc/guilib/FFmpegImage.cpp

@@ -395,6 +474,7 @@ bool CFFmpegImage::Decode(unsigned char * const pixels, unsigned int width, unsi

}

pictureRGB->data[0] = nullptr;

avpicture_free(pictureRGB);

av_free(pictureRGB);

This comment was marked as spam.

Sign in to view

ace20022 and others added 6 commits February 4, 2016 13:24

[FFmpegImage] Make constructor explicit.

c75f5fd

[dialogs] Use g_advancedSettings.m_pictureExtensions instead of hard …

88e132b

…coded list for image selection.

[FFmpegImage] Add animated pictures support.

ae0a312

[pictures] Add support for Animated Portable Network Graphics (APNG) …

c8a66d0

…via FFmpegImage.

[guilib] Use FFmpegImage for Graphics Interchange Format (GIF).

3b00de5

[guilib] Limit animated textures to 12 full hd frames, i.e., approx. …

ffc42b3

…100mb memory.

ace20022 force-pushed the ffmpeg_apng branch from 134a3dc to ffc42b3 Compare February 4, 2016 12:33

ace20022 changed the title ~~[guilib] Add animated png support.~~ [guilib] Add animated image/texture support via ffmpeg. Feb 4, 2016

ace20022 added a commit that referenced this pull request Feb 5, 2016

Merge pull request #8937 from ace20022/ffmpeg_apng

e95c344

[guilib] Add animated image/texture support via ffmpeg.

ace20022 merged commit e95c344 into xbmc:master Feb 5, 2016

ace20022 deleted the ffmpeg_apng branch February 5, 2016 10:49

razzeee added this to the Krypton 17.0-alpha1 milestone Feb 5, 2016

[guilib] Add animated image/texture support via ffmpeg. #8937

[guilib] Add animated image/texture support via ffmpeg. #8937

Conversation

ace20022 commented Jan 21, 2016

ace20022 commented Jan 21, 2016

fritsch commented Jan 21, 2016

ace20022 commented Jan 21, 2016

ace20022 commented Jan 22, 2016

mgonzales71 commented Jan 26, 2016

fritsch commented Jan 26, 2016

mgonzales71 commented Jan 26, 2016

ace20022 commented Jan 26, 2016

mgonzales71 commented Jan 26, 2016

ace20022 commented Jan 28, 2016

MilhouseVH commented Jan 28, 2016

fritsch commented Jan 28, 2016

This comment was marked as spam.

ace20022 commented Jan 28, 2016

MilhouseVH commented Jan 29, 2016

mgonzales71 commented Jan 29, 2016

afedchin commented Jan 29, 2016

fritsch commented Jan 29, 2016

afedchin commented Jan 29, 2016

fritsch commented Jan 29, 2016

ace20022 commented Jan 29, 2016

ace20022 commented Jan 29, 2016

popcornmix commented Jan 29, 2016

mgonzales71 commented Jan 29, 2016

fritsch commented Jan 29, 2016

mgonzales71 commented Jan 29, 2016

fritsch commented Jan 29, 2016

zag2me commented Jan 30, 2016

ace20022 commented Jan 30, 2016

zag2me commented Jan 30, 2016

ace20022 commented Feb 4, 2016

ace20022 commented Feb 4, 2016

fritsch commented Feb 4, 2016

stefansaraev commented Feb 4, 2016

ace20022 commented Feb 4, 2016

stefansaraev commented Feb 4, 2016