To help tweak the decoder-suspend functionality, we want to measure how long it takes to recover from the video being suspended. Telemetry will be keyed by audio presence ('AV' with audio, 'V' video only), hardware acceleration '(hw)', and by resolution ranges (e.g. '721-1080'). An 'All' key will also accumulate all numbers.

Comment on attachment 8779989 [details] Bug 1294349 - Histogram for VIDEO_SUSPEND_RECOVERY_TIME_MS - https://reviewboard.mozilla.org/r/70850/#review68244

Comment on attachment 8779990 [details] Bug 1294349 - Report VIDEO_SUSPEND_RECOVERY_TIME_MS - https://reviewboard.mozilla.org/r/70852/#review68246 ::: dom/media/MediaDecoderStateMachine.cpp:1467 (Diff revision 1) > + // Local reference to mInfo, so that it will be copied in the lambda below. > + MediaInfo& info = mInfo; > + > + bool hw = mReader->VideoIsHardwareAccelerated(); > + > // Do the seek. > mSeekTaskRequest.Begin( > - mSeekTask->Seek(Duration())->Then(OwnerThread(), __func__, this, > + mSeekTask->Seek(Duration()) > + ->Then(OwnerThread(), __func__, this, > - &MediaDecoderStateMachine::OnSeekTaskResolved, > + &MediaDecoderStateMachine::OnSeekTaskResolved, > - &MediaDecoderStateMachine::OnSeekTaskRejected)); > + &MediaDecoderStateMachine::OnSeekTaskRejected) > + ->CompletionPromise() > + ->Then(AbstractThread::MainThread(), __func__, > + [start, info, hw](){ ReportRecoveryTelemetry(start, info, hw); }, > + [](){})); bug 1294352 merged InitiateDecodeRecoverySeek() with InitiateSeek(). You might encounter merging conflict while landing.

Comment on attachment 8779990 [details] Bug 1294349 - Report VIDEO_SUSPEND_RECOVERY_TIME_MS - https://reviewboard.mozilla.org/r/70852/#review68246 > bug 1294352 merged InitiateDecodeRecoverySeek() with InitiateSeek(). You might encounter merging conflict while landing. Yes I saw that bug pop up on my radar, I'll follow up when it lands... Thank you for the reviews.

Comment on attachment 8779989 [details] Bug 1294349 - Histogram for VIDEO_SUSPEND_RECOVERY_TIME_MS - https://reviewboard.mozilla.org/r/70850/#review68860 datareview+

Pushed by gsquelart@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/17306a183ba3 Histogram for VIDEO_SUSPEND_RECOVERY_TIME_MS - r=francois,kaku https://hg.mozilla.org/integration/autoland/rev/3599d88d2b24 Report VIDEO_SUSPEND_RECOVERY_TIME_MS - r=kaku

https://hg.mozilla.org/mozilla-central/rev/17306a183ba3 https://hg.mozilla.org/mozilla-central/rev/3599d88d2b24

Comment on attachment 8779990 [details] Bug 1294349 - Report VIDEO_SUSPEND_RECOVERY_TIME_MS - https://reviewboard.mozilla.org/r/70852/#review71944 ::: dom/media/MediaDecoderStateMachine.cpp:1413 (Diff revision 4) > mVideoDecodeSuspendTimer.Reset(); > > if (mVideoDecodeSuspended) { > mVideoDecodeSuspended = false; > mOnPlaybackEvent.Notify(MediaEventType::ExitVideoSuspend); > mReader->SetVideoBlankDecode(false); Sorry, Gerald, I went thorugh this patch again while I was studying the telemetry data and I think that we should start counting the time from this line on so that it includes (exactlly) the time used to switch decoder. Althought the telemetry looks sensable now and I think it is unlikely that the switching-decoder-task been ran between this line and the line we intialize the seek. WDYT?