Closed Bug 1493613 Opened 6 years ago Closed 5 years ago

Make MediaStreamTrack the controller of MediaStreamGraph units

Categories

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

60 Branch
enhancement

Tracking

()

RESOLVED FIXED
mozilla70
Tracking Status
firefox70 --- fixed

People

(Reporter: pehrsons, Assigned: pehrsons)

References

(Blocks 1 open bug)

Details

Attachments

(5 files)

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.
Rank: 25
Priority: -- → P3
Assignee: nobody → apehrson
Status: NEW → ASSIGNED

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.

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.

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

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

Depends on D37932

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

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.

Pushed by pehrsons@gmail.com: 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
Regressions: 1570594
Regressions: 1570684
Regressions: 1570900
Regressions: 1571004
Regressions: 1571705
Depends on: 1573102
Regressions: 1573536
Regressions: 1573524
Depends on: 1577505
Regressions: 1570287
Regressions: 1587248

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 ?

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.

You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: