webrtc-pc icon indicating copy to clipboard operation
webrtc-pc copied to clipboard

Intended outcome when modifying direction in have-local-offer

Open k0nserv opened this issue 1 year ago • 5 comments

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]]

k0nserv avatar Jul 11 '22 15:07 k0nserv

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.

fippo avatar Jul 11 '22 16:07 fippo

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.

k0nserv avatar Jul 11 '22 16:07 k0nserv

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:

  • direction reflects this side's preference in offers and answers
  • currentDirection reflects the net negotiated direction

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

  • A direction of "sendrecv" means we have stuff to send and are open to receive
  • A currentDirection of "sendonly" means the other side has nothing to send at the moment

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.

jan-ivar avatar Jul 15 '22 20:07 jan-ivar

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

aboba avatar Jul 19 '22 22:07 aboba

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.

mickel8 avatar Dec 12 '23 09:12 mickel8