1969603 - Worker passed to RTCRtpScriptTransform never receives video frames from the ReadableStream

Status: RESOLVED FIXED

(Core :: WebRTC: Audio/Video, defect)

Description

[Clément Brisset]

Steps to reproduce:

I am creating webrtc call using insertable streams:

const pc = (this.pc = new RTCPeerConnection({
  bundlePolicy: 'max-bundle',
  sdpSemantics: 'unified-plan',
  encodedInsertableStreams: true,
}));

and then setting a RTCRtpScriptTransform on each of the transceiver like so:

rtcParticipant.transform = new RTCRtpScriptTransform(this.worker, {});

The worker listens to onrtctransform and installs a TransformStream like so:

self.onrtctransform = (event: RTCTransformEvent) => {
  const readable = event.transformer.readable;
  const writable = event.transformer.writable;
  const transformStream = new TransformStream({transform: transformFrame});
  readable.pipeThrough(transformStream).pipeTo(writable);
};

Actual results:

With this setup, when the call start I can see that onrtctransform gets called but transformFrame never receives any video frames

Expected results:

The ReadableStream emits RTCEncodedVideoFramesthat are then passed totransformFrame`

Comment 1

[Clément Brisset]

I managed to track down the problem to a recursive deadlock inside FrameTransformerProxy. I'm about to submit a patch for review

Comment 2

[Clément Brisset]

Attachment: Bug 1969603: Fix recursive deadlock in RTCRtpScriptTransform. r=pehrsons,jib,asuth,emilio,saschanaz

If there are any frames in mQueue TransformFrame will be called while
holding mMutex. TransformFrame in turn may call IsVideo back on the
proxy which will try to lock mMutex again

Comment 3

[BugBot [:suhaib / :marco/ :calixte]]

The Bugbug bot thinks this bug should belong to the 'Core::Audio/Video: Playback' component, and is moving the bug to that component. Please correct in case you think the bot is wrong.

Comment 4

[BugBot [:suhaib / :marco/ :calixte]]

The severity field is not set for this bug.
:jimm, could you have a look please?

For more information, please visit BugBot documentation.

Comment 5

[Jim Mathies [:jimm]]

Hey Byron, can we land this at this point? Jib mentioned in the standup it might be good if we had a test.

Comment 6

[Byron Campen [:bwc]]

I think it is ready to land.

Comment 7

[BugBot [:suhaib / :marco/ :calixte]]

There is an r+ patch which didn't land and no activity in this bug for 2 weeks.
:clement.brisset, could you have a look please?
If you still have some work to do, you can add an action "Plan Changes" in Phabricator.
For more information, please visit BugBot documentation.

Comment 8

[Clément Brisset]

Hey Jim, my apologies I did not see the flag for info in my inbox.
What additional information would you like me to provide?

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/b8299d26ace6
https://hg.mozilla.org/integration/autoland/rev/b7e8fda9d492
Fix recursive deadlock in RTCRtpScriptTransform. r=bwc

Comment 10

[amarc]

https://hg.mozilla.org/mozilla-central/rev/b7e8fda9d492