Closed Bug 1425873 Opened 2 years ago Closed 2 years ago

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

Categories

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

59 Branch
Unspecified
All
defect

Tracking

()

RESOLVED FIXED
mozilla59
Tracking Status
firefox59 --- fixed

People

(Reporter: jib, Assigned: bwc)

References

Details

Attachments

(2 files)

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 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.
Attachment #8937484 - Flags: review?(drno) → review+
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().
Attachment #8937483 - Flags: review?(jib) → review+
Check back on try.
Flags: needinfo?(docfaraday)
Pushed by bcampen@mozilla.com:
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
Flags: needinfo?(docfaraday)
https://hg.mozilla.org/mozilla-central/rev/f427c2fcbfe0
https://hg.mozilla.org/mozilla-central/rev/3f6f3dde981a
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
You need to log in before you can comment on or make changes to this bug.