[MSE] Add support for VideoPlaybackMetrics.
Created attachment 218653 [details] Patch
Comment on attachment 218653 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=218653&action=review > Source/WebCore/Modules/mediasource/VideoPlaybackQuality.idl:28 > + NoInterfaceObject, Why no NoInterfaceObject? > Source/WebCore/Modules/mediasource/VideoPlaybackQuality.idl:31 > + readonly attribute double creationTime; The spec says this should be a DOMHighResTimeStamp? Does the generator not support that? > Source/WebCore/html/HTMLMediaElement.cpp:5250 > +#if ENABLE(WEB_TIMING) > + DOMWindow* domWindow = document().domWindow(); > + Performance* performance = domWindow ? domWindow->performance() : nullptr; > + double now = performance ? performance->now() : 0; > +#else > + double now = monotonicallyIncreasingTime(); > +#endif I believe performance()->now() is implemented as: 1000.0 * document()->loader()->timing()->monotonicTimeToZeroBasedDocumentTime(monotonicallyIncreasingTime()) So you should probably match that in the non-WEB_TIMING enabled case.
Created attachment 218678 [details] Patch
Comment on attachment 218678 [details] Patch Attachment 218678 [details] did not pass efl-ews (efl): Output: http://webkit-queues.appspot.com/results/46948080
Comment on attachment 218678 [details] Patch Attachment 218678 [details] did not pass gtk-ews (gtk): Output: http://webkit-queues.appspot.com/results/47008090
Comment on attachment 218678 [details] Patch Attachment 218678 [details] did not pass efl-wk2-ews (efl-wk2): Output: http://webkit-queues.appspot.com/results/46948081
Comment on attachment 218678 [details] Patch Attachment 218678 [details] did not pass win-ews (win): Output: http://webkit-queues.appspot.com/results/46938079
Created attachment 218699 [details] Patch Protect getVideoPlaybackQuality with ENABLE guards.
Comment on attachment 218699 [details] Patch Attachment 218699 [details] did not pass efl-ews (efl): Output: http://webkit-queues.appspot.com/results/47138059
Comment on attachment 218699 [details] Patch Attachment 218699 [details] did not pass efl-wk2-ews (efl-wk2): Output: http://webkit-queues.appspot.com/results/47188069
Comment on attachment 218699 [details] Patch Attachment 218699 [details] did not pass gtk-ews (gtk): Output: http://webkit-queues.appspot.com/results/46908082
Comment on attachment 218699 [details] Patch Attachment 218699 [details] did not pass win-ews (win): Output: http://webkit-queues.appspot.com/results/46638075
Created attachment 218706 [details] Patch Also protect the m_droppedVideoFrames initializer.
Comment on attachment 218706 [details] Patch Attachment 218706 [details] did not pass gtk-ews (gtk): Output: http://webkit-queues.appspot.com/results/47128078
Created attachment 218765 [details] Patch Add new files to cmakelists.txt
Comment on attachment 218765 [details] Patch Attachment 218765 [details] did not pass gtk-ews (gtk): Output: http://webkit-queues.appspot.com/results/45898143
Created attachment 218768 [details] Patch Add VideoPlaybackQuality.idl to GNUMakefile.list.am.
Comment on attachment 218768 [details] Patch Attachment 218768 [details] did not pass gtk-ews (gtk): Output: http://webkit-queues.appspot.com/results/46278167
Comment on attachment 218768 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=218768&action=review > Source/WebCore/ChangeLog:52 > + (WebCore::SourceBuffer::TrackBuffer::TrackBuffer): needsRandomAccessFlag defaulst to true. Nit: "defaulst". > LayoutTests/media/media-source/media-source-video-playback-quality.html:17 > + function runTest() { Nit: a function's opening brace should be on a new line.
Comment on attachment 218768 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=218768&action=review > Source/WebCore/html/HTMLMediaElement.cpp:5252 > +#if ENABLE(WEB_TIMING) > + DOMWindow* domWindow = document().domWindow(); > + Performance* performance = domWindow ? domWindow->performance() : nullptr; > + double now = performance ? performance->now() : 0; > +#else > + double now = monotonicallyIncreasingTime(); > +#endif In an earlier review, Sam said: I believe performance()->now() is implemented as: 1000.0 * document()->loader()->timing()->monotonicTimeToZeroBasedDocumentTime(monotonicallyIncreasingTime()) So you should probably match that in the non-WEB_TIMING enabled case.
Created attachment 218797 [details] Patch Addressed review comments, and yet another attempt to fix the GTK bot.
Comment on attachment 218797 [details] Patch Clearing flags on attachment: 218797 Committed r160336: <http://trac.webkit.org/changeset/160336>
All reviewed patches have been landed. Closing bug.
(In reply to comment #2) > (From update of attachment 218653 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=218653&action=review Sam, sorry this got lost in the blizzard of build failure messages. > > Source/WebCore/Modules/mediasource/VideoPlaybackQuality.idl:28 > > + NoInterfaceObject, > > Why no NoInterfaceObject? I had thought this meant no window-level constructors, but that's not correct. It means no object prototype, and therefore the object can't be extended by page authors. I'll put up a follow-up patch to remove this. > > Source/WebCore/Modules/mediasource/VideoPlaybackQuality.idl:31 > > + readonly attribute double creationTime; > > The spec says this should be a DOMHighResTimeStamp? Does the generator not support that? The spec says a DOMHighResTimeStamp is a double. Since that's what the Performance.idl file lists as the return value as "now", that's what I used here. And now that I look, there doesn't seem to be support in the generator for that type. > > Source/WebCore/html/HTMLMediaElement.cpp:5250 > > +#if ENABLE(WEB_TIMING) > > + DOMWindow* domWindow = document().domWindow(); > > + Performance* performance = domWindow ? domWindow->performance() : nullptr; > > + double now = performance ? performance->now() : 0; > > +#else > > + double now = monotonicallyIncreasingTime(); > > +#endif > > I believe performance()->now() is implemented as: > > 1000.0 * document()->loader()->timing()->monotonicTimeToZeroBasedDocumentTime(monotonicallyIncreasingTime()) > > So you should probably match that in the non-WEB_TIMING enabled case. Okay, I did this in the committed patch. I'll post a follow up patch to address the NoInterfaceObject change above.
Re-opening.
Actually, I'll address the follow up patch in a new bug: http://webkit.org/b/125474.