As part of the long term goal of playing animated GIFs through the media framework, we'd like to start by supporting them in the video element. This work should proceed behind a pref.

An introduction note: we are trying to move aways from the MediaDecoderReader architecture where we have one blob doing the parsing of the raw data and decoding. Instead you only need to implement a MediaDataDemuxer (bug 1154164) that will extract raw compressed samples (a MediaRawData object) and then implement a MediaDataDecoder which uncompress a single MediaRawData into a VideoData object (a picture really) The overall architecture is described in bug 1148292. The aim is to be later able to reuse the codec with different containers if need be. Probably not a worry with gif. But mostly then you benefit from all the goodness of the MediaFormatReader which provides async / highly multi-threaded decoding. So in the end, we only have a single MediaDecoderReader and a pool of demuxer and codecs.

(In reply to Jean-Yves Avenard [:jya] from comment #1) > We are trying to move aways from the MediaDecoderReader architecture where > we have one blob doing the parsing of the raw data and decoding. In an effort to not understate our intent here, we won't be accepting any new subclasses of MediaDecoderReader.

Rebased version of part 1.

Rebased part 2.

Rebased part 3.

Rebased part 4.

Rebased part 5.

Rebased part 6.

There are printfs in both the constructor and the destructor of the VideoData class. This way we can keep track of how many VideoData objects we have in memory at any given moment.

Comment on attachment 8664543 [details] [diff] [review] (Part 6) - Keep track of how many VideoData objects are created and how many are destroyed at any given moment during playback Review of attachment 8664543 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/media/MediaData.cpp @@ +122,5 @@ > { > NS_ASSERTION(mDuration >= 0, "Frame must have non-negative duration."); > mKeyframe = aKeyframe; > mTimecode = aTimecode; > + printf("gifmediamemory %p created\n", this); We don't want to include printfs in release builds. You might want to use MOZ_LOG.

quick fly over. The image displays quickly because you set the duration of each frames that gives a refresh rate of 100Hz, causing the 77 images to be displayed in .7s only. this didn't compile for me on mac with clang (error with the gotos and variables not being initialised / defined) i was playing that GIF: http://netdna.webdesignerdepot.com/uploads7/50-inspiring-animated-gifs/006.gif it shows a black frame after a few images and the background becomes black. not sure why

(In reply to Chris Peterson [:cpeterson] from comment #21) > We don't want to include printfs in release builds. You might want to use > MOZ_LOG. This code is not ready for review.

Thank you for your feedback! (In reply to Jean-Yves Avenard [:jya] from comment #22) > The image displays quickly because you set the duration of each frames that > gives a refresh rate of 100Hz, causing the 77 images to be displayed in .7s > only. I have tried with 0.039 and it still doesn't seem to affect the frame duration. Did this value work for you? > this didn't compile for me on mac with clang (error with the gotos and > variables not being initialised / defined) This is strange because the "done" label was used before I even touched the code. I will redesign the "frame by frame" decoding code. However, I don't know what to say about the "done" label. > i was playing that GIF: > http://netdna.webdesignerdepot.com/uploads7/50-inspiring-animated-gifs/006. > gif > > it shows a black frame after a few images and the background becomes black. > > not sure why There are still some GiFs that crash. Also, loading the GIF directly seems to crash. Including them in <video> elements is working. I used this for tests: http://people.mozilla.org/~mvolmer/testpage2.html

(In reply to Mihai Volmer [:mihaivolmer] from comment #24) > Thank you for your feedback! > > (In reply to Jean-Yves Avenard [:jya] from comment #22) > > The image displays quickly because you set the duration of each frames that > > gives a refresh rate of 100Hz, causing the 77 images to be displayed in .7s > > only. > > I have tried with 0.039 and it still doesn't seem to affect the frame > duration. Did this value work for you? it did yes (for the gif I was using) > > > this didn't compile for me on mac with clang (error with the gotos and > > variables not being initialised / defined) > > This is strange because the "done" label was used before I even touched the > code. I will redesign the "frame by frame" decoding code. However, I don't > know what to say about the "done" label. it's the goto finished_frame; that gave error likely because bool hitFinishedFrame = false; is defined after the goto, but before the label. moving + bool hitFinishedFrame = false; to be before the loop will likely do the trick too (still ugly imho)

(In reply to Jean-Yves Avenard [:jya] from comment #25) > moving + bool hitFinishedFrame = false; to be before the loop will likely > do the trick too > > (still ugly imho) That code was just a hack to get things to work for the demo. I wouldn't r+ it. In fact, we're probably going to need to semi-rewrite the GIF decoder before we actually land anything in this bug. (See bug 1204392.)

Mihai, I'm clearing these feedback flags because I think we need to make progress on getting basic functionality working before we're ready for detailed feedback. At this time I think the biggest concern is fixing the frame duration problem.