1425873 - addTransceiver(<string>, {streams: [stream]) should fire ontrack with stream in streams argument.

Status: RESOLVED FIXED

(Core :: WebRTC: Signaling, defect, P2)

Description

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

This is required by the spec, and causes problems with e.g. simple replaceTrack on a newly added transceiver with addTransceiver("video");

STRs:
1. Open https://jsfiddle.net/jib1/1668hwsz/

Expected result: ontrack streams = 1
Actual result:   ontrack streams = 0

Basically, the event fires with an empty streams array in this case, which violates the spec.

Workaround:

Checking ☑ video works bc fiddle sidesteps streams argument: video2.srcObject = new MediaStream([track]);

Comment 1

[Byron Campen [:bwc]]

Attachment: Bug 1425873 - Part 1: Add test-case for trackless stream. r+jib

Review commit: https://reviewboard.mozilla.org/r/208158/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/208158/

Comment 2

[Byron Campen [:bwc]]

Attachment: Bug 1425873 - Part 2: Sync send stream ids even when there is no send track. r+drno

Review commit: https://reviewboard.mozilla.org/r/208160/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/208160/

Comment 3

[Nils Ohlmeier [:drno]]

Comment on attachment 8937484 [details]
Bug 1425873 - Part 2: Sync send stream ids even when there is no send track. r+drno

https://reviewboard.mozilla.org/r/208160/#review213948

LGTM with the one comment addressed.

::: media/webrtc/signaling/src/peerconnection/TransceiverImpl.cpp:432
(Diff revision 1)
> +    // TODO: Remove
> +    MOZ_CRASH();

I'm assuming this TODO was targeted at removing this before landing?
If not, then it might help to explain a little bit more when to remove.

Comment 4

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

Comment on attachment 8937483 [details]
Bug 1425873 - Part 1: Add test-case for trackless stream. r+jib

https://reviewboard.mozilla.org/r/208158/#review213954

::: dom/media/tests/mochitest/test_peerConnection_transceivers.html:803
(Diff revision 1)
>  
> +  let checkSendrecvWithTracklessStream = async () => {
> +    let pc1 = new RTCPeerConnection();
> +    let pc2 = new RTCPeerConnection();
> +
> +    let stream = await getUserMedia({audio: true});

No need to call gUM here. Just use:

    let stream = new MediaStream();

::: dom/media/tests/mochitest/test_peerConnection_transceivers.html:819
(Diff revision 1)
> +        }
> +      ]);
> +
> +    pc1.close();
> +    pc2.close();
> +    stopTracks(stream);

stopTracks() is not needed if we use new MediaStream().

Comment 5

[Byron Campen [:bwc]]

Comment on attachment 8937483 [details]
Bug 1425873 - Part 1: Add test-case for trackless stream. r+jib

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/208158/diff/1-2/

Comment 6

[Byron Campen [:bwc]]

Comment on attachment 8937484 [details]
Bug 1425873 - Part 2: Sync send stream ids even when there is no send track. r+drno

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/208160/diff/1-2/

Comment 7

[Byron Campen [:bwc]]

Check back on try.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f427c2fcbfe0
Part 1: Add test-case for trackless stream. r+jib r=jib
https://hg.mozilla.org/integration/autoland/rev/3f6f3dde981a
Part 2: Sync send stream ids even when there is no send track. r+drno r=drno

Comment 9

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/f427c2fcbfe0
https://hg.mozilla.org/mozilla-central/rev/3f6f3dde981a