Clarify invariants around `CurrentDriver`
Categories
(Core :: Audio/Video: MediaStreamGraph, enhancement)
Tracking
()
People
(Reporter: padenot, Unassigned)
Details
From https://phabricator.services.mozilla.com/D80836#2486510
In D80836#2466923, @karlt wrote:
Ah, I assume CurrentDriver() was returning null in
CachedAudioOutputLatency().Perhaps this case could instead be handled there, if
CachedAudioOutputLatency() might be used elsewhere one day.CurrentDriver() that can return nullptr when not on the render thread is quite
an invariant change. We should consider renaming it in the usual Gecko style
for getters, or having two versions, one CurrentDriver that can never fail and
is intended to be called on the render thread, and GetCurrentDriver that is
used from other threads and this would need to be null-checked.
|
||
Comment 2•6 years ago
•
|
||
Yes, the naming is not so good given CurrentDriver() can return null in some cases. In some ways that's similar to many similarly named methods returning null after an object is unlinked, but its different because the graph can be shutdown before other objects are finished with the graph.
The two versions, for on and off render thread, seem a reasonable option to me.
I also wonder about an alternative of having the MTG hold onto a driver for longer so that we don't get nullptr from CurrentDriver() until there are no tracks in the graph. The expectation is that a graph is only ever used when a client owns a track, and Andreas made some changes to align with this model. If doing that then an AudioCallbackDriver would need to release its SharedThreadPool on Shutdown() for the same reasons, so I guess there may be some null checks required in AudioCallbackDriver. Perhaps there might be other consequences of holding onto a driver past shutdown that I haven't considered.
Description
•