It would be nice for JavaScript to be able to query a video element to see what framerate it is getting. This can be used for development, to see if improvements to video code improves the frame rate for example. It could be useful for general playback javascript to determine if a machine can playback a particular video size in a reasonable manner and switch to another size if it cannot.

WHATWG thread about the topic: http://lists.whatwg.org/htdig.cgi/whatwg-whatwg.org/2010-May/026523.html For testing bug 577843 I implemented a mozFrameCount so I could compute framerate of the playing video and compare playback performance with and without the scaling patch. I'll add the patch for that to this bug. Another approach is mentioned in the above thread, instead of a frame count use dropped frames as the measure: http://lists.whatwg.org/htdig.cgi/whatwg-whatwg.org/2010-May/026528.html

Example HTML that uses mozFrameCount to display a framerate.

This updates the existing patch to trunk and adds: - mozDroppedFrames to video element - mozDownloadRate and mozPlaybackRate to media elements The first gives us the same stat as Flash supplies. The second provides an alternative to the data that was removed from the progress event in bug 584615. These are also stats that have been asked for by people implementing live streaming bitrate fallback.

This is awesome! Can we have this in trunk, please? It would allow implementation of adaptive HTTP streaming in JavaScript!

This is indeed awesome. Adaptive streaming stuff aside, this will be useful for us (YouTube) in just monitoring streaming performance / issues, especially as compared to Flash playback.

Based on feedback I'm changing the names slightly: HTMLMediaElement: // The approximate rate at which the media resource is being downloaded in // kilobytes per second. If the rate is not available (possibly due // to not having downloaded enough data to compute a consistent value) // this will be NaN. readonly attribute float mozDownloadRate; // The approximate rate at which the media resource is being decoded in // kilobytes per second. If the rate is not available this will be // NaN. readonly attribute float mozDecodeRate; HTMLVideoElement: // A count of the number of frames that have been decoded and ready // to be displayed. This can be used for computing the decoding framerate readonly attribute unsigned long mozDecodedFrames; // A count of the number of frames that have been dropped for performance // reasons during playback. readonly attribute unsigned long mozDroppedFrames; So mozFrameCount becomes mozDecodedFrames and mozPlaybackRate becomes mozDecodeRate.

Updated to use changed names.

Implements playback statistics.

Fixes small error in IDL comment. Rates are bytes per second, not kilobytes per second.

Access to mDecodeDecodedFrames needs to be protected by a lock or something. Right now it's written on a decoder thread and read on the main thread. I'm a bit concerned about the definitions of mozDecodedFrames and mozDroppedFrames. Currently mozDecodedFrames measures the number of frames presented by the decoder for display (not the same as the number of frames actually painted). mozDroppedFrames measures the number of frames dropped by the decoder, but doesn't count frames dropped while we sync up to the audio in AdvanceFrames, nor the frames that the decoder thread did SetCurrentImage on but which weren't actually painted. I think maybe the way to go would be * mozDecodedFrames: total number of frames seen by the decoder, including frames it skipped * mozDisplayedFrames: total number of unique frames that we actually drew to the screen (as best we can tell) The difference would be the number of frames we skipped for any reason. But at this stage I think we should focus on FF4 blockers and get this in when we can fix it properly.

