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

When should it be possible to add previously discarded codecs in a subsequent offer? #266

Closed
taylor-b opened this issue Mar 28, 2016 · 56 comments

Comments

@taylor-b
Copy link
Collaborator

Section 5.2.2, "Subsequent Offers", says:

o The m= line and corresponding "a=rtpmap" and "a=fmtp" lines MUST
only include codecs present in the remote description.

However, consider a situation where Alice offers codecs foo and bar, and Bob answers with only codec foo, thereby rejecting bar. If Bob were to now create a subsequent offer, his remote description still contains both foo and bar, so he would technically be allowed to offer both codecs, even though bar was previously rejected.

Is this intentional? It seems to go against the spirit of the rule. Perhaps the rule is meant to be "only include codecs present in the current local and remote descriptions".

For context, see: https://www.ietf.org/mail-archive/web/rtcweb/current/msg15697.html

@juberti
Copy link
Contributor

juberti commented Mar 29, 2016

Yes, your understanding is correct. The set to be offered should only include the subset of codecs that were negotiated.

@ibc
Copy link

ibc commented Mar 29, 2016

The set to be offered should only include the subset of codecs that were negotiated.

Please, let's specify the context of those already negotiated codecs. Is that per m=line? is that per "RTP session" (whatever that means nowadays)? is that per full SDP?

If Alice just offers an m=audio line in the initial SDP offer and Bob answers, aren't they allowed to re-offer later with an added m=video line?

@ibc
Copy link

ibc commented Mar 29, 2016

Also, are all those rules take into account multi-stream/participants when in a SFU scenario?

Alice may join a SFU conference by offering VP8 and H264 for her sending m=video section, and the SFU may reply with just VP8. But later when a new non-VP8 capable peer join the conference, the SFU may want to generate a re-offer for Alice in behalf of Bob with a new m=video line (a=sendonly) having just H264 codec (since Bob). Why is this illegal?

@ibc
Copy link

ibc commented Mar 29, 2016

Related issue open in Chromium project: https://bugs.chromium.org/p/webrtc/issues/detail?id=5697

@taylor-b
Copy link
Collaborator Author

Is that per m=line? is that per "RTP session" (whatever that means nowadays)? is that per full SDP?

I assumed it meant per m= line, though this should definitely be made clear. If my assumption is correct, there would be no issue in your later comment about an SFU adding a new m= line.

@ibc
Copy link

ibc commented Mar 29, 2016

Thanks @taylor-b, it makes sense. Hope it's clarified in the spec.

@juberti
Copy link
Contributor

juberti commented Apr 15, 2016

Eric and I discussed today and I think the simplest change is to reference "the codecs present in the most recent answer", since that is the set of negotiated codecs.

This should also be extended to the other bullets in 5.2.2 regarding negotiated properties.

@ibc
Copy link

ibc commented Apr 16, 2016

Eric and I discussed today and I think the simplest change is to reference "the codecs present in the most recent answer", since that is the set of negotiated codecs.

@juberti @ekr have you consider this scenario?:

  • Alice may join a SFU conference by offering VP8 and H264 in her m=video (a=sendrecv), and the SFU may reply with just VP8 to ensure VP8 is used.
  • Later, when a new non-VP8 capable peer joins the conference, the SFU may want to generate a re-offer for Alice by changing the codec to just H264.

With your change this is not legal. And I don't agree because it is an artificial limitation.

If we play the SDP game we should follow its rules. Does some RFC state that my suggested use case is illegal?

@juberti
Copy link
Contributor

juberti commented Apr 18, 2016

SFUs can do whatever they want. This spec is for JSEP implementations.

@ibc
Copy link

ibc commented Apr 18, 2016

@juberti please, re-read my concern. It's about a valid SDP received by WebRTC browser.

@juberti
Copy link
Contributor

juberti commented Apr 18, 2016

Nothing prohibits the SFU from re-offering with H.264 + VP8, and the JSEP endpoint will handle this properly when it is received (generating an appropriate answer).

The text being discussed here only affects the offers that JSEP generates.

@fluffy
Copy link
Contributor

fluffy commented Aug 3, 2016

Re-openened due to discution on ietf list

@fluffy fluffy reopened this Aug 3, 2016
@rshpount
Copy link

rshpount commented Aug 3, 2016

I think re-offer should contain the full set of codecs or this will cause interesting problems with 3pcc or SIP interop. The order of codecs should ideally be the same as currently negotiated to avoid codec flips. Essentially, what this means all supported codecs not currently negotiated should be added to the end of the offer after the currently negotiated ones.

@cdh4u
Copy link
Contributor

cdh4u commented Aug 14, 2016

