Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

An incoming offer can generate simulcast #13902

Closed
wants to merge 2,027 commits into from
Closed

An incoming offer can generate simulcast #13902

wants to merge 2,027 commits into from

Conversation

alvestrand
Copy link
Contributor

This test needs discussion - my reading of draft-ietf-mmusic-simulcast is that an incoming offer with two RIDs should cause a transceiver to be created with two encodings avaialble, and an answer with two rid lines with send direction, indicating that the answerer is capable of sending 2 layers of simulcast.

This currently fails on both Chrome and Firefox.

@alvestrand
Copy link
Contributor Author

Actually JSEP says:

"  In addition, JSEP does not provide a mechanism to handle an incoming
   offer requesting simulcast from the JSEP endpoint.  This means that
   setting up simulcast in the case where the JSEP endpoint receives the
   initial offer requires out-of-band signaling or SDP inspection.
   However, in the case where the JSEP endpoint sets up simulcast in its
   in initial offer, any established simulcast streams will continue to
   work upon receipt of an incoming re-offer.  Future versions of this
   specification may add additional APIs to handle the incoming initial
   offer scenario.
"

The SDP in the test would be reasonable if it was supposed to work, but apparently it's not supposed to.

@wpt-pr-bot
Copy link
Collaborator

There are no reviewers for this pull request besides its author. Please reach out on W3C's irc server (irc.w3.org, port 6665) on channel #testing (web client) to get help with this. Thank you!

@nils-ohlmeier
Copy link
Contributor

nils-ohlmeier commented Nov 4, 2018 •
edited
Loading

In Firefox this should work, but you need to set the Encodings before setting the Offer.

Test code https://searchfox.org/mozilla-central/source/dom/media/tests/mochitest/test_peerConnection_simulcastAnswer.html#59

@fippo
Copy link
Contributor

fippo commented Nov 4, 2018

You might want to call getUserMedia, otherwise the m-line will be inactive...

http://jsfiddle.net/xcmq4vbn/1/ works in Firefox and has less boilerplate than the Firefox test. Calling setParameters works after setting the offer.

youennf
youennf reviewed Nov 4, 2018
View reviewed changes
webrtc/protocol/simulcast-negotiate.html Outdated
const answer = await pc.createAnswer();
const rid_lines = answer.sdp.split('\r\n')
.filter(line => line.match('^a=rid'));
assert_equals(rid_lines.length, 2, 'Answer shold have 2 RIDs');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/shold/should/

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we expecting the max-fr attributes to translate into maxFramerate values within the encoding parameters?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If they have any effect at all, that's the effect that they should have.

webrtc/protocol/simulcast-negotiate.html Outdated
<title>Negotiate Simulcast</title>
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script src="../RTCPeerConnection-helper.js"></script>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it used?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not yet.

webrtc/protocol/simulcast-negotiate.html Outdated
'use strict';



Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two unneeded empty lines.

youennf
youennf reviewed Nov 4, 2018
View reviewed changes
Copy link
Contributor

@youennf youennf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In Firefox this should work, but you need to set the Encodings before setting the Offer.

Test code https://searchfox.org/mozilla-central/source/dom/media/tests/mochitest/test_peerConnection_simulcastAnswer.html#59

In such a case, do you need to have simulcast enabled in the offer?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@youennf youennf youennf left review comments

@aboba aboba aboba left review comments

@gsnedders gsnedders Awaiting requested review from gsnedders

@jgraham jgraham Awaiting requested review from jgraham

@plinss plinss Awaiting requested review from plinss

Assignees

@alvestrand alvestrand

Labels
webrtc wg-webrtc
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.