Closed Bug 1305949 Opened 3 years ago Closed 3 years ago

Do some cleaning around direct listeners and video sinks

Categories

(Core :: Audio/Video: MediaStreamGraph, defect, P2)

defect

Tracking

()

RESOLVED FIXED
mozilla53
Tracking Status
firefox53 --- fixed

People

(Reporter: pehrsons, Assigned: pehrsons)

References

Details

Attachments

(3 files, 1 obsolete file)

Bug 1256079 unveiled an intermittent where one of the symptoms (after some digging) was that the installation of a direct track listener failed and nobody was checking it -- we just assumed the installation would succeed.

In this bug I intend to land a default implementation of DirectMediaStreamTrackListener::NotifyDirectListenerInstalled that asserts that the install was successful. Listeners where it's ok to fail installation (i.e., where it's handled) should override this method.

Hopefully this can help catch other intermittents.
Rank: 25
Depends on: 1309886
Comment on attachment 8795718 [details]
Bug 1305949 - Assert by default when direct track listener installation fails.

https://reviewboard.mozilla.org/r/81688/#review84902
Attachment #8795718 - Flags: review?(rjesup) → review+
Comment on attachment 8795719 [details]
Bug 1305949 - Refactor code that feeds video stream sink when it gets added.

https://reviewboard.mozilla.org/r/81690/#review85374

LGTM
Attachment #8795719 - Flags: review?(ctai) → review+
Comment on attachment 8800574 [details]
Bug 1305949 - Use the same path for passing on missed data to video sink, as during normal operation.

https://reviewboard.mozilla.org/r/85486/#review85376
Attachment #8800574 - Flags: review?(ctai) → review+
Comment on attachment 8800575 [details]
Bug 1305949 - Use only one listener in MediaPipelineTransmit.

https://reviewboard.mozilla.org/r/85488/#review85382

LGTM
Attachment #8800575 - Flags: review?(ctai) → review+
We want to get rid of direct listeners for audio, so I don't want to spend time on greening up the assert, but it would be nice to land the other patches.
Summary: Assert that direct track listener installs succeed → Do some cleaning around direct listeners and video sinks
Pushed by pehrsons@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/49b90756985c
Refactor code that feeds video stream sink when it gets added. r=ctai
https://hg.mozilla.org/integration/mozilla-inbound/rev/ddfa37f89612
Use the same path for passing on missed data to video sink, as during normal operation. r=ctai
https://hg.mozilla.org/integration/mozilla-inbound/rev/7b4edaa06578
Use only one listener in MediaPipelineTransmit. r=ctai
Attachment #8795718 - Attachment is obsolete: true
You need to log in before you can comment on or make changes to this bug.