1454998 - Remove the notion of streams from the MediaStreamGraph

Status: RESOLVED FIXED

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

Description

[Andreas Pehrson [:pehrsons]]

This will be huge, but will greatly simplify the MSG.

With all producers and consumers using track apis and listeners to interact with streams, we will be able to remove MediaStreams from the graph altogether.

The grouping of tracks into a stream should happen only on main thread.

The complex graph hierarchy that supports cloning, addTrack, removeTrack, etc. [1] can then be simplified by cutting out the mPlaybackStream layer. We need to keep one extra layer on top of the source to support cloning and individual disabling of the tracks.

The layers would move from DOMMediaStream to MediaStreamTrack and likely be mSourceTrack and mPlaybackTrack, where mSourceTrack is optional.


[1] https://searchfox.org/mozilla-central/rev/f65d7528e34ef1a7665b4a1a7b7cdb1388fcd3aa/dom/media/DOMMediaStream.h#101-136

Comment 1

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1454998 - Remove the notion of streams from MediaStreamGraph. r?padenot!,karlt!

Comment 2

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1454998 - Rename streams to tracks. r?padenot!,karlt!,smaug!

Depends on D45821

Comment 3

[Andreas Pehrson [:pehrsons]]

The patches are huge, but getting them green was a breeze.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=e3eaffaecf200c24ce15a286dd6c1a8e82c7cdeb

Comment 4

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1454998 - Move TrackInfo::mTrackId away from TrackID as they're independent. r?karlt

TrackID has been used to denote tracks that are part of MediaStreams in the
MediaStreamGraph. The TrackInfo::mTrackId IDs are no longer used in the
MediaStream APIs and as such it's not necessary that they have the type TrackID.

This patch changes TrackInfo::mTrackId to be of type uint32_t instead, as that
is the type of Mp4parseTrackInfo::track_id, which is assigned to
TrackInfo::mTrackId as the sole remaining non-static user.

Other users set a static trackId of 1 or 2, and as such remain supported by
this change.

Comment 5

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1454998 - Break out PrincipalHandle into its own file. r?padenot

Depends on D46763

Comment 6

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1454998 - Remove track blocking from MediaInputPort. r?karlt

This functionality is not used since we moved to only having a single track per
MediaStream (bug 1493613).

Depends on D46764

Comment 7

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1454998 - Remove calls to SetStreamOrderDirty() after affecting MediaStreams' finished state. r?karlt

The stream order does not depend on streams' finished state, so these calls are
unnecessary.

Depends on D47686

Comment 8

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1454998 - Remove AudioNodeStream::SampleRate(). r?karlt

AudioNodeStream is a subclass of MediaStream, which now exposes a
public const mSampleRate member.

Depends on D45821

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6105a4176729
Move TrackInfo::mTrackId away from TrackID as they're independent. r=karlt
https://hg.mozilla.org/integration/autoland/rev/3ce4dc7e9ae2
Break out PrincipalHandle into its own file. r=padenot
https://hg.mozilla.org/integration/autoland/rev/d0e8d413cd1c
Remove track blocking from MediaInputPort. r=karlt
https://hg.mozilla.org/integration/autoland/rev/ab721cb2066b
Remove calls to SetStreamOrderDirty() after affecting MediaStreams' finished state. r=karlt
https://hg.mozilla.org/integration/autoland/rev/ae6056b748d1
Remove the notion of streams from MediaStreamGraph. r=padenot,karlt
https://hg.mozilla.org/integration/autoland/rev/8ff03f2f4ca2
Remove AudioNodeStream::SampleRate(). r=karlt
https://hg.mozilla.org/integration/autoland/rev/80417bdfa721
Rename streams to tracks. r=padenot,karlt,smaug

Comment 10

[Raul Gurzau (:RaulG)]

Backed out 7 changesets (bug 1454998) for build bustages at MediaTrackGraph.h on a CLOSED TREE.

Backout link: https://hg.mozilla.org/integration/autoland/rev/5f5e153eb14b8ecd44b5453e7a4a9e53e95193dd

**Push with failures:**https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception&revision=80417bdfa72112c6f9472c29ce49e8ff81e8c688&selectedJob=269383949

Log link: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=269383949&repo=autoland&lineNumber=20447

