diff --git a/pkg/rtc/transport.go b/pkg/rtc/transport.go index be22bbc4f..d94ef169b 100644 --- a/pkg/rtc/transport.go +++ b/pkg/rtc/transport.go @@ -1007,12 +1007,12 @@ func (t *PCTransport) queueOrConfigureSender( enableAudioNACK bool, ) { params := configureSenderParams{ - transceiver, - enabledCodecs, - rtcpFeedbackConfig, - !t.params.IsOfferer, - enableAudioStereo, - enableAudioNACK, + transceiver: transceiver, + enabledCodecs: enabledCodecs, + rtcpFeedbackConfig: rtcpFeedbackConfig, + filterOutH264HighProfile: !t.params.IsOfferer, + enableAudioStereo: enableAudioStereo, + enableAudioNACK: enableAudioNACK, } if !t.params.IsOfferer { t.sendersPendingConfigMu.Lock() @@ -1021,10 +1021,17 @@ func (t *PCTransport) queueOrConfigureSender( return } - configureSender(params) + // Offerer: no remote offer to echo payload types from. + configureSender(params, nil) } -func (t *PCTransport) processSendersPendingConfig() { +// processSendersPendingConfig configures the senders queued while answering the +// remote's offer (single peer connection mode). offerAudioPT (mime type -> the +// payload type the offer assigned) is parsed from the offer by the caller before +// SetRemoteDescription, so the answer echoes the offered payload types for audio +// codecs (and stays consistent with the forwarded RTP). It is nil when there is +// nothing to echo. +func (t *PCTransport) processSendersPendingConfig(offerAudioPT map[mime.MimeType]webrtc.PayloadType) { t.sendersPendingConfigMu.Lock() pending := t.sendersPendingConfig t.sendersPendingConfig = nil @@ -1037,7 +1044,7 @@ func (t *PCTransport) processSendersPendingConfig() { continue } - configureSender(p) + configureSender(p, offerAudioPT) } if len(unprocessed) != 0 { @@ -2347,11 +2354,19 @@ func (t *PCTransport) handleICEGatheringCompleteAnswerer() error { t.pendingRestartIceOffer = nil t.params.Logger.Debugw("accept remote restart ice offer after ICE gathering") + + // Parse the offer payload types before SetRemoteDescription so this does not + // race with pion's use of the same description. + var offerAudioPT map[mime.MimeType]webrtc.PayloadType + if parsed, err := offer.Unmarshal(); err == nil { + offerAudioPT = offerAudioPayloadTypes(parsed) + } + if err := t.setRemoteDescription(offer); err != nil { return err } t.params.Handler.OnSetRemoteDescriptionOffer() - t.processSendersPendingConfig() + t.processSendersPendingConfig(offerAudioPT) return t.createAndSendAnswer() } @@ -2896,7 +2911,7 @@ func (t *PCTransport) handleRemoteOfferReceived(sd *webrtc.SessionDescription, o } t.params.Handler.OnSetRemoteDescriptionOffer() - t.processSendersPendingConfig() + t.processSendersPendingConfig(offerAudioPayloadTypes(parsed)) rtxRepairs := nonSimulcastRTXRepairsFromSDP(parsed, t.params.Logger) if len(rtxRepairs) > 0 { @@ -3081,7 +3096,7 @@ type configureSenderParams struct { enableAudioNACK bool } -func configureSender(params configureSenderParams) { +func configureSender(params configureSenderParams, offerAudioPT map[mime.MimeType]webrtc.PayloadType) { configureSenderCodecs( params.transceiver, params.enabledCodecs, @@ -3090,14 +3105,14 @@ func configureSender(params configureSenderParams) { ) if params.transceiver.Kind() == webrtc.RTPCodecTypeAudio { - configureSenderAudio(params.transceiver, params.enableAudioStereo, params.enableAudioNACK) + configureSenderAudio(params.transceiver, params.enableAudioStereo, params.enableAudioNACK, offerAudioPT) } } // configure subscriber transceiver for audio stereo and nack // pion doesn't support per transciver codec configuration, so the nack of this session will be disabled // forever once it is first disabled by a transceiver. -func configureSenderAudio(tr *webrtc.RTPTransceiver, stereo bool, nack bool) { +func configureSenderAudio(tr *webrtc.RTPTransceiver, stereo bool, nack bool, offerAudioPT map[mime.MimeType]webrtc.PayloadType) { sender := tr.Sender() if sender == nil { return @@ -3121,12 +3136,65 @@ func configureSenderAudio(tr *webrtc.RTPTransceiver, stereo bool, nack bool) { } } } + // When answering a subscriber's offer (single peer connection mode), echo + // the payload type the offer assigned for this codec instead of the server's + // MediaEngine payload type. Otherwise the answer can advertise e.g. Opus on a + // PT that was never offered, which Firefox rejects (received packets decode to + // 0 samples / silence). The forwarded RTP already uses the offered PT. + if len(offerAudioPT) > 0 { + if pt, ok := offerAudioPT[mime.NormalizeMimeType(c.MimeType)]; ok { + c.PayloadType = pt + } + } configCodecs = append(configCodecs, c) } tr.SetCodecPreferences(configCodecs) } +// offerAudioPayloadTypes returns mime type -> payload type for the audio codecs +// in a remote offer, so the subscriber answer can echo the offered payload types +// (RFC 3264 6.1). The caller parses the offer before SetRemoteDescription, so this +// does not race with pion's use of the same description. +func offerAudioPayloadTypes(parsed *sdp.SessionDescription) map[mime.MimeType]webrtc.PayloadType { + if parsed == nil { + return nil + } + out := map[mime.MimeType]webrtc.PayloadType{} + for _, md := range parsed.MediaDescriptions { + if !strings.EqualFold(md.MediaName.Media, "audio") { + continue + } + for _, a := range md.Attributes { + if a.Key != "rtpmap" { + continue + } + // value e.g. "109 opus/48000/2" + fields := strings.Fields(a.Value) + if len(fields) < 2 { + continue + } + pt, err := strconv.Atoi(fields[0]) + if err != nil { + continue + } + codecName := fields[1] + if i := strings.Index(codecName, "/"); i >= 0 { + codecName = codecName[:i] + } + mt := mime.NormalizeMimeTypeCodec(codecName).ToMimeType() + if mt == mime.MimeTypeUnknown { + continue + } + out[mt] = webrtc.PayloadType(pt) + } + } + if len(out) == 0 { + return nil + } + return out +} + // In single peer connection mode, set up enebled codecs for sender. // The config provides config of direction. // For publisher peer connection those are publish enabled codecs diff --git a/pkg/rtc/transport_test.go b/pkg/rtc/transport_test.go index ab328d7c5..3c0ea425e 100644 --- a/pkg/rtc/transport_test.go +++ b/pkg/rtc/transport_test.go @@ -617,7 +617,7 @@ func TestConfigureAudioTransceiver(t *testing.T) { tr, err := pc.AddTransceiverFromKind(webrtc.RTPCodecTypeAudio, webrtc.RTPTransceiverInit{Direction: webrtc.RTPTransceiverDirectionSendonly}) require.NoError(t, err) - configureSenderAudio(tr, testcase.stereo, testcase.nack) + configureSenderAudio(tr, testcase.stereo, testcase.nack, nil) codecs := tr.Sender().GetParameters().Codecs for _, codec := range codecs { if mime.IsMimeTypeStringOpus(codec.MimeType) { @@ -636,6 +636,32 @@ func TestConfigureAudioTransceiver(t *testing.T) { } } +// When answering a subscriber offer, the sender's audio payload type must echo +// the payload type the offer assigned (RFC 3264), otherwise Firefox decodes no +// audio. See https://github.com/livekit/livekit/issues/4599. +func TestConfigureAudioTransceiverEchoesOfferPayloadType(t *testing.T) { + var me webrtc.MediaEngine + registerCodecs(&me, []*livekit.Codec{{Mime: mime.MimeTypeOpus.String()}}, RTCPFeedbackConfig{Audio: []webrtc.RTCPFeedback{{Type: webrtc.TypeRTCPFBNACK}}}, false) + pc, err := webrtc.NewAPI(webrtc.WithMediaEngine(&me)).NewPeerConnection(webrtc.Configuration{}) + require.NoError(t, err) + defer pc.Close() + tr, err := pc.AddTransceiverFromKind(webrtc.RTPCodecTypeAudio, webrtc.RTPTransceiverInit{Direction: webrtc.RTPTransceiverDirectionSendonly}) + require.NoError(t, err) + + // offer mapped Opus to a payload type different from the server MediaEngine's. + const offeredOpusPT = webrtc.PayloadType(109) + configureSenderAudio(tr, false, true, map[mime.MimeType]webrtc.PayloadType{mime.MimeTypeOpus: offeredOpusPT}) + + var found bool + for _, codec := range tr.Sender().GetParameters().Codecs { + if mime.IsMimeTypeStringOpus(codec.MimeType) { + require.Equal(t, offeredOpusPT, codec.PayloadType) + found = true + } + } + require.True(t, found, "opus codec must be present in sender preferences") +} + // In single-PC mode the publisher PC carries both publish and subscribe // directions. If the MediaEngine were built only from the publish codec list, // the SDP offer would not advertise some codecs in the m-section even though