Note that there is a generic problem about what we mean by "codec". For example, assuming we are only allowed to re-offer previously negotiated codecs (e.g. H.264), are we still allowed to re-offer a different H.264 "codec configuration"?

@ibc
Copy link

ibc commented Sep 8, 2016

Related: can an endpoint re-offer by introducing new PT values that were not present in previous O/A?

https://mailarchive.ietf.org/arch/msg/rtcweb/CF-bGveuUOlMWTyuVlWv2Ul1UO0

@fluffy
Copy link
Contributor

fluffy commented Sep 9, 2016

So one way to create a re-offer with all the new PT value that got removed in prev O/A is to create a new PC, add all the steams/tracks to it. Then call create offer to create a "fresh" offer and use that for the offer. However, might be a bit complicated with what happens with incoming media, some of it would be incoming on old PC then move to new PC.

@rshpount
Copy link

rshpount commented Sep 9, 2016

This is even more complicated since you are not supposed to re-use the old PT in the new offer for different codecs. Since this is a completely new offer with no information about previously used PT, this restriction is hard to enforce.

@ibc
Copy link

ibc commented Sep 9, 2016

So one way to create a re-offer with all the new PT value that got removed in prev O/A is to create a new PC, add all the steams/tracks to it

Honestly, I don't think such an approach is acceptable.

@fluffy
Copy link
Contributor

fluffy commented Sep 9, 2016

Hmm - I see your point.

@ibc
Copy link

ibc commented Sep 9, 2016

I don't want to renegotiate ICE+DTLS. I just want to add my audio/video on an already connected transport, and make it work :)

@ghost ghost assigned juberti Feb 11, 2017
@ghost ghost added the in progress label Feb 11, 2017
@juberti juberti removed the discuss label Feb 11, 2017
@ibc
Copy link

ibc commented Feb 12, 2017

Hi, sorry as I couldn't track all the conclusions in this issue. However I'd like to confirm whether the current Chrome behavior is correct (according to the conclusion above) or not:

  • Chrome is offered with VP8 PT 123.
  • Chrome answers with VP8 PT 123 (including a sending video track) and no other video codecs.
  • Chrome removes the video track from its PeerConnection.
  • Chrome re-adds the same video track into its PeerConnection.
  • Chrome calls createOffer().

The new offer contains VP8 PT 123, but also VP9 PT 101, H264 etc etc etc. I didn't expect it since the initial SDP O/A did not contain them. Of course, there is a single BUNDLED transport.

If this behavior correct and should Chrome fix it?

@fluffy
Copy link
Contributor

fluffy commented Feb 12, 2017

I think that behavior is correct by the new definition which is roughly reoffers will offer everything the browser can do at that point of time, but will put the stuff currently in use first.

@ibc
Copy link

ibc commented Feb 12, 2017

Fine with me :)

@juberti
Copy link
Contributor

juberti commented Feb 13, 2017

Note that removal/readdition from a MediaStream should have no impact on offers/answers. IOW, the case you describe above is essentially: offer with all codecs, answer with just VP8, reoffer contains all codecs.

@juberti
Copy link
Contributor

juberti commented Feb 13, 2017

See also #602 as a trivial followup on this issue.

@juberti
Copy link
Contributor

juberti commented Feb 13, 2017

Which has been merged, bringing this issue, finally, to a close.

@juberti juberti closed this as completed Feb 13, 2017
@ghost ghost removed the in progress label Feb 13, 2017
@ibc
Copy link

ibc commented Feb 13, 2017

Note that removal/readdition from a MediaStream should have no impact on offers/answers

What about if the video track I add to the local stream is not the same as the one I previously removed from it? For example, it is a video track retrieved via getUserMedia() with different video resolution/constraints settings.

@juberti
Copy link
Contributor

juberti commented Feb 13, 2017

It has no impact. Only tracks given to addTrack or setTrack are used.

@ibc
Copy link

ibc commented Feb 13, 2017

It has no impact. Only tracks given to addTrack or setTrack are used.

I did not understand you, sorry. I know that, according to the new API, changes in the local MediaStream do not affect what the PeerConnection sends (although current Chrome does not behave like that and reacts on track addition/removal into the MediaStream).

Thanks.

@ibc
Copy link

ibc commented Feb 13, 2017

Just to be 200% sure, I would like to confirm that the following behaviour in Firefox is wrong. The scenario is the same as above but now with Firefox:

  • Firefox is offered with VP8 PT 123.
  • Firefox answers with VP8 PT 123 (including a sending video track) and no other video codecs.
  • Firefox removes the video track from its PeerConnection.
  • Firefox adds another local video track into its PeerConnection.
  • Firefox calls createOffer().

