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.

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

Plan X: Add an API for using RID to do simulcast #353

Merged
merged 5 commits into from
Nov 25, 2015

Conversation

pthatcherg
Copy link
Contributor

No description provided.

<dt>DOMString rid</dt>
<dd>
<p> If set, this RTP encoding will be sent with the RID header extension as
defined by [[!RTCWEB-JSEP]].
Copy link
Member

Choose a reason for hiding this comment

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

Why does this need to be set, can we not simply require that browsers use RID? Having the browser choose the RID will work much better than allowing the application pick something.

Copy link
Member

Choose a reason for hiding this comment

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

Here I'm concerned about applications choosing for collisions, or choosing identifiers that are too long for good performance.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Several people (including Adam and Byron, if I recall correctly) requested a way that applications could give some meaning to each layer beyond just a number. This is one reason why a string was chosen for the RID instead of an int. For example, this allows the application to choose "F" for full, "H" for half and "Q" for quarter, or "S", "M", "L", or whatever.

It's possible that a similar thing could be accomplished by putting the encodings in the right order in the SDP, but that hasn't been fully fleshed out. It's also possible that the RID can be chosen by the browser and then additional semantics can be signaled to the MCU outside of SDP, which was my original proposal.

I'm not worried about collisions. If the application chooses the same RID for two or more encodings, we just throw an exception (I could add some text to make that explicit). And we could also add some text about keeping the ID short but I'd hope that someone doing something as advanced as simulcast would know that they shouldn't choose a long value for something that's going to go into a header extension.

@stefhak
Copy link
Contributor

stefhak commented Nov 4, 2015

This approach to simulcast was discussed at TPAC and generally liked. The main concerns seemed to be:

  • need more knobs than scaleDownResolution (e.g. framrate)
  • still not 100% clear how things interact with Track constraints

@pthatcherg
Copy link
Contributor Author

I think we also agreed we could land this PR and think about add more
controls (framerate) later.

I'm not sure about your second bullet point, though. Is it just a subset
of the first?

On Tue, Nov 3, 2015 at 11:46 PM, stefan hakansson notifications@github.com
wrote:

This approach to simulcast was discussed at TPAC and generally liked. The
main concerns seemed to be:

  • need more knobs than scaleDownResolution (e.g. framrate)
  • still not 100% clear how things interact with Track constraints


Reply to this email directly or view it on GitHub
#353 (comment).

@stefhak
Copy link
Contributor

stefhak commented Nov 4, 2015

@pthatcherg I'll consult with the other chairs regarding merging, I don't have a clear memory. But (to reiterate) it was clear to me that these is support for moving forward with this approach.
Regarding the second bullet, yes it is really a subset of the first. One example was framerate (which seemed to be something many wanted a knob for), there is a track constraint for constraining framerate, should a simulcast knob for framerate be independent, or should it be a multiplier of the track framerate? I think we had pretty aligned views in the room, it is more a mental note that there may be some more things here to sort out.

@pthatcherg
Copy link
Contributor Author

On Wed, Nov 4, 2015 at 9:12 PM, stefan hakansson notifications@github.com
wrote:

@pthatcherg https://github.com/pthatcherg I'll consult with the other
chairs regarding merging, I don't have a clear memory. But (to reiterate)
it was clear to me that these is support for moving forward with this
approach.
Regarding the second bullet, yes it is really a subset of the first. One
example was framerate (which seemed to be something many wanted a knob
for), there is a track constraint for constraining framerate, should a
simulcast knob for framerate be independent, or should it be a multiplier
of the track framerate? I think we had pretty aligned views in the room, it
is more a mental note that there may be some more things here to sort out.

​Ah, yes. Whether we do framerateScale or maxFramerate did not end up
with consensus. I see what you mean now.​


Reply to this email directly or view it on GitHub
#353 (comment).

@mhofman
Copy link

mhofman commented Nov 5, 2015

I thought we converged to agreeing maxFramerate was enough, and that a framerateScale might be weird for source with variable framerate, e.g. screensharing.

As an application developer, max-framerate is what I want.

@elagerway
Copy link
Contributor

@pthatcherg there is a branch conflict here, could you look at it?

@alvestrand
Copy link
Contributor

I think maxFramerate is the one that's possible to get consensus on.
BTW, the branch needs conflict resolution.

@pthatcherg
Copy link
Contributor Author

The merge conflict is now resolved.

By the way, I also put in the "16 alphanumeric characters" limit on the RID
value.

On Thu, Nov 19, 2015 at 7:42 AM, Harald Alvestrand <notifications@github.com

wrote:

Assigned #353 #353 to @pthatcherg
https://github.com/pthatcherg.


Reply to this email directly or view it on GitHub
#353 (comment).

@aboba
Copy link
Contributor

aboba commented Nov 24, 2015

If we add maxFramerate to RTCRtpEncodingParameters (which seems ok to me), do we also need max-fr within fmtp parameters (e.g. for VP8). Given that max-fr is not an H.264 SDP parameter, I would prefer a maxFramerate attribute within RTCRtpEncodingParameters. I have similar concerns about trying to control resolution via max-fs in SDP (listed as an SDP parameter in VP8 as well as H.264) as well as via resolutionScale, max-height, max-width, and imageattr (RFC 6236). Keep this simple, please.

@alvestrand
Copy link
Contributor

I wouldn't want to mess with fmtp if I can avoid it. The max-fs in VP8 in particular (the one I know something about) limits what you can receive, but says nothing about what you can send. Entirely different from the proposed rid parameters for rid negotiation.
But this spec is all about API, not SDP.

alvestrand added a commit that referenced this pull request Nov 25, 2015
Plan X: Add an API for using RID to do simulcast
@alvestrand alvestrand merged commit 0d5262c into w3c:master Nov 25, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants