w3c / webrtc-pc

WebRTC 1.0 API
https://w3c.github.io/webrtc-pc/
Other
439 stars 115 forks source link

Intended outcome when modifying direction in have-local-offer #2751

Open k0nserv opened 2 years ago

k0nserv commented 2 years ago

Hello,

I'm trying to understand something about local modifications during have-local-offer.

Scenario: A transceiver is created with a sending track, its direction defaults to sendrecv. There has been at least one negotiation and currentDirection is sendonly

Events:

  1. A negotiation is started for some other reason
  2. An offer is created with createOffer
  3. The offer is applied as the local description
  4. The existing track is removed with removeTrack this causes direction to change to recvonly.
  5. An answer is received from the remote peer and applied as the remote description.

At this point what should the direction of the transceiver in question be: recvonly, sendonly, or sendrecv?

By my reading of the specification it should be sendrecv if the remote peer answers with sendrecv or sendonly if the remote peer answered with recvonly. The reason for this is that when applying the remote answer the specification says to update both transceiver.[[CurrentDirection]] and transceiver.[[Direction]] to the reversed value from the peer per step 4.5.9.2.13.2 of "set a session description" instructions.

It should be noted that libWebrtc doesn't do this, it leaves transceiver.[[Direction]] untouched and only updates transceiver.[[CurrentDirection]]

fippo commented 2 years ago

I think https://jsfiddle.net/fippo/s1fguya7/3/ reproduces this. The current direction should be sendOnly and negotiationneeded should fire, suggesting you need to do another negotiation.

k0nserv commented 2 years ago

That matches what I see. I've got two js fiddles to help with experimenting:

The behaviour can be reproduced by pasting the following into the sender's console after starting the session(by exchanging the initial offers/answers):

const offer = await pc.createOffer();
await pc.setLocalDescription(offer);

let sender = Object.values(mediaStream.senders)[0];
pc.removeTrack(sender);

const trans = pc.getTransceivers()[0];
console.dir({ trans, direction: trans.direction, currentDirection: trans.currentDirection });

dc.send(JSON.stringify({ type: 'offer', sdp: offer.sdp }))

This does trigger a negotiationneeded event and in the subsequent offer the transceiver is recvonly. However, this is observed behaviour in Chrome, not what I would expect from the specification.

Based on the specification I would expect:

  1. The direction in the offer is sendrecv or sendonly, matching the value of transceiver.[[Direction]]
  2. removeTrack causes the transceiver's direction to become recvonly or inactive
  3. When applying the remote answer from the peer, the transceiver's direction should change back to sendonly or sendrecv from recvonly or inactive(per 4.5.9.2.13.2)
  4. When checking if negotiation is needed we would find that the transceiver's direction matches that of the remote description and no negotiation is thus needed.

I can see how negotiationneeded would fire if the direction of the transceiver doesn't match the remote description. However, if implementations are supposed to accept the direction in an answer by updating the transceiver's direction I don't see how it could become sendonly, nor why negotiationneeded should fire.

jan-ivar commented 2 years ago

This was added in https://github.com/w3c/webrtc-pc/pull/2033, and I think we made a mistake. As you show, it creates a race between API use and (perfect) renegotiation. No-one's implemented this, so it might be problematic to implement at this point. I propose we revert it.

I think @stefhak was right that:

E.g. it's normal for them to differ:

In this view of them as independent attributes, updating them should be deterministic to the app.

It's the nature of negotiation that changes on one side don't always produce a net change in result.

aboba commented 2 years ago

Feedback from June 19, 2022 WEBRTC WG Virtual Interim: "Ready for PR"

mickel8 commented 11 months ago

Sorry for bumping the topic once again but shouldn't the currentDirection be determined as an intersection of direction (i.e. our preference) and the answerer's direction (remote capability)?

Right now w3c says:

2. Set transceiver.[[[CurrentDirection]]](https://www.w3.org/TR/webrtc/#dfn-currentdirection) to direction.

and JSEP 4.2.5 says the same

4.2.5.  currentDirection

   The currentDirection property indicates the last negotiated direction
   for the transceiver's associated "m=" section.  More specifically, it
   indicates the direction attribute [RFC3264] of the associated "m="
   section in the last applied answer (including provisional answers),
   with "send" and "recv" directions reversed if it was a remote answer.
   For example, if the direction attribute for the associated "m="
   section in a remote answer is "recvonly", currentDirection is set to
   "sendonly".

   If an answer that references this transceiver has not yet been
   applied or if the transceiver is stopped, currentDirection is set to
   "null".

However, consider the scenario where we offer sendonly and receive sendonly. This is probably a protocol violation (according to section 5.10 of JSEP) but let's assume that someone did SDP munging incorrectly or there is a bug in a remote implementation

pc = new RTCPeerConnection()
pc.addTransceiver('audio', {direction: 'sendonly'})
offer = await pc.createOffer()
await pc.setLocalDescription(offer)

pc2 = new RTCPeerConnection()
await pc2.setRemoteDescription(offer)
answer = await pc2.createAnswer()
sdp = answer.sdp.replace('recvonly', 'sendonly');
await pc.setRemoteDescription(answer2)
pc.getTransceivers()

This wil result in

currentDirection: "recvonly"
direction: "sendonly"

while the more intuitive outcome, I belive, would be

currentDirection: "inactive"
direction: "sendonly"

or an error.