Repository: pion/webrtc (go)

Issue:
Handling of undeclared SSRC for multimedia streams
Right now Pion only handles undeclared SSRC if the SDP contains a single media section.
We can extended this behavior to SDP with audio and single video description based on the payload type, without breaking simulcast or changing any behavior. [like chrome ](https://chromium.googlesource.com/external/webrtc/+/refs/heads/master/call/rtp_demuxer.cc#372)
I'm testing this and going to submit a PR.

related https://github.com/pion/webrtc/issues/880 

Pull request:
Resolve undeclared SSRC using the payload type
#### Description

Introduces a fallback mechanism to handle undeclared SSRCs from multiple sections using the RTP stream's payload type. For legacy clients without MID extension support, it documents the existing behavior for handling undeclared SSRCs in single media sections.

This matches the behavior with chrome and Firefox. https://chromium.googlesource.com/external/webrtc/+/refs/heads/master/call/rtp_demuxer.cc#372

#### Reference issue

resolves #3065


Patch:
diff --git a/errors.go b/errors.go
index 4bf9fe81..53942291 100644
--- a/errors.go
+++ b/errors.go
@@ -211,7 +211,7 @@ var (
 		"remoteDescription contained media section without mid value",
 	)
 	errPeerConnRemoteDescriptionNil                  = errors.New("remoteDescription has not been set yet")
-	errPeerConnSingleMediaSectionHasExplicitSSRC     = errors.New("single media section has an explicit SSRC")
+	errMediaSectionHasExplictSSRCAttribute           = errors.New("media section has an explicit SSRC")
 	errPeerConnRemoteSSRCAddTransceiver              = errors.New("could not add transceiver for remote SSRC")
 	errPeerConnSimulcastMidRTPExtensionRequired      = errors.New("mid RTP Extensions required for Simulcast")
 	errPeerConnSimulcastStreamIDRTPExtensionRequired = errors.New("stream id RTP Extensions required for Simulcast")
diff --git a/peerconnection.go b/peerconnection.go
index cfac67db..99b182ad 100644
--- a/peerconnection.go
+++ b/peerconnection.go
@@ -1577,22 +1577,16 @@ func (pc *PeerConnection) startSCTP(maxMessageSize uint32) {
 	}
 }
 