(In reply to comment #12) > Access to mDecodeDecodedFrames needs to be protected by a lock or something. > Right now it's written on a decoder thread and read on the main thread. It's protected by the mVideoUpdateLock lock when written but is missing it on the read. I'll fix that. Is that the right lock to use or do you want one specific for this purpose? > mozDroppedFrames measures the number of frames dropped by > the decoder, but doesn't count frames dropped while we sync up to the audio in > AdvanceFrames, Yes, this is a bug. It should be adding to the dropped frames here. > nor the frames that the decoder thread did SetCurrentImage on > but which weren't actually painted. This is because it is intended to measure whether the hardware can keep up with the decoding of the resource rather than the painting of it. This is useful for example when decoding the video and processing the video data via a canvas. I don't think this would be included in a count that only counted frames when painted. > * mozDisplayedFrames: total number of unique frames that we actually drew to > the screen (as best we can tell) Possibly displayed frames is useful too but it doesn't address the canvas usecase I mention above i think. I was considering just the decoding case as if painting can't keep up we'll no doubt see this reflected in the decoder skipping frames as well. Maybe both stats are needed.

(In reply to comment #13) > (In reply to comment #12) > > Access to mDecodeDecodedFrames needs to be protected by a lock or something. > > Right now it's written on a decoder thread and read on the main thread. > > It's protected by the mVideoUpdateLock lock when written but is missing it on > the read. I'll fix that. Is that the right lock to use or do you want one > specific for this purpose? That's fine. (In reply to comment #13) > This is because it is intended to measure whether the hardware can keep up > with the decoding of the resource rather than the painting of it. This is > useful for example when decoding the video and processing the video data via > a canvas. I don't think this would be included in a count that only counted > frames when painted. I think it would make sense to count canvas paints as "a paint" for the purpose of computing mozDisplayedFrames. If we did that, are there any usecases for counting the number of frames that were presented as the ImageContainer's "current frame"?

Robert, I need this bug landed to meet our quarterly goals. Thanks.

Comment on attachment 468617 [details] [diff] [review] Implements playback statistics on video and media elements We need to more precisely nail down what we measure here.

I'll take this on. I'll need perf stats working before I start the refactorings in bug 592833 and friends.

We discussed today what statistics we should report for our own internal perf testing. Here's what we came up with: mozParsedFrames - number of frames that have been demuxed and extracted out of the media. mozDecodedFrames - number of frames that have been decoded - converted into YCbCr. mozPresentedFrames - number of frames that have been presented to the rendering pipeline for rendering - were "set as the current image". mozPaintedFrames - number of frames which were presented to the rendering pipeline and ended up being painted on the screen. Note that if the video is not on screen (e.g. in another tab or scrolled off screen), this counter will not increase. mozPaintDelay - the time delay between presenting the last frame and it being painted on screen (approximately).

* Add the statistics I outlined in comment #18. * Change the state machine thread's sleep time after presenting a frame, so that we sleep until it's time to display the next frame, rather than sleeping for the duration of the frame. Without this change, our presentation time gets later and later, as because of timer latency we gradually wake up later and later in the frame "period", and the "lateness" compounds over time. * Minor change to nsWebMReader::DecodeVideoFrame()'s keyframe skipping logic to reflect that you can have multiple [key]frames per chunk per packet in a WebM file.

(In reply to comment #19) > Created attachment 495942 [details] [diff] [review] > Patch: playback statistics v2 > > * Add the statistics I outlined in comment #18. > * Change the state machine thread's sleep time after presenting a frame, so > that we sleep until it's time to display the next frame, rather than sleeping > for the duration of the frame. Without this change, our presentation time gets > later and later, as because of timer latency we gradually wake up later and > later in the frame "period", and the "lateness" compounds over time. > * Minor change to nsWebMReader::DecodeVideoFrame()'s keyframe skipping logic to > reflect that you can have multiple [key]frames per chunk per packet in a WebM > file. Can you split these into separate patches? The latter two patches might be regressy so we should make sure we can land them (or back them out) separately.

Adds playback statistics.

The nsWebMReader::DecodeVideoFrame() function assumes that there's always exactly 1 frame per chunk per packet. This may not necessarily be the case, though usually it is, and it would be silly to mux a WebM file in any other way. This patch ensures that we can report the playback statistics correctly and keyframe skip correctly on such files.

You should probably split out the playback statistics patch into a gfx-only patch that provides the latest-time-stamp functionality and the rest. + // Number of presented frames which were drawn on screen. + readonly attribute unsigned long mozPaintedFrames; You probably want to explain that presented frames may not be painted if the browser doesn't repaint the window frequently enough. + * Sets the time at which we'd like the contained to be image painted. ? Why do we need to track anything in ImageContainer other than a single TimeStamp containing the last paint time for the current image (or null if it hasn't been painted yet)? Why do an extra layer tree traversal in NotifyPainted? We can update the timestamp when we paint in each ImageLayer implementation.

Comment on attachment 496254 [details] [diff] [review] Patch v2 2/3: Reduce paint delay in video playback Wouldn't it be slightly simpler to have a local variable waitTime which is AUDIO_DURATION_MS by default and mPlayStartTime - mPlayDuration + TimeDuration::FromMilliseconds(videoData->mEndTime - mStartTime) - TimeStamp::Now() in the video case?

Comment on attachment 496258 [details] [diff] [review] Patch v2 3/3: Handle multiple frames per chunk per packet in WebM playback statistics. Seems like more of a Matthew patch to me!

(In reply to comment #22) > Created attachment 496254 [details] [diff] [review] > Patch v2 2/3: Reduce paint delay in video playback It'd be good to land this in FF4, it's a general bug fix.

OK, someone needs to address comment #25 then

(In reply to comment #27) > (In reply to comment #22) > > Created attachment 496254 [details] [diff] [review] > > Patch v2 2/3: Reduce paint delay in video playback > > It'd be good to land this in FF4, it's a general bug fix. I'll spin this off into its own bug, and we can land it by itself.

(In reply to comment #29) > I'll spin this off into its own bug, and we can land it by itself. Bug 634787.

So bug 633164 changed nsMediaDecoder::Invalidate() so that we only Invalidate() a video's frame if the image container size has changed. This causes us to call nsVideoFrame::BuildLayer() a lot less than once per painted frame (usually about 5 times per second for a 30fps video I'm testing on, though it varies). So I can't count the number of painted frames by counting the number of times that nsVideoFrame::BuildLayer() is called. How else can I count the number of video frames which get painted on screen?

Comment on attachment 496254 [details] [diff] [review] Patch v2 2/3: Reduce paint delay in video playback The changes made in patch 2 were landed in bug 634787.

I've split of the mozParsedFrames, mozDecodedFrames and mozPresentedFrames into this patch, since they're so much simpler to implement than mozPaintedFrames and mozFrameDelay. I'll implement those in subsequent patches.

Comment on attachment 514667 [details] [diff] [review] Patch 1: Demuxing/decoding stats Looks good to me but I want Matthew to check too.

Comment on attachment 514667 [details] [diff] [review] Patch 1: Demuxing/decoding stats Yes, I know Matthew technically isn't a super-reviewer :-)

+/* readonly attribute unsigned long mozDecodedFrames; */ +NS_IMETHODIMP nsHTMLVideoElement::GetMozParsedFrames(PRUint32 *aMozDecodedFrames) Comment and parameter name don't match member function name. Same for the dropped/decoded. And presented is missing a similar comment. Also, if mDecoder is null, doesn't it make more sense to return an error from these getters? + PRUint32 parsed=0, decoded=0; Pleaseusespaces. (Same issue in a second place, too.) + virtual PRBool DecodeVideoFrame(PRBool& aKeyframeSkip, + PRInt64 aTimeThreshold, + PRUint32& aParsed, + PRUint32& aDecoded) = 0; I think I'd prefer aParsed/aDecoded to be wrapped in a new stats struct and passed in as a single item, especially if we're going to add more in the future. As I read further, I see the stats have their own monitor, so this should definitely all be in a new struct with its own monitor, and passed around/fetched similar to the existing network/playback statistics. +#define IMPL_GET_STAT_METHOD(X) \ +PRUint32 nsMediaDecoder::Get##X##Frames() { \ + mozilla::MonitorAutoEnter mon(mStatsMonitor); \ + return m##X##Frames; \ +} + +IMPL_GET_STAT_METHOD(Parsed); +IMPL_GET_STAT_METHOD(Decoded); +IMPL_GET_STAT_METHOD(Presented); This seems unnecessary for three tiny methods, and it makes grepping for stuff a lot harder. Moving to a GetPlaybackStatistics() method like I suggested above would remove the need for separate getters, anyway.

(In reply to comment #36) > Also, if mDecoder is null, doesn't it make more sense to return an error from > these getters? I think probably not. If there is no decoder, then 0 is the correct answer. > As I read further, I see the stats have their own monitor, so this should > definitely all be in a new struct with its own monitor, and passed > around/fetched similar to the existing network/playback statistics. Actually, do we need the stats to have their own monitor? Why can't they use the decoder monitor?

(In reply to comment #37) > I think probably not. If there is no decoder, then 0 is the correct answer. Is that true if we've been playing and then DecodeError fires?

We can say that the counters are reset if a DecodeError fires.

Thanks for your comments Matthew and Roc. (In reply to comment #37) > (In reply to comment #36) > > Also, if mDecoder is null, doesn't it make more sense to return an error from > > these getters? > > I think probably not. If there is no decoder, then 0 is the correct answer. Yeah, if we don't have a decoder, we'll have decoded 0 frames. > Actually, do we need the stats to have their own monitor? Why can't they use > the decoder monitor? IIRC (and it was at least month since I tested this) when mozPaintedFrames and mozFrameDelay was implemented as per my original patch, using the decoder monitor caused a slight frame rate drop. (In reply to comment #39) > We can say that the counters are reset if a DecodeError fires. We only fire DecodeError (in the nsBuiltinDecoder architecture) upon failure in LoadMetadata(), so it's unlikely the counters will be very useful in this situation anyway. In the case where we've been playing and a DecodeError fires, it would make sense to keep the stats around, since we will have decoded and presented frames, but that can't happen currently.

(In reply to comment #40) > > Actually, do we need the stats to have their own monitor? Why can't they use > > the decoder monitor? > > IIRC (and it was at least month since I tested this) when mozPaintedFrames and > mozFrameDelay was implemented as per my original patch, using the decoder > monitor caused a slight frame rate drop. OK. Then I agree with Matthew, we should have a struct with a monitor containing the values it protects.

Adjusted with Matthews comments. Re-requesting review, since it's changed significantly.

Unbitrotten, based upon latest trunk and new Patch 1. Carrying forward r=kinetik.

(In reply to comment #24) > Why do we need to track anything in ImageContainer other than a single > TimeStamp containing the last paint time for the current image (or null if it > hasn't been painted yet)? If we only track the paint time of the current image (or null if it's not been painted yet) on the ImageContainer, what do we return in HTMLMediaElement.mozFrameDelay when the current image has not been painted yet? 0? But then we couldn't distinguish between no delay (e.g. perfect perf) and when we'd sampled before the frame had been painted. Alternatively if we returned the last returned value in this case, we may never get a non-zero value if we always poll before the current frame has been painted. If we store the paint target time on the ImageContainer, then we know what the delay is, and we can retain the delay of the previous image, and return that when the current image has not been painted yet. Alternatively we could have some kind of notification on the nsMediaDecoder when an Image is painted, and maintain the delay of the last painted frame there?

(In reply to comment #44) > If we only track the paint time of the current image (or null if it's not been > painted yet) on the ImageContainer, what do we return in > HTMLMediaElement.mozFrameDelay when the current image has not been painted yet? > 0? It should return the most recent delay. However, we can achieve this by having the video decoder store the last frame's delay internally just before it does SetCurrentImage on the next frame. Then mozFrameDelay can call into the decoder to get the delay, and that will return the current frame's delay if that's non-null, otherwise the last frame's delay.

* Adds paint time attribute to Image.

Implement mozFrameDelay and mozPaintCount. Not quite sure it's ready for review, posting so cjones can see what I'm doing to give me feedback on how to do this for shadow layers.

Currently the PLayers protocol is all one-way, child->parent (content->compositor). I think the cleanest way to implement statistics like this would be adding feedback parent->child on painting. This is potentially useful for other stuff too. So to http://mxr.mozilla.org/mozilla-central/source/gfx/layers/ipc/PLayers.ipdl#239, I'd suggest adding a child: Painted(PaintStats[] stats); message. PaintStats could be defined as a union of more specific stats, like http://mxr.mozilla.org/mozilla-central/source/gfx/layers/ipc/PLayers.ipdl#139. You would want to attach a PLayer to the specific stats, to know which Layer to notify in the child. To collect the stats, you would probably want to create a compositor-side interface in http://mxr.mozilla.org/mozilla-central/source/gfx/layers/ipc/ShadowLayers.cpp for ShadowLayerManager, modeled after the Transaction stuff for ShadowLayerForwarder. The compositor-side layer managers would "open" stats gathering at the beginning of a transaction, then "close" it at the end. (You would still want to close them for empty layers transactions, unlike the ShadowLayerForwarder Transactions which remain open.) Closing stats gathering would cause an IPC message to be sent to the child. If no stats were collected, no message would need to be sent. On the child side, you'd want to iterate though the stats and update individual layers. If all these stats are layer-backend agnostic, you could add generic processing like in http://mxr.mozilla.org/mozilla-central/source/gfx/layers/ipc/ShadowLayersParent.cpp#161. If they might be backend-specific, we'd need something like http://mxr.mozilla.org/mozilla-central/source/gfx/layers/basic/BasicLayers.cpp#2793. Some asides - It will be possible for PLayer:__delete__ to race with these Painted() messages, which will make crashy. We'll need to fix this by adding another step to the PLayer deletion protocol: add a PLayer:Destroy child->parent message that ends up triggering PLayer:__delete__ parent->child. - I think we'll need that for PLayers too. I'm not sure how coherent all that is. Let me know if I can help more. Also, IMHO landing same-process stats shouldn't block on cross-process stats.

Fixed two typo in comments, ready for review.

Comment on attachment 516083 [details] [diff] [review] Patch: Implement mozFrameDelay and mozPaintCount Note this patch is based on top of the patch "add paint time to Images".

I don't think we need the mPainted flag on the Image. We only need a flag on the ImageContainer to say whether the current image has been painted yet. In fact we don't even need a separate flag, since mPaintTime is null if and only if the current image hasn't been painted yet.

Note mPaintCount counts the number of Images which have been painted at least once, and video frames often end up being painted several times. We need the mPainted flag on the Image because it may lose it's "current image" status while painting, and in that case we still need to increment mPaintCount, but not set mPaintTime if this was the first time that Image was painted. But we can't tell if this is the first time we've painted the (now non-current) Image if we reset mPaintTime when the current image changed. If an Image has been painted, and the current image changes while we were painting the Image for a second time, mPaintTime will be reset. If we assume we should increment mPaintCount because mPaintTime is null, we'll end up incrementing mPaintCount twice for that frame.

We could instead have a flag on the ImageContainer to store whether its previous image was painted. That basically would need to be set to !mPaintTime.IsNull() when the current image changes, that would handle the problematic case.

Let's do that then.

+ * Implementations must call CurrentImageChanged() in a threadsafe manner. What does "in a threadsafe manner" mean? + * has not yet been painted. Threadsafe. I think it's clearer to say "can be called on any thread". + PRBool mPreviousImagePainted; PRPackedBool Maybe we should just bite the bullet and move the monitor in each implementation up to the superclass?

Merging ImageContainers' monitors are suggested.

Review comments addressed. Based on top of "Patch: Push ImageContainer subclass' locks up into a superclass monitor.".

Comment on attachment 516418 [details] [diff] [review] Patch: Push ImageContainer subclass' locks up into a superclass monitor. Looks great, but please change the comments in ImageLayers.h to mention the monitor and say which methods take the monitor.

http://hg.mozilla.org/mozilla-central/rev/8c22e163c9f0 http://hg.mozilla.org/mozilla-central/rev/d6f8aff170a0 http://hg.mozilla.org/mozilla-central/rev/3263ffab7ce7 http://hg.mozilla.org/mozilla-central/rev/48a74f63565e http://hg.mozilla.org/mozilla-central/rev/8b6d6b01cbdf

The following is what I see in the latest nightly: Framerate is: 30.01 Dropped frames: undefined Download rate: NaN Playback rate: NaN Is this expected?

dropped.innerHTML = v1.mozDroppedFrames; playback.innerHTML = Math.round(v1.mozDecodeRate); download.innerHTML = Math.round(v1.mozDownloadRate); Probably just typos, since these properties don't exist on the video element. However, I can verify for you that these stats are indeed working on the video element, as I'm working on a visualization that uses them now.

Thanks, David.

Comment on attachment 468611 [details] HTML file example to display dropped frames, download rate and other stats That example is for the original stats produced by the original patch. For a demo which uses the current stats, see: http://people.mozilla.org/~cpearce/paint-stats-demo.html

(In reply to comment #64) > Comment on attachment 468611 [details] > HTML file example to display dropped frames, download rate and other stats > > That example is for the original stats produced by the original patch. For a > demo which uses the current stats, see: > http://people.mozilla.org/~cpearce/paint-stats-demo.html Looks good to me.

https://developer.mozilla.org/en/DOM/HTMLVideoElement https://developer.mozilla.org/en/Firefox_5_for_developers#DOM