Fix #3179: WebRTC: Make sure the same m-lines order for offer and ans…

…wer. v4.0.265
ossrs · Sep 15, 2022 · 0ffd716 · 0ffd716
1 parent aea2bfb
commit 0ffd716
Show file tree

Hide file tree

Showing 7 changed files with 94 additions and 8 deletions.
diff --git a/trunk/doc/CHANGELOG.md b/trunk/doc/CHANGELOG.md
@@ -8,6 +8,7 @@ The changelog for SRS.
 
 ## SRS 4.0 Changelog
 
+* v4.0, 2022-09-16, For [#3179](https://github.com/ossrs/srs/issues/3179): WebRTC: Make sure the same m-lines order for offer and answer. v4.0.265
 * v4.0, 2022-09-09, For [#3174](https://github.com/ossrs/srs/issues/3174): WebRTC: Support Unity to publish or play stream. v4.0.264
 * v4.0, 2022-09-09, Fix [#3093](https://github.com/ossrs/srs/issues/3093): WebRTC: Ignore unknown fmtp for h.264. v4.0.263
 * v4.0, 2022-09-06, Fix [#3170](https://github.com/ossrs/srs/issues/3170): WebRTC: Support WHIP(WebRTC-HTTP ingestion protocol). v4.0.262

diff --git a/trunk/research/players/js/srs.sdk.js b/trunk/research/players/js/srs.sdk.js
@@ -301,8 +301,8 @@ function SrsRtcPlayerAsync() {
     //      webrtc://r.ossrs.net/live/livestream?token=xxx
     self.play = async function(url) {
         var conf = self.__internal.prepareUrl(url);
-        self.pc.addTransceiver("audio", {direction: "recvonly"});
         self.pc.addTransceiver("video", {direction: "recvonly"});
+        self.pc.addTransceiver("audio", {direction: "recvonly"});
 
         var offer = await self.pc.createOffer();
         await self.pc.setLocalDescription(offer);

diff --git a/trunk/src/app/srs_app_rtc_conn.cpp b/trunk/src/app/srs_app_rtc_conn.cpp
@@ -1992,7 +1992,7 @@ srs_error_t SrsRtcConnection::add_publisher(SrsRtcUserConfig* ruc, SrsSdp& local
         return srs_error_wrap(err, "publish negotiate");
     }
 
-    if ((err = generate_publish_local_sdp(req, local_sdp, stream_desc, ruc->remote_sdp_.is_unified())) != srs_success) {
+    if ((err = generate_publish_local_sdp(req, local_sdp, stream_desc, ruc->remote_sdp_.is_unified(), ruc->audio_before_video_)) != srs_success) {
         return srs_error_wrap(err, "generate local sdp");
     }
 
@@ -2058,7 +2058,7 @@ srs_error_t SrsRtcConnection::add_player(SrsRtcUserConfig* ruc, SrsSdp& local_sd
         ++it;
     }
 
-    if ((err = generate_play_local_sdp(req, local_sdp, stream_desc, ruc->remote_sdp_.is_unified())) != srs_success) {
+    if ((err = generate_play_local_sdp(req, local_sdp, stream_desc, ruc->remote_sdp_.is_unified(), ruc->audio_before_video_)) != srs_success) {
         return srs_error_wrap(err, "generate local sdp");
     }
 
@@ -2853,9 +2853,14 @@ srs_error_t SrsRtcConnection::negotiate_publish_capability(SrsRtcUserConfig* ruc
     // TODO: FIME: Should check packetization-mode=1 also.
     bool has_42e01f = srs_sdp_has_h264_profile(remote_sdp, "42e01f");
 
+    // How many video descriptions we have parsed.
+    int nn_any_video_parsed = 0;
+
     for (int i = 0; i < (int)remote_sdp.media_descs_.size(); ++i) {
         const SrsMediaDesc& remote_media_desc = remote_sdp.media_descs_.at(i);
 
+        if (remote_media_desc.is_video()) nn_any_video_parsed++;
+
         SrsRtcTrackDescription* track_desc = new SrsRtcTrackDescription();
         SrsAutoFree(SrsRtcTrackDescription, track_desc);
 
@@ -2878,6 +2883,9 @@ srs_error_t SrsRtcConnection::negotiate_publish_capability(SrsRtcUserConfig* ruc
         }
 
         if (remote_media_desc.is_audio()) {
+            // Update the ruc, which is about user specified configuration.
+            ruc->audio_before_video_ = !nn_any_video_parsed;
+
             // TODO: check opus format specific param
             std::vector<SrsMediaPayloadType> payloads = remote_media_desc.find_media_with_encoding_name("opus");
             if (payloads.empty()) {
@@ -3091,7 +3099,7 @@ srs_error_t SrsRtcConnection::negotiate_publish_capability(SrsRtcUserConfig* ruc
     return err;
 }
 
-srs_error_t SrsRtcConnection::generate_publish_local_sdp(SrsRequest* req, SrsSdp& local_sdp, SrsRtcSourceDescription* stream_desc, bool unified_plan)
+srs_error_t SrsRtcConnection::generate_publish_local_sdp(SrsRequest* req, SrsSdp& local_sdp, SrsRtcSourceDescription* stream_desc, bool unified_plan, bool audio_before_video)
 {
     srs_error_t err = srs_success;
 
@@ -3116,6 +3124,29 @@ srs_error_t SrsRtcConnection::generate_publish_local_sdp(SrsRequest* req, SrsSdp
 
     local_sdp.group_policy_ = "BUNDLE";
 
+    if (audio_before_video) {
+        if ((err = generate_publish_local_sdp_for_audio(local_sdp, stream_desc)) != srs_success) {
+            return srs_error_wrap(err, "audio");
+        }
+        if ((err = generate_publish_local_sdp_for_video(local_sdp, stream_desc, unified_plan)) != srs_success) {
+            return srs_error_wrap(err, "video");
+        }
+    } else {
+        if ((err = generate_publish_local_sdp_for_video(local_sdp, stream_desc, unified_plan)) != srs_success) {
+            return srs_error_wrap(err, "video");
+        }
+        if ((err = generate_publish_local_sdp_for_audio(local_sdp, stream_desc)) != srs_success) {
+            return srs_error_wrap(err, "audio");
+        }
+    }
+
+    return err;
+}
+
+srs_error_t SrsRtcConnection::generate_publish_local_sdp_for_audio(SrsSdp& local_sdp, SrsRtcSourceDescription* stream_desc)
+{
+    srs_error_t err = srs_success;
+
     // generate audio media desc
     if (stream_desc->audio_track_desc_) {
         SrsRtcTrackDescription* audio_track = stream_desc->audio_track_desc_;
@@ -3150,6 +3181,13 @@ srs_error_t SrsRtcConnection::generate_publish_local_sdp(SrsRequest* req, SrsSdp
         local_media_desc.payload_types_.push_back(payload->generate_media_payload_type());
     }
 
+    return err;
+}
+
+srs_error_t SrsRtcConnection::generate_publish_local_sdp_for_video(SrsSdp& local_sdp, SrsRtcSourceDescription* stream_desc, bool unified_plan)
+{
+    srs_error_t err = srs_success;
+
     for (int i = 0;  i < (int)stream_desc->video_track_descs_.size(); ++i) {
         SrsRtcTrackDescription* video_track = stream_desc->video_track_descs_.at(i);
 
@@ -3213,9 +3251,14 @@ srs_error_t SrsRtcConnection::negotiate_play_capability(SrsRtcUserConfig* ruc, s
         return srs_error_wrap(err, "fetch rtc source");
     }
 
+    // How many video descriptions we have parsed.
+    int nn_any_video_parsed = 0;
+
     for (int i = 0; i < (int)remote_sdp.media_descs_.size(); ++i) {
         const SrsMediaDesc& remote_media_desc = remote_sdp.media_descs_.at(i);
 
+        if (remote_media_desc.is_video()) nn_any_video_parsed++;
+
         // Whether feature enabled in remote extmap.
         int remote_twcc_id = 0;
         if (true) {
@@ -3231,6 +3274,9 @@ srs_error_t SrsRtcConnection::negotiate_play_capability(SrsRtcUserConfig* ruc, s
         std::vector<SrsRtcTrackDescription*> track_descs;
         SrsMediaPayloadType remote_payload(0);
         if (remote_media_desc.is_audio()) {
+            // Update the ruc, which is about user specified configuration.
+            ruc->audio_before_video_ = !nn_any_video_parsed;
+
             // TODO: check opus format specific param
             vector<SrsMediaPayloadType> payloads = remote_media_desc.find_media_with_encoding_name("opus");
             if (payloads.empty()) {
@@ -3390,7 +3436,7 @@ void video_track_generate_play_offer(SrsRtcTrackDescription* track, string mid,
     }
 }
 
-srs_error_t SrsRtcConnection::generate_play_local_sdp(SrsRequest* req, SrsSdp& local_sdp, SrsRtcSourceDescription* stream_desc, bool unified_plan)
+srs_error_t SrsRtcConnection::generate_play_local_sdp(SrsRequest* req, SrsSdp& local_sdp, SrsRtcSourceDescription* stream_desc, bool unified_plan, bool audio_before_video)
 {
     srs_error_t err = srs_success;
 
@@ -3417,6 +3463,29 @@ srs_error_t SrsRtcConnection::generate_play_local_sdp(SrsRequest* req, SrsSdp& l
 
     std::string cname = srs_random_str(16);
 
+    if (audio_before_video) {
+        if ((err = generate_play_local_sdp_for_audio(local_sdp, stream_desc, cname)) != srs_success) {
+            return srs_error_wrap(err, "audio");
+        }
+        if ((err = generate_play_local_sdp_for_video(local_sdp, stream_desc, unified_plan, cname)) != srs_success) {
+            return srs_error_wrap(err, "video");
+        }
+    } else {
+        if ((err = generate_play_local_sdp_for_video(local_sdp, stream_desc, unified_plan, cname)) != srs_success) {
+            return srs_error_wrap(err, "video");
+        }
+        if ((err = generate_play_local_sdp_for_audio(local_sdp, stream_desc, cname)) != srs_success) {
+            return srs_error_wrap(err, "audio");
+        }
+    }
+
+    return err;
+}
+
+srs_error_t SrsRtcConnection::generate_play_local_sdp_for_audio(SrsSdp& local_sdp, SrsRtcSourceDescription* stream_desc, std::string cname)
+{
+    srs_error_t err = srs_success;
+
     // generate audio media desc
     if (stream_desc->audio_track_desc_) {
         SrsRtcTrackDescription* audio_track = stream_desc->audio_track_desc_;
@@ -3476,6 +3545,13 @@ srs_error_t SrsRtcConnection::generate_play_local_sdp(SrsRequest* req, SrsSdp& l
         }
     }
 
+    return err;
+}
+
+srs_error_t SrsRtcConnection::generate_play_local_sdp_for_video(SrsSdp& local_sdp, SrsRtcSourceDescription* stream_desc, bool unified_plan, std::string cname)
+{
+    srs_error_t err = srs_success;
+
     for (int i = 0;  i < (int)stream_desc->video_track_descs_.size(); ++i) {
         SrsRtcTrackDescription* track = stream_desc->video_track_descs_[i];
 

diff --git a/trunk/src/app/srs_app_rtc_conn.hpp b/trunk/src/app/srs_app_rtc_conn.hpp
@@ -568,11 +568,15 @@ class SrsRtcConnection : public ISrsResource, public ISrsDisposingHandler, publi
     srs_error_t on_binding_request(SrsStunPacket* r);
     // publish media capabilitiy negotiate
     srs_error_t negotiate_publish_capability(SrsRtcUserConfig* ruc, SrsRtcSourceDescription* stream_desc);
-    srs_error_t generate_publish_local_sdp(SrsRequest* req, SrsSdp& local_sdp, SrsRtcSourceDescription* stream_desc, bool unified_plan);
+    srs_error_t generate_publish_local_sdp(SrsRequest* req, SrsSdp& local_sdp, SrsRtcSourceDescription* stream_desc, bool unified_plan, bool audio_before_video);
+    srs_error_t generate_publish_local_sdp_for_audio(SrsSdp& local_sdp, SrsRtcSourceDescription* stream_desc);
+    srs_error_t generate_publish_local_sdp_for_video(SrsSdp& local_sdp, SrsRtcSourceDescription* stream_desc, bool unified_plan);
     // play media capabilitiy negotiate
     //TODO: Use StreamDescription to negotiate and remove first negotiate_play_capability function
     srs_error_t negotiate_play_capability(SrsRtcUserConfig* ruc, std::map<uint32_t, SrsRtcTrackDescription*>& sub_relations);
-    srs_error_t generate_play_local_sdp(SrsRequest* req, SrsSdp& local_sdp, SrsRtcSourceDescription* stream_desc, bool unified_plan);
+    srs_error_t generate_play_local_sdp(SrsRequest* req, SrsSdp& local_sdp, SrsRtcSourceDescription* stream_desc, bool unified_plan, bool audio_before_video);
+    srs_error_t generate_play_local_sdp_for_audio(SrsSdp& local_sdp, SrsRtcSourceDescription* stream_desc, std::string cname);
+    srs_error_t generate_play_local_sdp_for_video(SrsSdp& local_sdp, SrsRtcSourceDescription* stream_desc, bool unified_plan, std::string cname);
     srs_error_t create_player(SrsRequest* request, std::map<uint32_t, SrsRtcTrackDescription*> sub_relations);
     srs_error_t create_publisher(SrsRequest* request, SrsRtcSourceDescription* stream_desc);
 };

diff --git a/trunk/src/app/srs_app_rtc_server.cpp b/trunk/src/app/srs_app_rtc_server.cpp
@@ -290,6 +290,7 @@ SrsRtcUserConfig::SrsRtcUserConfig()
     req_ = new SrsRequest();
     publish_ = false;
     dtls_ = srtp_ = true;
+    audio_before_video_ = false;
 }
 
 SrsRtcUserConfig::~SrsRtcUserConfig()

diff --git a/trunk/src/app/srs_app_rtc_server.hpp b/trunk/src/app/srs_app_rtc_server.hpp
@@ -90,6 +90,10 @@ class SrsRtcUserConfig
     bool publish_;
     bool dtls_;
     bool srtp_;
+
+    // The order of audio and video, or whether audio is before video. Please make sure the order is match for offer and
+    // answer, or client might fail at setRemoveDescription(answer). See https://github.com/ossrs/srs/issues/3179
+    bool audio_before_video_;
 public:
     SrsRtcUserConfig();
     virtual ~SrsRtcUserConfig();

diff --git a/trunk/src/core/srs_core_version4.hpp b/trunk/src/core/srs_core_version4.hpp
@@ -9,6 +9,6 @@
 
 #define VERSION_MAJOR       4
 #define VERSION_MINOR       0
-#define VERSION_REVISION    264
+#define VERSION_REVISION    265
 
 #endif