-//nolint:cyclop
 func (pc *PeerConnection) handleUndeclaredSSRC(
 	ssrc SSRC,
-	remoteDescription *SessionDescription,
+	mediaSection *sdp.MediaDescription,
 ) (handled bool, err error) {
-	if len(remoteDescription.parsed.MediaDescriptions) != 1 {
-		return false, nil
-	}
-
-	onlyMediaSection := remoteDescription.parsed.MediaDescriptions[0]
 	streamID := ""
 	id := ""
 	hasRidAttribute := false
 	hasSSRCAttribute := false
 
-	for _, a := range onlyMediaSection.Attributes {
+	for _, a := range mediaSection.Attributes {
 		switch a.Key {
 		case sdp.AttrKeyMsid:
 			if split := strings.Split(a.Value, " "); len(split) == 2 {
@@ -1609,7 +1603,7 @@ func (pc *PeerConnection) handleUndeclaredSSRC(
 	if hasRidAttribute {
 		return false, nil
 	} else if hasSSRCAttribute {
-		return false, errPeerConnSingleMediaSectionHasExplicitSSRC
+		return false, errMediaSectionHasExplictSSRCAttribute
 	}
 
 	incoming := trackDetails{
@@ -1618,7 +1612,7 @@ func (pc *PeerConnection) handleUndeclaredSSRC(
 		streamID: streamID,
 		id:       id,
 	}
-	if onlyMediaSection.MediaName.Media == RTPCodecTypeAudio.String() {
+	if mediaSection.MediaName.Media == RTPCodecTypeAudio.String() {
 		incoming.kind = RTPCodecTypeAudio
 	}
 
@@ -1636,6 +1630,38 @@ func (pc *PeerConnection) handleUndeclaredSSRC(
 	return true, nil
 }
 
+// For legacy clients that didn't support urn:ietf:params:rtp-hdrext:sdes:rtp-stream-id
+// or urn:ietf:params:rtp-hdrext:sdes:mid extension, and didn't declare a=ssrc lines.
+// Assumes that the payload type is unique across the media section.
+func (pc *PeerConnection) findMediaSectionByPayloadType(
+	payloadType PayloadType,
+	remoteDescription *SessionDescription,
+) (selectedMediaSection *sdp.MediaDescription, ok bool) {
+	for i := range remoteDescription.parsed.MediaDescriptions {
+		descr := remoteDescription.parsed.MediaDescriptions[i]
+		media := descr.MediaName.Media
+		if !strings.EqualFold(media, "video") && !strings.EqualFold(media, "audio") {
+			continue
+		}
+
+		formats := descr.MediaName.Formats
+		for _, payloadStr := range formats {
+			payload, err := strconv.ParseUint(payloadStr, 10, 8)
+			if err != nil {
+				continue
+			}
+
+			// Return the first media section that has the payload type.
+			// Assuming that the payload type is unique across the media section.
+			if PayloadType(payload) == payloadType {
+				return remoteDescription.parsed.MediaDescriptions[i], true
+			}
+		}
+	}
+
+	return nil, false
+}
+
 // Chrome sends probing traffic on SSRC 0. This reads the packets to ensure that we properly
 // generate TWCC reports for it. Since this isn't actually media we don't pass this to the user.
 func (pc *PeerConnection) handleNonMediaBandwidthProbe() {
@@ -1665,7 +1691,7 @@ func (pc *PeerConnection) handleNonMediaBandwidthProbe() {
 	}
 }
 
-func (pc *PeerConnection) handleIncomingSSRC(rtpStream io.Reader, ssrc SSRC) error { //nolint:gocognit,cyclop
+func (pc *PeerConnection) handleIncomingSSRC(rtpStream io.Reader, ssrc SSRC) error { //nolint:gocyclo,gocognit,cyclop
 	remoteDescription := pc.RemoteDescription()
 	if remoteDescription == nil {
 		return errPeerConnRemoteDescriptionNil
@@ -1683,8 +1709,34 @@ func (pc *PeerConnection) handleIncomingSSRC(rtpStream io.Reader, ssrc SSRC) err
 		}
 	}
 
-	// If the remote SDP was only one media section the ssrc doesn't have to be explicitly declared
-	if handled, err := pc.handleUndeclaredSSRC(ssrc, remoteDescription); handled || err != nil {
+	// if the SSRC is not declared in the SDP and there is only one media section,
+	// we attempt to resolve it using this single section
+	// This applies even if the client supports RTP extensions:
+	// (urn:ietf:params:rtp-hdrext:sdes:rtp-stream-id and urn:ietf:params:rtp-hdrext:sdes:mid)
+	// and even if the RTP stream contains an incorrect MID or RID.
+	// while this can be incorrect, this is done to maintain compatibility with older behavior.
+	if len(remoteDescription.parsed.MediaDescriptions) == 1 {
+		mediaSection := remoteDescription.parsed.MediaDescriptions[0]
+		if handled, err := pc.handleUndeclaredSSRC(ssrc, mediaSection); handled || err != nil {
+			return err
+		}
+	}
+
+	// We read the RTP packet to determine the payload type
+	b := make([]byte, pc.api.settingEngine.getReceiveMTU())
+
+	i, err := rtpStream.Read(b)
+	if err != nil {
+		return err
+	}
+
+	if i < 4 {
+		return errRTPTooShort
+	}
+
+	payloadType := PayloadType(b[1] & 0x7f)
+	params, err := pc.api.mediaEngine.getRTPParametersByPayloadType(payloadType)
+	if err != nil {
 		return err
 	}
 
@@ -1692,6 +1744,14 @@ func (pc *PeerConnection) handleIncomingSSRC(rtpStream io.Reader, ssrc SSRC) err
 		RTPHeaderExtensionCapability{sdp.SDESMidURI},
 	)
 	if !audioSupported && !videoSupported {
+		// try to find media section by payload type as a last resort for legacy clients.
+		mediaSection, ok := pc.findMediaSectionByPayloadType(payloadType, remoteDescription)
+		if ok {
+			if ok, err = pc.handleUndeclaredSSRC(ssrc, mediaSection); ok || err != nil {
+				return err
+			}
+		}
+
 		return errPeerConnSimulcastMidRTPExtensionRequired
 	}
 
@@ -1706,23 +1766,6 @@ func (pc *PeerConnection) handleIncomingSSRC(rtpStream io.Reader, ssrc SSRC) err
 		RTPHeaderExtensionCapability{sdp.SDESRepairRTPStreamIDURI},
 	)
 
-	b := make([]byte, pc.api.settingEngine.getReceiveMTU())
-
-	i, err := rtpStream.Read(b)
-	if err != nil {
-		return err
-	}
-
-	if i < 4 {
-		return errRTPTooShort
-	}
-
-	payloadType := PayloadType(b[1] & 0x7f)
-	params, err := pc.api.mediaEngine.getRTPParametersByPayloadType(payloadType)
-	if err != nil {
-		return err
-	}
-
 	streamInfo := createStreamInfo(
 		"",
 		ssrc,


Test patch:
diff --git a/peerconnection_media_test.go b/peerconnection_media_test.go
index 30c9307a..20cf4f9d 100644
--- a/peerconnection_media_test.go
+++ b/peerconnection_media_test.go
@@ -441,6 +441,20 @@ func filterSsrc(offer string) (filteredSDP string) {
 	return
 }
 
+func filterSDPExtensions(offer string) (filteredSDP string) {
+	scanner := bufio.NewScanner(strings.NewReader(offer))
+	for scanner.Scan() {
+		l := scanner.Text()
+		if strings.HasPrefix(l, "a=extmap") {
+			continue
+		}
+
+		filteredSDP += l + "\n"
+	}
+
+	return
+}
+
 // If a SessionDescription has a single media section and no SSRC
 // assume that it is meant to handle all RTP packets.
 func TestUndeclaredSSRC(t *testing.T) {
@@ -530,6 +544,127 @@ func TestUndeclaredSSRC(t *testing.T) {
 		sendVideoUntilDone(t, unhandledSimulcastError, []*TrackLocalStaticSample{vp8Writer})
 		closePairNow(t, pcOffer, pcAnswer)
 	})
+
+	t.Run("multiple media sections, no sdp extensions", func(t *testing.T) {
+		pcOffer, pcAnswer, err := newPair()
+		assert.NoError(t, err)
+
+		vp8Writer, err := NewTrackLocalStaticSample(RTPCodecCapability{MimeType: MimeTypeVP8}, "video", "pion")
+		assert.NoError(t, err)
+
+		_, err = pcOffer.CreateDataChannel("data", nil)
+		assert.NoError(t, err)
+
+		_, err = pcOffer.AddTrack(vp8Writer)
+		assert.NoError(t, err)
+
+		opusWriter, err := NewTrackLocalStaticSample(RTPCodecCapability{MimeType: MimeTypeOpus}, "audio", "pion")
+		assert.NoError(t, err)
+
+		_, err = pcOffer.AddTrack(opusWriter)
+		assert.NoError(t, err)
+
+		onVideoTrackFired := make(chan struct{})
+		onAudioTrackFired := make(chan struct{})
+
+		gotVideo, gotAudio := false, false
+		pcAnswer.OnTrack(func(trackRemote *TrackRemote, _ *RTPReceiver) {
+			switch trackRemote.Kind() {
+			case RTPCodecTypeVideo:
+				assert.False(t, gotVideo, "already got video track")
+				assert.Equal(t, trackRemote.StreamID(), vp8Writer.StreamID())
+				assert.Equal(t, trackRemote.ID(), vp8Writer.ID())
+				gotVideo = true
+				onVideoTrackFired <- struct{}{}
+			case RTPCodecTypeAudio:
+				assert.False(t, gotAudio, "already got audio track")
+				assert.Equal(t, trackRemote.StreamID(), opusWriter.StreamID())
+				assert.Equal(t, trackRemote.ID(), opusWriter.ID())
+				gotAudio = true
+				onAudioTrackFired <- struct{}{}
+			default:
+				assert.Fail(t, "unexpected track kind", trackRemote.Kind())
+			}
+		})
+
+		offer, err := pcOffer.CreateOffer(nil)
+		assert.NoError(t, err)
+
+		offerGatheringComplete := GatheringCompletePromise(pcOffer)
+		assert.NoError(t, pcOffer.SetLocalDescription(offer))
+		<-offerGatheringComplete
+
+		offer.SDP = filterSDPExtensions(filterSsrc(pcOffer.LocalDescription().SDP))
+		assert.NoError(t, pcAnswer.SetRemoteDescription(offer))
+
+		answer, err := pcAnswer.CreateAnswer(nil)
+		assert.NoError(t, err)
+
+		answerGatheringComplete := GatheringCompletePromise(pcAnswer)
+		assert.NoError(t, pcAnswer.SetLocalDescription(answer))
+		<-answerGatheringComplete
+
+		assert.NoError(t, pcOffer.SetRemoteDescription(*pcAnswer.LocalDescription()))
+
+		wait := sync.WaitGroup{}
+		wait.Add(2)
+		go func() {
+			sendVideoUntilDone(t, onVideoTrackFired, []*TrackLocalStaticSample{vp8Writer})
+			wait.Done()
+		}()
+		go func() {
+			sendVideoUntilDone(t, onAudioTrackFired, []*TrackLocalStaticSample{opusWriter})
+			wait.Done()
+		}()
+
+		wait.Wait()
+		closePairNow(t, pcOffer, pcAnswer)
+	})
+
+	t.Run("findMediaSectionByPayloadType test", func(t *testing.T) {
+		parsed := &SessionDescription{
+			parsed: &sdp.SessionDescription{
+				MediaDescriptions: []*sdp.MediaDescription{
+					{
+						MediaName: sdp.MediaName{
+							Media:   "video",
+							Protos:  []string{"UDP", "TLS", "RTP", "SAVPF"},
+							Formats: []string{"96", "97", "98", "99", "BAD", "100", "101", "102"},
+						},
+					},
+					{
+						MediaName: sdp.MediaName{
+							Media:   "audio",
+							Protos:  []string{"UDP", "TLS", "RTP", "SAVPF"},
+							Formats: []string{"8", "9", "101"},
+						},
+					},
+					{
+						MediaName: sdp.MediaName{
+							Media:   "application",
+							Protos:  []string{"UDP", "DTLS", "SCTP"},
+							Formats: []string{"webrtc-datachannel"},
+						},
+					},
+				},
+			},
+		}
+		peer := &PeerConnection{}
+
+		video, ok := peer.findMediaSectionByPayloadType(96, parsed)
+		assert.True(t, ok)
+		assert.NotNil(t, video)
+		assert.Equal(t, "video", video.MediaName.Media)
+
+		audio, ok := peer.findMediaSectionByPayloadType(8, parsed)
+		assert.True(t, ok)
+		assert.NotNil(t, audio)
+		assert.Equal(t, "audio", audio.MediaName.Media)
+
+		missing, ok := peer.findMediaSectionByPayloadType(42, parsed)
+		assert.False(t, ok)
+		assert.Nil(t, missing)
+	})
 }
 
 func TestAddTransceiverFromTrackSendOnly(t *testing.T) {