The new offer contains VP8 PT 120 instead of 123 (wrong).

Bug/issue detailed here: https://bugzilla.mozilla.org/show_bug.cgi?id=1339234

@ekr
Copy link
Contributor

ekr commented Feb 14, 2017

What exact text do you believe prohibits this?

@rshpount
Copy link

Per RFC 3264 this is allowed. What is not allowed is to use PT 123 for anything except VP8.

Per RFC3264 8.3.2 Changing the Set of Media Formats:

However, in the case of RTP, the mapping from a particular dynamic payload type number to a particular codec within that media stream MUST NOT change for the duration of a session. For example, if A generates an offer with G.711 assigned to dynamic payload type number 46, payload type number 46 MUST refer to G.711 from that point forward in any offers or answers for that media stream within the session. However, it is acceptable for multiple payload type numbers to be mapped to the same codec, so that an updated offer could also use payload type number 72 for G.711.

@ibc
Copy link

ibc commented Feb 14, 2017

Well, probably nothing prohibits a re-offer to include new PT values pointing to same codec+configurations as previously negotiated. Said that, a re-offer cannot offer a previously used PT for other codec+configuration.

So IMHO this could be simplified by mandating reusing same PT values as before.

@ekr
Copy link
Contributor

ekr commented Feb 14, 2017

I'm not seeing a lot of value in requiring JSEP impls to behave this way.

@fluffy
Copy link
Contributor

fluffy commented Feb 14, 2017

Just noting that if it was a different m line, then it could use any PT it wanted for VP8. And in the above example, it is fine for it to use 120 in the 2nd offer as long as 120 had not previously been used for something other than VP8.

To answer EKR question, the only thing of value here is following use case. In offer/answer 1, we have PT 120 => H264 and PT 123 => VP8. If in offer/answer 2 (on same m-lines, same mid, same blah blah blah) we swapped to PT120 => VP8, if we got a late media packet that was sent before the switch, but arrived after the switch, it might get delivered to the VP8 codec when actually it was encoded with H.264. I think that is all the is meant to stop. And I have allwasy viewed that trying to sop this is silly because what is going to happen is that late packet is going to get discarded. If sending a 264 packet to your VP8 codecs causes anything to happen other than it getting discarded, well seem like that is a problem.

@juberti
Copy link
Contributor

juberti commented Feb 14, 2017

The behavior mentioned by @ibc does seem complicated, since the offerer needs to decode VP8 on both PT 120 and 123 at least until the O/A completes, but I agree it appears to be legal, and AFAICT it does not create any interoperability problems.

@ibc
Copy link

ibc commented Feb 14, 2017

Just noting that if it was a different m line, then it could use any PT it wanted for VP8

Uh no no. That's not possible. As long as such a m line belongs to the same BUNDLE group, all the m lines in that group are part of the same RTP session®️ [1] and, hence, their PT values must be unique (in the sense that they must point to the same codec+configuration®️).

I already asked about this exact subject several months ago. The given rationale was:

"Existing RTP intermediaries®️ [2] may rely on a single 1:1 mapping between PT and codec+configuration when carried over the same IP:port tuple, this is, the same RTP session"

Those are the related threads:

[1] Future generations may experience difficulties in understanding the concept of "RTP session".
[2] Although no one has ever seen them in a WebRTC environment, they may exist.

@ibc
Copy link

ibc commented Feb 14, 2017

The behavior mentioned by @ibc does seem complicated, since the offerer needs to decode VP8 on both PT 120 and 123 at least until the O/A completes, but I agree it appears to be legal, and AFAICT it does not create any interoperability problems.

The sad thing of all of this is that, at the end, for things to work the offerer and re-offerer must always be the same peer.

@fluffy
Copy link
Contributor

fluffy commented Feb 14, 2017

I think we definitely need to be able to have the reoffer move the media to a different IP since we do that all the time on non webrtc stuff. I might be missing what the issue is here but I want that to work.

@ibc
Copy link

ibc commented Feb 14, 2017

Move the m= section into a separate transport just because it includes PT values already present in the other m= sections but pointing to different codec+configurations?

@ekr
Copy link
Contributor

ekr commented Feb 14, 2017

I don't see how any of this is a an issue. The rule is you can't reuse the PT. There's no rule that you can't reuse the codec with a new PT.

@ibc
Copy link

ibc commented Feb 15, 2017

There's no rule that you can't reuse the codec with a new PT.

That's right, but it's also true that it causes errors in implementations (such as when Firefox renegotiates with Chrome by offering two different PT for Opus and Chrome assumes receipt of just the first PT).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

9 participants