814227 - Support multiple supported codec generation in answers

Status: RESOLVED DUPLICATE of bug 1333879

(Core :: WebRTC: Signaling, defect, P3)

Description

[Adam Roach [:abr]]

As mentioned in bug 786327, comment 40, we should ideally generate answers that contain a full intersection of supported codecs rather than stopping at the first match.

Comment 1

[Adam Roach [:abr]]

This work is going to be primarily in media/webrtc/signaling/src/sipcc/core/gsm/gsm_sdp.c. On initial analysis, I think we want to completely remove the "payload" field from fsmdef_media_t (see .../core/gsm/h/fsm.h) in favor of using the newer "payloads" array. This ripples into the fsmdef_previous_sdp_t, which needs to track an array of payloads instead of a single payload.

In gsm_sdp.c, this will mostly impact gsmsdp_update_local_sdp_media (which needs to be changed to generate SDP for each payload type in the payloads list) and gsmsdp_negotiate_codec (which needs to be updated to calculate a full union rather than stopping at the first match).

Comment 2

[Randell Jesup [:jesup] (needinfo me)]

We could work/release without it, but it would be embarrassing.

Comment 3

[Alex Chronopoulos [:achronop]]

Hi, can I work on this? Thanks!

Comment 4

[Adam Roach [:abr]]

(In reply to Alexandros Chronopoulos [:achronop] from comment #3)
> Hi, can I work on this? Thanks!

My recollection is that the only thing needed to "fix" this bug as described is to remove a single return statement.

The reason the bug is still open is that the team needs a broader discussion around (a) whether this is the right course of action (I think it is), and (b) whether the media stack will gracefully deal with an arbitrary mix of different codec types arriving withing a single stream.

In other words, solving this bug is more a matter of consensus building (task a) and deep, dark, serious media handling research (task b), both of which are going to require a lot of time and specialized expertise.

Comment 5

[Timothy B. Terriberry (:derf)]

(In reply to Adam Roach [:abr] from comment #4)
> (b) whether the media stack will gracefully deal with an arbitrary mix of
> different codec types arriving withing a single stream.

GIPS has an awful lot of code to deal with (b) correctly. Your guess is as good as mine if any of it works, because of course we're not currently testing it at all.

Comment 6

[Alex Chronopoulos [:achronop]]

Bugs for internal discussion, I never thought about that. I will keep watching it since looks like an interesting debate. In parallel I will try something with some coding. Thanks anyway.

Comment 8

[Bulk Bug Changes for mreavy's org]

Mass change P2->P3 to align with new Mozilla triage process.

Comment 9

[Philipp Hancke [:fippo]]

I just ran into an interesting side effect of this. Steps to reproduce:
1) On https://webrtc.github.io/samples/src/content/peerconnection/pc1/ make a call
2) observe offer codec PTs: 109 9 0 8 101
3) observe answer codec PTs: 109 101 (opus and telephone-event)
4) create new offer and observe the new list of codecs: 109 101 0 8 9

Sadly this makes the new offer a bit hard to compare for POF/PAN purposes.
It is also odd that the additional codecs are now in numerical order.

Comment 10

[Nils Ohlmeier [:drno]]

Byron could you please take a look at this when you have some time?

Comment 12

[Byron Campen [:bwc]]

Pretty sure we fixed this already in bug 1333879. We are putting all codecs that were successfully negotiated in our answer.