1493613 - Make MediaStreamTrack the controller of MediaStreamGraph units

Status: RESOLVED FIXED

(Core :: Audio/Video: MediaStreamGraph, enhancement, P3)

Description

[Andreas Pehrson [:pehrsons]]

We want to remove MediaStreams from the MediaStreamGraph and only media for tracks. This makes all MediaStreamGraph units (tracks) independent and we can drop a fair bit of processing overhead.

The grouping of tracks into streams will happen on main thread in (currently) DOMMediaStream.

A/V sync will initially be handled by an MSG being run by one clock, and two tracks processed together must be from the same MSG.

Comment 1

[Andreas Pehrson [:pehrsons]]

This went pretty fast. 4 days to write and 1 day of debugging to fix things to make it pass locally. Probably some ironing out to do for try oranges still, but expect a patch soon.

Comment 2

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1493613 - Remove assert that next driver must be non-audio when no audio tracks are present. r?padenot

A legit case that fails this assert is:

• CloseAudioInput() on main thread for last non-webaudio MediaStream
• AudioContext closes on main thread
• CloseAudioInputImpl() on graph thread sets next driver to an output-only audio
  driver since there are AudioNodeStreams still present
• AudioContext's Close operation is applied on graph thread, first all
  AudioNodeStreams are suspended, making the graph consider itself as having no
  audio tracks present. Then we check the next driver, which is an audio driver
  per above. This fails the assert.

Comment 3

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1493613 - Clear mUpdateTracks in DestroyImpl to avoid leaking image buffers until CC shutdown. r?padenot

Normally a track in mUpdateTracks is cleared by ExtractPendingInput, when that
track's ending is processed. However, if the SourceMediaStream is destroyed
before an ended track is processed, the track including it's buffered segment
in mUpdateTracks will leak until the SourceMediaStream is destroyed.

This might not be until late XPCOM Shutdown when the cycle collector shuts down,
which is too late to release graphics resources.

Depends on D37931

Comment 4

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1493613 - Update muted state through MediaStreamTrackSource. r?bwc

This ensures all clones of the original track also receives the new muted state.

Depends on D37932

Comment 5

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1493613 - Move MediaStream control from DOMMediaStream to MediaStreamTrack. r?padenot

This is inherently large, because modifying these bits of DOMMediaStream and
MediaStreamTrack affects all consumers and producers of all DOMMediaStreams and
MediaStreamTracks.

Things are generally much simpler now.

Producers of tracks now create a MediaStream in the graph, add it to a
MediaStreamTrackSource subclass that takes ownership of it, and add the source
to a MediaStreamTrack. Should the producer need a DOMMediaStream it is now much
simpler to create as the only thing needed is the current window. The stream is
a rather simple wrapper around an array of MediaStreamTracks.

HTMLMediaElement is still not as straight forward as other consumers since it
consumes the DOMMediaStream directly, as opposed to a set of tracks.
The new MediaStreamRenderer helper class helps bridge the gap between this fact
and the new track-based MediaStreamGraph interface, as it needs to juggle
registering multiple audio tracks for audio output. This hooks into existing
HTMLMediaElement logic and brings a welcome simplification to all the glue
previously needed there.

Depends on D37933

Comment 6

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1493613 - Remove TracksCreatedListener. r?padenot

It could lead to a ref-cycle leak if it was added as listener to a
MediaStreamTrack but the underlying track in the graph was never created, so
that the TracksCreatedListener never received NotifyOutput or NotifyRemoved
events.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/146464e21594
Remove assert that next driver must be non-audio when no audio tracks are present. r=padenot
https://hg.mozilla.org/integration/autoland/rev/cf84eeffed22
Clear mUpdateTracks in DestroyImpl to avoid leaking image buffers until CC shutdown. r=padenot
https://hg.mozilla.org/integration/autoland/rev/0b03dd9d20ac
Update muted state through MediaStreamTrackSource. r=bwc,smaug
https://hg.mozilla.org/integration/autoland/rev/c54cb3c10992
Move MediaStream control from DOMMediaStream to MediaStreamTrack. r=padenot
https://hg.mozilla.org/integration/autoland/rev/cab1435d2f33
Remove TracksCreatedListener. r=padenot

Comment 8

[Alexandru Michis [:malexandru]]

https://hg.mozilla.org/mozilla-central/rev/146464e21594
https://hg.mozilla.org/mozilla-central/rev/cf84eeffed22
https://hg.mozilla.org/mozilla-central/rev/0b03dd9d20ac
https://hg.mozilla.org/mozilla-central/rev/c54cb3c10992
https://hg.mozilla.org/mozilla-central/rev/cab1435d2f33

Comment 9

[guest271314]

This issue and https://bugzilla.mozilla.org/show_bug.cgi?id=1454998 are referenced at https://searchfox.org/mozilla-central/source/dom/media/webaudio/MediaStreamAudioDestinationNode.cpp#94 through line #102.

Firefox implementation of MediaStreamAudioDestinationNode is useful for capturing the output of the node and all audio nodes that are subsequently connected to the MediaStreamAudioDestinationNode, as we can capture the output as soon as the node is created and playback commences at HTMLMediaElement, for example using parec and pactl at the command line.

Chromium implementation of MediaStreamAudioDestinationNode does not advance currentTime beyond 0 when no input audio nodes are connected to the MediaStreamAudioDestinationNode.

It is not immediately clear which implementation is correct per the specification. The specification does not clearly state that a live MediaStreamTrack from MediaStreamAudioDestinationNode must advance HTMLMediaElement currentTime even when no inputs are connected to the node.

Which is the correct implementation https://github.com/WebAudio/web-audio-api/issues/2293 ?

Comment 10

[Paul Adenot (:padenot)]

Spec is: https://w3c.github.io/mediacapture-main/getusermedia.html#mediastreams-in-media-elements, you can see in the description of currentTime that it should be linearly increasing whenever the stream is effectively playing.