element-hq / element-meta

Shared/meta documentation and project artefacts for Element clients
66 stars 11 forks source link

Consistent handling of end-of-candidates `m.call.candidates` events #2323

Closed stefanceriu closed 4 months ago

stefanceriu commented 4 months ago

Steps to reproduce

  1. Where are you starting? What can you see?
    • starting from a DM
  2. What do you click?
    • the call button
  3. More steps…

Outcome

What did you expect?

Web does so but includes an invalid Candidate dictionary at the end of the list e.g.

{
  "type": "m.call.candidates",
  "content": {
    "candidates": [
      {
        "candidate": "[candidate:123 1 udp 123 123.123.123.123 123 typ relay raddr 123.123.123.123 rport 123 generation 0 ufrag D5L1 network-id 2]",
        "sdpMid": "0",
        "sdpMLineIndex": 0,
        "usernameFragment": "D5L1"
      },

      ...

      {
        "candidate": ""
      }

    ],
    "version": "1",
    "call_id": "123",
    "party_id": "123"
  },
  "room_id": "!123:matrix.org"
}

All Candidate dictionaries are supposed to have 3 required fields: candidate, sdpMLineIndex and sdpMid which doesn't happen in this case, so the RustSDK refuses to accept them.

Operating system

macOS

Browser information

Chrome

URL for webapp

staging.element.io

Application version

No response

Homeserver

No response

Will you send logs?

No

ara4n commented 4 months ago

@stefanceriu this looks to be a bug in the spec, not in EW. A blank candidate entry is how you signify the end of trickle ICE (i.e. that there are no more candidates to be discovered), and so is semantically important to signal the end of the ICE setup. Looks like the spec forgot that this was a thing.

ara4n commented 4 months ago

Actually the spec got it right in words: https://spec.matrix.org/v1.9/client-server-api/#end-of-candidates; it's "just" the formal definition which got it wrong

stefanceriu commented 4 months ago

Oh, I'm sorry, I totally missed that. I'm going to close this ticket and move the discussion to the Rust side here https://github.com/matrix-org/matrix-rust-sdk/issues/3187