Log snippet:
[task 2019-10-02T08:38:13.611Z] 08:38:13 INFO - make[4]: Entering directory '/builds/worker/workspace/build/src/obj-firefox/extensions/permissions'
[task 2019-10-02T08:38:13.611Z] 08:38:13 INFO - extensions/permissions/Unified_cpp_permissions0.o
[task 2019-10-02T08:38:13.611Z] 08:38:13 INFO - make[4]: Leaving directory '/builds/worker/workspace/build/src/obj-firefox/extensions/permissions'
[task 2019-10-02T08:38:13.961Z] 08:38:13 INFO - make[4]: Entering directory '/builds/worker/workspace/build/src/obj-firefox/storage/test/gtest'
[task 2019-10-02T08:38:13.962Z] 08:38:13 INFO - /builds/worker/fetches/sccache/sccache /builds/worker/fetches/gcc/bin/g++ -o Unified_cpp_storage_test_gtest0.o -c -I/builds/worker/workspace/build/src/obj-firefox/dist/stl_wrappers -I/builds/worker/workspace/build/src/obj-firefox/dist/system_wrappers -include /builds/worker/workspace/build/src/config/gcc_hidden.h -DNDEBUG=1 -DTRIMMED=1 -DSTATIC_EXPORTABLE_JS_API -DMOZ_HAS_MOZGLUE -DMOZILLA_INTERNAL_API -DIMPL_LIBXUL -I/builds/worker/workspace/build/src/storage/test/gtest -I/builds/worker/workspace/build/src/obj-firefox/storage/test/gtest -I/builds/worker/workspace/build/src/storage -I/builds/worker/workspace/build/src/obj-firefox/dist/include -I/builds/worker/workspace/build/src/obj-firefox/dist/include/nspr -I/builds/worker/workspace/build/src/obj-firefox/dist/include/nss -fPIC -DMOZILLA_CLIENT -include /builds/worker/workspace/build/src/obj-firefox/mozilla-config.h -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -fstack-protector-strong -Wall -Wempty-body -Wignored-qualifiers -Woverloaded-virtual -Wpointer-arith -Wsign-compare -Wtype-limits -Wunreachable-code -Wwrite-strings -Wno-invalid-offsetof -Wduplicated-cond -Wno-error=maybe-uninitialized -Wno-error=deprecated-declarations -Wno-error=array-bounds -Wno-error=coverage-mismatch -Wno-error=free-nonheap-object -Wformat -D_GLIBCXX_USE_CXX11_ABI=0 -fno-sized-deallocation -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -fstack-protector-strong -fno-exceptions -fno-strict-aliasing -fno-rtti -ffunction-sections -fdata-sections -fno-exceptions -fno-math-errno -pthread -pipe -g -freorder-blocks -O2 -fno-omit-frame-pointer -funwind-tables -Werror -MD -MP -MF .deps/Unified_cpp_storage_test_gtest0.o.pp Unified_cpp_storage_test_gtest0.cpp
[task 2019-10-02T08:38:13.962Z] 08:38:13 INFO - make[4]: Leaving directory '/builds/worker/workspace/build/src/obj-firefox/storage/test/gtest'
[task 2019-10-02T08:38:13.969Z] 08:38:13 INFO - make[4]: Entering directory '/builds/worker/workspace/build/src/obj-firefox/extensions/permissions/test/gtest'
[task 2019-10-02T08:38:13.969Z] 08:38:13 INFO - mkdir -p '.deps/'
[task 2019-10-02T08:38:13.969Z] 08:38:13 INFO - make[4]: Leaving directory '/builds/worker/workspace/build/src/obj-firefox/extensions/permissions/test/gtest'
[task 2019-10-02T08:38:13.969Z] 08:38:13 INFO - make[4]: Entering directory '/builds/worker/workspace/build/src/obj-firefox/extensions/permissions/test/gtest'
[task 2019-10-02T08:38:13.970Z] 08:38:13 INFO - extensions/permissions/test/gtest/Unified_cpp_test_gtest0.o
[task 2019-10-02T08:38:13.970Z] 08:38:13 INFO - make[4]: Leaving directory '/builds/worker/workspace/build/src/obj-firefox/extensions/permissions/test/gtest'
[task 2019-10-02T08:38:14.066Z] 08:38:14 INFO - make[4]: Entering directory '/builds/worker/workspace/build/src/obj-firefox/xpcom/components'
[task 2019-10-02T08:38:14.066Z] 08:38:14 INFO - /builds/worker/fetches/sccache/sccache /builds/worker/fetches/gcc/bin/g++ -o StaticComponents.o -c -I/builds/worker/workspace/build/src/obj-firefox/dist/stl_wrappers -I/builds/worker/workspace/build/src/obj-firefox/dist/system_wrappers -include /builds/worker/workspace/build/src/config/gcc_hidden.h -DNDEBUG=1 -DTRIMMED=1 -DOS_POSIX=1 -DOS_LINUX=1 -DSTATIC_EXPORTABLE_JS_API -DMOZ_HAS_MOZGLUE -DMOZILLA_INTERNAL_API -DIMPL_LIBXUL -I/builds/worker/workspace/build/src/xpcom/components -I/builds/worker/workspace/build/src/obj-firefox/xpcom/components -I/builds/worker/workspace/build/src/obj-firefox/xpcom -I/builds/worker/workspace/build/src/xpcom/base -I/builds/worker/workspace/build/src/xpcom/build -I/builds/worker/workspace/build/src/xpcom/ds -I/builds/worker/workspace/build/src/chrome -I/builds/worker/workspace/build/src/js/xpconnect/loader -I/builds/worker/workspace/build/src/layout/build -I/builds/worker/workspace/build/src/modules/libjar -I/builds/worker/workspace/build/src/obj-firefox/ipc/ipdl/_ipdlheaders -I/builds/worker/workspace/build/src/ipc/chromium/src -I/builds/worker/workspace/build/src/ipc/glue -I/builds/worker/workspace/build/src/obj-firefox/dist/include -I/builds/worker/workspace/build/src/obj-firefox/dist/include/nspr -I/builds/worker/workspace/build/src/obj-firefox/dist/include/nss -fPIC -DMOZILLA_CLIENT -include /builds/worker/workspace/build/src/obj-firefox/mozilla-config.h -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -fstack-protector-strong -Wall -Wempty-body -Wignored-qualifiers -Woverloaded-virtual -Wpointer-arith -Wsign-compare -Wtype-limits -Wunreachable-code -Wwrite-strings -Wno-invalid-offsetof -Wduplicated-cond -Wno-error=maybe-uninitialized -Wno-error=deprecated-declarations -Wno-error=array-bounds -Wno-error=coverage-mismatch -Wno-error=free-nonheap-object -Wformat -D_GLIBCXX_USE_CXX11_ABI=0 -fno-sized-deallocation -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -fstack-protector-strong -fno-exceptions -fno-strict-aliasing -fno-rtti -ffunction-sections -fdata-sections -fno-exceptions -fno-math-errno -pthread -pipe -g -freorder-blocks -O2 -fno-omit-frame-pointer -funwind-tables -Werror -I/builds/worker/workspace/build/src/widget/gtk/compat-gtk3 -pthread -I/usr/include/gtk-3.0 -I/usr/include/pango-1.0 -I/usr/include/gio-unix-2.0/ -I/usr/include/atk-1.0 -I/usr/include/cairo -I/usr/include/gdk-pixbuf-2.0 -I/usr/include/freetype2 -I/usr/include/glib-2.0 -I/usr/lib/x86_64-linux-gnu/glib-2.0/include -I/usr/include/pixman-1 -I/usr/include/libpng12 -I/usr/include/gtk-3.0/unix-print -MD -MP -MF .deps/StaticComponents.o.pp StaticComponents.cpp
[task 2019-10-02T08:38:14.066Z] 08:38:14 INFO - In file included from /builds/worker/workspace/build/src/obj-firefox/dist/include/MediaEngine.h:9:0,
[task 2019-10-02T08:38:14.066Z] 08:38:14 INFO - from /builds/worker/workspace/build/src/obj-firefox/dist/include/mozilla/MediaManager.h:8,
[task 2019-10-02T08:38:14.068Z] 08:38:14 INFO - from StaticComponents.cpp:42:
[task 2019-10-02T08:38:14.068Z] 08:38:14 ERROR - /builds/worker/workspace/build/src/obj-firefox/dist/include/MediaTrackGraph.h:445:13: error: explicit specialization in non-namespace scope 'class mozilla::MediaTrack'
[task 2019-10-02T08:38:14.068Z] 08:38:14 INFO - template <>
[task 2019-10-02T08:38:14.068Z] 08:38:14 INFO - ^
[task 2019-10-02T08:38:14.068Z] 08:38:14 ERROR - /builds/worker/workspace/build/src/obj-firefox/dist/include/MediaTrackGraph.h:446:41: error: template-id 'GetData<mozilla::MediaSegment>' in declaration of primary template
[task 2019-10-02T08:38:14.068Z] 08:38:14 INFO - MediaSegment* GetData<MediaSegment>() const {
[task 2019-10-02T08:38:14.068Z] 08:38:14 INFO - ^~~~~
[task 2019-10-02T08:38:14.068Z] 08:38:14 INFO - /builds/worker/workspace/build/src/config/rules.mk:785: recipe for target 'StaticComponents.o' failed
[task 2019-10-02T08:38:14.068Z] 08:38:14 ERROR - make[4]: *** [StaticComponents.o] Error 1
[task 2019-10-02T08:38:14.069Z] 08:38:14 INFO - make[4]: Leaving directory '/builds/worker/workspace/build/src/obj-firefox/xpcom/components'
[task 2019-10-02T08:38:14.069Z] 08:38:14 INFO - /builds/worker/workspace/build/src/config/recurse.mk:74: recipe for target 'xpcom/components/target-objects' failed
[task 2019-10-02T08:38:14.069Z] 08:38:14 ERROR - make[3]: *** [xpcom/components/target-objects] Error 2
[task 2019-10-02T08:38:14.069Z] 08:38:14 INFO - make[3]: *** Waiting for unfinished jobs....
[task 2019-10-02T08:38:14.070Z] 08:38:14 INFO - make[4]: Entering directory '/builds/worker/workspace/build/src/obj-firefox/storage'
[task 2019-10-02T08:38:14.070Z] 08:38:14 INFO - storage/Unified_cpp_storage0.o

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b310c9ac5d35
Move TrackInfo::mTrackId away from TrackID as they're independent. r=karlt
https://hg.mozilla.org/integration/autoland/rev/a4d88d5b86d2
Break out PrincipalHandle into its own file. r=padenot
https://hg.mozilla.org/integration/autoland/rev/398d14785168
Remove track blocking from MediaInputPort. r=karlt
https://hg.mozilla.org/integration/autoland/rev/3f763ede55c3
Remove calls to SetStreamOrderDirty() after affecting MediaStreams' finished state. r=karlt
https://hg.mozilla.org/integration/autoland/rev/7cd5ea4167b6
Remove the notion of streams from MediaStreamGraph. r=padenot,karlt
https://hg.mozilla.org/integration/autoland/rev/6a9a41e0bf24
Remove AudioNodeStream::SampleRate(). r=karlt
https://hg.mozilla.org/integration/autoland/rev/9ea7aabf10fb
Rename streams to tracks. r=padenot,karlt,smaug

Comment 12

[Raul Gurzau (:RaulG)]

https://hg.mozilla.org/mozilla-central/rev/b310c9ac5d35
https://hg.mozilla.org/mozilla-central/rev/a4d88d5b86d2
https://hg.mozilla.org/mozilla-central/rev/398d14785168
https://hg.mozilla.org/mozilla-central/rev/3f763ede55c3
https://hg.mozilla.org/mozilla-central/rev/7cd5ea4167b6
https://hg.mozilla.org/mozilla-central/rev/6a9a41e0bf24
https://hg.mozilla.org/mozilla-central/rev/9ea7aabf10fb

Comment 13

[Andreas Pehrson [:pehrsons]]

What docs did you have in mind Chris? This is an internal optimization really, I don't expect anything js-observable to come from this.

Comment 14

[Chris Mills [:cmills]]

(In reply to Andreas Pehrson [:pehrsons] from comment #13)

What docs did you have in mind Chris? This is an internal optimization really, I don't expect anything js-observable to come from this.

It looked like there was a WebIDL change. Does that not need changing in the docs?

Comment 15

[Andreas Pehrson [:pehrsons]]

There was a chrome-only name change that is only for our automated tests.

Comment 16

[Chris Mills [:cmills]]

(In reply to Andreas Pehrson [:pehrsons] from comment #15)

There was a chrome-only name change that is only for our automated tests.

OK, no worries then. Thanks for clarifying!