diff --git a/internal/session/video_codec.go b/internal/session/video_codec.go index 550ba36..834599a 100644 --- a/internal/session/video_codec.go +++ b/internal/session/video_codec.go @@ -65,6 +65,7 @@ func offeredVideoCodecs(raw string) ([]offeredVideoCodec, error) { if !strings.EqualFold(mediaDescription.MediaName.Media, "video") { continue } + feedback := offeredRTCPFeedback(mediaDescription) for _, format := range mediaDescription.MediaName.Formats { payloadType, err := strconv.ParseUint(format, 10, 8) if err != nil { @@ -81,7 +82,7 @@ func offeredVideoCodecs(raw string) ([]offeredVideoCodec, error) { result = append(result, offeredVideoCodec{ codec: videoCodec, parameters: webrtc.RTPCodecParameters{ - RTPCodecCapability: webrtc.RTPCodecCapability{MimeType: string(videoCodec), ClockRate: codec.ClockRate, SDPFmtpLine: codec.Fmtp}, + RTPCodecCapability: webrtc.RTPCodecCapability{MimeType: string(videoCodec), ClockRate: codec.ClockRate, SDPFmtpLine: codec.Fmtp, RTCPFeedback: feedback[uint8(payloadType)]}, PayloadType: webrtc.PayloadType(payloadType), }, }) @@ -93,6 +94,30 @@ func offeredVideoCodecs(raw string) ([]offeredVideoCodec, error) { return result, nil } +// offeredRTCPFeedback collects the a=rtcp-fb lines of a media section by +// payload type. SetCodecPreferences overwrites the transceiver's codec +// capability wholesale, so a codec rebuilt from the offer must carry the +// feedback the client asked for or the answer advertises none at all. +func offeredRTCPFeedback(mediaDescription *sdp.MediaDescription) map[uint8][]webrtc.RTCPFeedback { + feedback := make(map[uint8][]webrtc.RTCPFeedback) + for _, attribute := range mediaDescription.Attributes { + if attribute.Key != "rtcp-fb" { + continue + } + format, value, found := strings.Cut(attribute.Value, " ") + if !found { + continue + } + payloadType, err := strconv.ParseUint(format, 10, 8) + if err != nil { + continue + } + feedbackType, parameter, _ := strings.Cut(strings.TrimSpace(value), " ") + feedback[uint8(payloadType)] = append(feedback[uint8(payloadType)], webrtc.RTCPFeedback{Type: feedbackType, Parameter: parameter}) + } + return feedback +} + func firstVideoTransceiver(pc *webrtc.PeerConnection) *webrtc.RTPTransceiver { for _, transceiver := range pc.GetTransceivers() { if transceiver.Kind() == webrtc.RTPCodecTypeVideo { diff --git a/internal/session/video_codec_e2e_test.go b/internal/session/video_codec_e2e_test.go new file mode 100644 index 0000000..0b1c399 --- /dev/null +++ b/internal/session/video_codec_e2e_test.go @@ -0,0 +1,128 @@ +package session + +import ( + "sort" + "sync" + "testing" + "time" + + "github.com/pion/webrtc/v4" +) + +// TestConnectedPeerNegotiatesRTCPFeedback is the runtime counterpart to +// TestAnswerRetainsOfferedRTCPFeedback: it completes a real ICE + DTLS +// handshake against the manager and asserts on the codec parameters the client +// ends up applying. Pion decides whether to run its NACK, PLI and transport-cc +// interceptors from these negotiated parameters, not from the SDP text, so this +// is what actually determines whether feedback works on a live session. +func TestConnectedPeerNegotiatesRTCPFeedback(t *testing.T) { + manager := newTestManager(t, 0) + liveSession, ownerToken, err := manager.Create(nil) + if err != nil { + t.Fatal(err) + } + client, err := webrtc.NewPeerConnection(webrtc.Configuration{}) + if err != nil { + t.Fatal(err) + } + defer client.Close() + + capability := webrtc.RTPCodecCapability{ + MimeType: webrtc.MimeTypeH264, + ClockRate: 90000, + SDPFmtpLine: "level-asymmetry-allowed=1;packetization-mode=1;profile-level-id=42e01f", + RTCPFeedback: []webrtc.RTCPFeedback{ + {Type: "goog-remb"}, + {Type: "ccm", Parameter: "fir"}, + {Type: "nack"}, + {Type: "nack", Parameter: "pli"}, + {Type: "transport-cc"}, + }, + } + track, err := webrtc.NewTrackLocalStaticSample(capability, "camera", "client") + if err != nil { + t.Fatal(err) + } + transceiver, err := client.AddTransceiverFromTrack(track, webrtc.RTPTransceiverInit{Direction: webrtc.RTPTransceiverDirectionSendrecv}) + if err != nil { + t.Fatal(err) + } + if err = transceiver.SetCodecPreferences([]webrtc.RTPCodecParameters{{RTPCodecCapability: capability}}); err != nil { + t.Fatal(err) + } + + connected := make(chan struct{}) + var once sync.Once + client.OnConnectionStateChange(func(state webrtc.PeerConnectionState) { + if state == webrtc.PeerConnectionStateConnected { + once.Do(func() { close(connected) }) + } + }) + + offer, err := client.CreateOffer(nil) + if err != nil { + t.Fatal(err) + } + gathered := webrtc.GatheringCompletePromise(client) + if err = client.SetLocalDescription(offer); err != nil { + t.Fatal(err) + } + <-gathered + + answer, err := manager.CreateAnswer(liveSession.ID, ownerToken, client.LocalDescription().SDP) + if err != nil { + t.Fatal(err) + } + if err = client.SetRemoteDescription(webrtc.SessionDescription{Type: webrtc.SDPTypeAnswer, SDP: answer.SDP}); err != nil { + t.Fatal(err) + } + + select { + case <-connected: + case <-time.After(15 * time.Second): + t.Fatalf("peer connection never connected (ice=%s, state=%s)", client.ICEConnectionState(), client.ConnectionState()) + } + + want := []string{"ccm fir", "goog-remb", "nack", "nack pli", "transport-cc"} + for _, direction := range []struct { + name string + codecs []webrtc.RTPCodecParameters + }{ + {"receiver", transceiver.Receiver().GetParameters().Codecs}, + {"sender", transceiver.Sender().GetParameters().Codecs}, + } { + if len(direction.codecs) == 0 { + t.Errorf("%s negotiated no codec at all", direction.name) + continue + } + got := negotiatedFeedback(direction.codecs[0].RTCPFeedback) + for _, feedback := range want { + if !got[feedback] { + t.Errorf("%s codec %s did not negotiate %q feedback (got %v)", + direction.name, direction.codecs[0].MimeType, feedback, sortedKeys(got)) + } + } + } +} + +// negotiatedFeedback keys RTCPFeedback the way it appears in SDP ("nack pli"). +func negotiatedFeedback(feedback []webrtc.RTCPFeedback) map[string]bool { + present := make(map[string]bool, len(feedback)) + for _, item := range feedback { + if item.Parameter == "" { + present[item.Type] = true + continue + } + present[item.Type+" "+item.Parameter] = true + } + return present +} + +func sortedKeys(set map[string]bool) []string { + keys := make([]string, 0, len(set)) + for key := range set { + keys = append(keys, key) + } + sort.Strings(keys) + return keys +} diff --git a/internal/session/video_codec_test.go b/internal/session/video_codec_test.go index 3f2b55b..a01ff00 100644 --- a/internal/session/video_codec_test.go +++ b/internal/session/video_codec_test.go @@ -70,3 +70,108 @@ func TestCreateAnswerSelectsClientPreferredH264(t *testing.T) { t.Fatalf("answer did not retain the H.264-only offer:\n%s", answer.SDP) } } + +// TestAnswerRetainsOfferedRTCPFeedback guards the RTCP feedback the client +// offers. Rebuilding the codec from the offer SDP drops any RTCPFeedback that +// prepareOutputForOffer does not copy over, and SetCodecPreferences then +// replaces the transceiver's full codec capability — so an empty list silently +// strips every a=rtcp-fb line from the answer. Losing "nack" removes the only +// packet-recovery path and losing "nack pli" removes the client's only way to +// ask for a keyframe, which corrupts the decoder's static regions until the +// encoder's next natural IDR. +func TestAnswerRetainsOfferedRTCPFeedback(t *testing.T) { + manager := newTestManager(t, 0) + liveSession, ownerToken, err := manager.Create(nil) + if err != nil { + t.Fatal(err) + } + client, err := webrtc.NewPeerConnection(webrtc.Configuration{}) + if err != nil { + t.Fatal(err) + } + defer client.Close() + capability := webrtc.RTPCodecCapability{ + MimeType: webrtc.MimeTypeH264, + ClockRate: 90000, + SDPFmtpLine: "level-asymmetry-allowed=1;packetization-mode=1;profile-level-id=42e01f", + RTCPFeedback: []webrtc.RTCPFeedback{ + {Type: "goog-remb"}, + {Type: "ccm", Parameter: "fir"}, + {Type: "nack"}, + {Type: "nack", Parameter: "pli"}, + {Type: "transport-cc"}, + }, + } + track, err := webrtc.NewTrackLocalStaticSample(capability, "camera", "client") + if err != nil { + t.Fatal(err) + } + transceiver, err := client.AddTransceiverFromTrack(track, webrtc.RTPTransceiverInit{Direction: webrtc.RTPTransceiverDirectionSendrecv}) + if err != nil { + t.Fatal(err) + } + if err = transceiver.SetCodecPreferences([]webrtc.RTPCodecParameters{{RTPCodecCapability: capability}}); err != nil { + t.Fatal(err) + } + offer, err := client.CreateOffer(nil) + if err != nil { + t.Fatal(err) + } + if err = client.SetLocalDescription(offer); err != nil { + t.Fatal(err) + } + answer, err := manager.CreateAnswer(liveSession.ID, ownerToken, offer.SDP) + if err != nil { + t.Fatal(err) + } + + offered := videoFeedbackLines(offer.SDP) + answered := videoFeedbackLines(answer.SDP) + for _, feedback := range []string{"nack", "nack pli", "ccm fir", "goog-remb", "transport-cc"} { + if offered[feedback] == 0 { + t.Fatalf("offer is missing %q feedback, test setup is wrong:\n%s", feedback, offer.SDP) + } + if answered[feedback] == 0 { + t.Errorf("answer dropped %q feedback offered by the client", feedback) + } + } + if t.Failed() { + t.Logf("answer m=video section:\n%s", strings.Join(videoSectionLines(answer.SDP), "\n")) + } +} + +// videoSectionLines returns the lines of the first m=video section of an SDP. +func videoSectionLines(sdp string) []string { + var lines []string + inVideo := false + for _, line := range strings.Split(sdp, "\r\n") { + if strings.HasPrefix(line, "m=") { + if inVideo { + break + } + inVideo = strings.HasPrefix(line, "m=video") + } + if inVideo { + lines = append(lines, line) + } + } + return lines +} + +// videoFeedbackLines counts a=rtcp-fb lines in the video section by feedback +// type, keyed the way they appear in SDP ("nack", "nack pli", "ccm fir", ...). +func videoFeedbackLines(sdp string) map[string]int { + counts := make(map[string]int) + for _, line := range videoSectionLines(sdp) { + _, attribute, found := strings.Cut(line, "a=rtcp-fb:") + if !found { + continue + } + _, feedback, found := strings.Cut(attribute, " ") + if !found { + continue + } + counts[strings.TrimSpace(feedback)]++ + } + return counts +}