Fix m-line ordering in SDP offers to match track insertion order (#1551)

Author: DavidWever

src/SIPSorcery/net/RTP/RTPSession.cs

@@ -111,6 +111,7 @@ public class RTPSession : IMediaSession, IDisposable
         private string m_sdpSessionID = null;           // Need to maintain the same SDP session ID for all offers and answers.
         private ulong m_sdpAnnouncementVersion = 0;     // The SDP version needs to increase whenever the local SDP is modified (see https://tools.ietf.org/html/rfc6337#section-5.2.5).
         internal int m_rtpChannelsCount = 0;            // Need to know the number of RTP Channels
+        private int m_nextMediaInsertionOrder = 0;      // Need to keep track of insertion order of different media for SDP renegotiation.
 
         // The stream used for the underlying RTP session to create a single RTP channel that will
         // be used to multiplex all required media streams. (see addSingleTrack())
@@ -1033,6 +1034,7 @@ protected virtual AudioStream GetOrCreateAudioStream(int index)
             else if (index == AudioStreamList.Count)
             {
                 AudioStream audioStream = new AudioStream(rtpSessionConfig, index);
+                audioStream.MediaInsertionOrder = Interlocked.Increment(ref m_nextMediaInsertionOrder);
                 AudioStreamList.Add(audioStream);
                 return audioStream;
             }
@@ -1049,6 +1051,7 @@ protected virtual VideoStream GetOrCreateVideoStream(int index)
             else if (index == VideoStreamList.Count)
             {
                 VideoStream videoStream = new VideoStream(rtpSessionConfig, index);
+                videoStream.MediaInsertionOrder = Interlocked.Increment(ref m_nextMediaInsertionOrder);
                 VideoStreamList.Add(videoStream);
                 return videoStream;
             }
@@ -1064,6 +1067,7 @@ protected virtual TextStream GetOrCreateTextStream(int index)
             else if (index == TextStreamList.Count)
             {
                 TextStream textStream = new TextStream(rtpSessionConfig, index);
+                textStream.MediaInsertionOrder = Interlocked.Increment(ref m_nextMediaInsertionOrder);
                 TextStreamList.Add(textStream);
                 return textStream;
             }
@@ -2256,51 +2260,26 @@ protected virtual RTPChannel CreateRtpChannel()
         /// <returns>A list of the local tracks that have been added to this session.</returns>
         protected List<MediaStream> GetMediaStreams()
         {
-            List<MediaStream> mediaStream = new List<MediaStream>();
+            List<MediaStream> mediaStreams = new List<MediaStream>();
 
-            foreach (var audioStream in AudioStreamList)
-            {
-                if (audioStream.LocalTrack != null)
-                {
-                    mediaStream.Add(audioStream);
-                }
-                else if (audioStream.RtcpSession != null && !audioStream.RtcpSession.IsClosed && audioStream.RemoteTrack != null)
-                {
-                    var inactiveAudioTrack = new MediaStreamTrack(audioStream.MediaType, false, audioStream.RemoteTrack.Capabilities, MediaStreamStatusEnum.Inactive);
-                    audioStream.LocalTrack = inactiveAudioTrack;
-                    mediaStream.Add(audioStream);
-                }
-            }
-
-            foreach (var videoStream in VideoStreamList)
-            {
-                if (videoStream.LocalTrack != null)
-                {
-                    mediaStream.Add(videoStream);
-                }
-                else if (videoStream.RtcpSession != null && !videoStream.RtcpSession.IsClosed && videoStream.RemoteTrack != null)
-                {
-                    var inactiveAudioTrack = new MediaStreamTrack(videoStream.MediaType, false, videoStream.RemoteTrack.Capabilities, MediaStreamStatusEnum.Inactive);
-                    videoStream.LocalTrack = inactiveAudioTrack;
-                    mediaStream.Add(videoStream);
-                }
-            }
-
-            foreach (var textstram in TextStreamList)
+            foreach (var stream in AudioStreamList
+                .Concat<MediaStream>(VideoStreamList)
+                .Concat(TextStreamList))
             {
-                if (textstram.LocalTrack != null)
+                if (stream.LocalTrack != null)
                 {
-                    mediaStream.Add(textstram);
+                    mediaStreams.Add(stream);
                 }
-                else if (textstram.RtcpSession != null && !textstram.RtcpSession.IsClosed && textstram.RemoteTrack != null)
+                else if (stream.RtcpSession != null && !stream.RtcpSession.IsClosed && stream.RemoteTrack != null)
                 {
-                    var inactiveTextTrack = new MediaStreamTrack(textstram.MediaType, false, textstram.RemoteTrack.Capabilities, MediaStreamStatusEnum.Inactive);
-                    textstram.LocalTrack = inactiveTextTrack;
-                    mediaStream.Add(textstram);
+                    var inactiveTrack = new MediaStreamTrack(stream.MediaType, false, stream.RemoteTrack.Capabilities, MediaStreamStatusEnum.Inactive);
+                    stream.LocalTrack = inactiveTrack;
+                    mediaStreams.Add(stream);
                 }
             }
 
-            return mediaStream;
+            mediaStreams.Sort((a, b) => a.MediaInsertionOrder.CompareTo(b.MediaInsertionOrder));
+            return mediaStreams;
         }
 
         /// <summary>

src/SIPSorcery/net/RTP/Streams/MediaStream.cs

@@ -68,6 +68,12 @@ public PendingPackages(RTPHeader hdr, int localPort, IPEndPoint remoteEndPoint,
 
         public int Index = -1;
 
+        /// <summary>
+        /// Tracks the global order in which this stream was added to the session,
+        /// across all media types. Used to preserve m-line ordering per RFC 3264 §8.
+        /// </summary>
+        public int MediaInsertionOrder = -1;
+
         /// <summary>
         /// Fires when the connection for a media type is classified as timed out due to not
         /// receiving any RTP or RTCP packets within the given period.

test/unit/net/RTP/RTPSessionUnitTest.cs

@@ -606,5 +606,71 @@ public void ModifiedWellKnownFormatIDUnitTest()
 
             rtpSession.Close("normal");
         }
+
+        /// <summary>
+        /// Tests that when video is added before audio, the SDP offer preserves that order
+        /// (video m-line first, audio m-line second) per RFC 3264 §8.
+        /// </summary>
+        [Fact]
+        public void OfferMLineOrderVideoFirstThenAudio()
+        {
+            logger.LogDebug("--> {MethodName}", System.Reflection.MethodBase.GetCurrentMethod().Name);
+            logger.BeginScope(System.Reflection.MethodBase.GetCurrentMethod().Name);
+
+            RTPSession rtpSession = new RTPSession(false, false, false);
+
+            // Add video first.
+            MediaStreamTrack videoTrack = new MediaStreamTrack(SDPMediaTypesEnum.video, false,
+                new List<SDPAudioVideoMediaFormat> { new SDPAudioVideoMediaFormat(SDPMediaTypesEnum.video, 96, "VP8", 90000) });
+            rtpSession.addTrack(videoTrack);
+
+            // Add audio second.
+            MediaStreamTrack audioTrack = new MediaStreamTrack(SDPMediaTypesEnum.audio, false,
+                new List<SDPAudioVideoMediaFormat> { new SDPAudioVideoMediaFormat(SDPWellKnownMediaFormatsEnum.PCMU) });
+            rtpSession.addTrack(audioTrack);
+
+            var offer = rtpSession.CreateOffer(IPAddress.Loopback);
+
+            logger.LogDebug("Offer SDP:\n{Sdp}", offer);
+
+            Assert.Equal(2, offer.Media.Count);
+            Assert.Equal(SDPMediaTypesEnum.video, offer.Media[0].Media);
+            Assert.Equal(SDPMediaTypesEnum.audio, offer.Media[1].Media);
+
+            rtpSession.Close("normal");
+        }
+
+        /// <summary>
+        /// Tests that the traditional audio-then-video order is preserved when tracks are added
+        /// in that order (backward compatibility).
+        /// </summary>
+        [Fact]
+        public void OfferMLineOrderAudioFirstThenVideo()
+        {
+            logger.LogDebug("--> {MethodName}", System.Reflection.MethodBase.GetCurrentMethod().Name);
+            logger.BeginScope(System.Reflection.MethodBase.GetCurrentMethod().Name);
+
+            RTPSession rtpSession = new RTPSession(false, false, false);
+
+            // Add audio first.
+            MediaStreamTrack audioTrack = new MediaStreamTrack(SDPMediaTypesEnum.audio, false,
+                new List<SDPAudioVideoMediaFormat> { new SDPAudioVideoMediaFormat(SDPWellKnownMediaFormatsEnum.PCMU) });
+            rtpSession.addTrack(audioTrack);
+
+            // Add video second.
+            MediaStreamTrack videoTrack = new MediaStreamTrack(SDPMediaTypesEnum.video, false,
+                new List<SDPAudioVideoMediaFormat> { new SDPAudioVideoMediaFormat(SDPMediaTypesEnum.video, 96, "VP8", 90000) });
+            rtpSession.addTrack(videoTrack);
+
+            var offer = rtpSession.CreateOffer(IPAddress.Loopback);
+
+            logger.LogDebug("Offer SDP:\n{Sdp}", offer);
+
+            Assert.Equal(2, offer.Media.Count);
+            Assert.Equal(SDPMediaTypesEnum.audio, offer.Media[0].Media);
+            Assert.Equal(SDPMediaTypesEnum.video, offer.Media[1].Media);
+
+            rtpSession.Close("normal");
+        }
     }
 }