Closed Bug 1529738 Opened 6 months ago Closed 6 months ago

Add Gecko Profiler markers to report when video decode skips to next keyframe

Categories

(Core :: Audio/Video: Playback, enhancement, P2)

enhancement

Tracking

()

RESOLVED FIXED
mozilla67
Tracking Status
firefox67 --- fixed

People

(Reporter: cpearce, Assigned: cpearce)

References

Details

Attachments

(1 file)

For debugging playback issues, it would be handy to have profiler markers when we skip to next keyframe. That would help identify the points where decode is struggling.

10:31:55 <cpearce> mstange: we should add profiler markers for when video playback drops frames... Can you suggest an example marker that we could look at figure out how to do this?
10:33:53 <mstange> cpearce: sure! do you want a duration marker ("frames were dropped between this timestamp and that timestamp") or an event marker ("frames were dropped at this point")?
10:34:39 <cpearce> mstange: probably just an event marker, as it's not really a long running task.
10:35:20 <mstange> cpearce: ok. do you want to put custom data into it, or is it just a string that's always the same?
10:36:15 <mstange> if the latter, you can just do #include "GeckoProfiler.h", PROFILER_ADD_MARKER("Video frames dropped", GRAPHICS);
10:36:48 <cpearce> mstange: it would potentially be useful to report the number of frames dropped, but it's not critical; when this happens, we skip the decode forward to the next keyframe, so it would be a variable number of frames... usually the number of frames skipped isn't interesting though, so a static string to start with should be fine.
10:37:14 <cpearce> mstange: ok, that sounds easy.
10:37:18 <mstange> cool
10:37:30 <cpearce> mstange: is it safe to call off main thread?
10:37:43 <mstange> yes, it's safe to call anywhere
10:37:50 <cpearce> Awesome. Easy as.
10:39:32 <mstange> if you want to put more information in it, one option is to use a text marker in combination with nsPrintfCString, here's an example: https://searchfox.org/mozilla-central/rev/b36e97fc776635655e84f2048ff59f38fa8a4626/dom/base/nsDOMNavigationTiming.cpp#485-497

See Also: → 1529795

Markus: The markers I've added in this patch are not showing up in the profiler, even when I'm sampling the "Media" threads. These run on one of the "MediaPlayback #N" threads, which should be being sampled... I also often don't see the "MediaPlayback" threads show up as sampled in the profiler. Can you take a look? I see the MediaPDecoder threads appear in the profile.

Flags: needinfo?(mstange)

Can you link me to an example profile? What's probably happening is that the relevant MediaPlayback threads were hidden by default because they're idle for most of their run time. You can unhide them by finding them in the threads context menu.

Flags: needinfo?(mstange) → needinfo?(cpearce)

I didn't realise I could unhide mostly idle threads, thanks for that tip. Unfortunately, even if I do that, I still don't see the markers being reported. My logging confirms that the markers are being reported, and that profiler_thread_is_being_profiled() returns true on the thread upon which we're reporting the markers.

http://bit.ly/2Xx3FIK

Flags: needinfo?(cpearce) → needinfo?(mstange)

Oh oops, I see the issue in the patch. You're using AUTO_PROFILER_LABEL instead of PROFILER_ADD_MARKER. The former will only be visible in the call tree for the samples that are collected while code inside this scope is running. The latter will add a marker.

Flags: needinfo?(mstange)

(In reply to Markus Stange [:mstange] from comment #5)

Oh oops, I see the issue in the patch. You're using AUTO_PROFILER_LABEL instead of PROFILER_ADD_MARKER.

That's it! Thanks!

Assignee: nobody → cpearce
Status: NEW → ASSIGNED
Rank: 15
Priority: -- → P2
Pushed by cpearce@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/ec57fe6f4e17
Add profiler markers for video frames dropped due to slow video decode. r=jya,mstange
Status: ASSIGNED → RESOLVED
Closed: 6 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla67
You need to log in before you can comment on or make changes to this bug.