From e04c3970994bd6c4cd1260ed144c6f1bc74de761 Mon Sep 17 00:00:00 2001
From: Philipp Hancke <phancke@microsoft.com>
Date: Wed, 21 Dec 2022 15:46:46 +0100
Subject: [PATCH] Enforce stream id uniqueness in RtpSender::set_stream_ids

https://w3c.github.io/webrtc-pc/#dfn-create-an-rtcrtpsender
has a step saying
  For each stream in streams, add stream.id to
  [[AssociatedMediaStreamIds]] if it's not already there

This applies to addTrack and setStreams and the set of streams in
addTransceiver.

Tests that default to the stream id as sync group add
"-sync" as a postfix

BUG=webrtc:14769

Change-Id: I806d2fd87a98d50e54709755541f3f1efff1d8ab
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/288701
Commit-Queue: Philipp Hancke <phancke@microsoft.com>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38942}
---
 pc/rtp_sender.cc                   | 8 ++++++++
 pc/rtp_sender.h                    | 6 +++---
 pc/rtp_sender_receiver_unittest.cc | 9 +++++++++
 test/pc/e2e/media/media_helper.cc  | 4 ++--
 4 files changed, 22 insertions(+), 5 deletions(-)

diff --git a/pc/rtp_sender.cc b/pc/rtp_sender.cc
index 1bbca3c41a..fe3b3b1aa5 100644
--- a/pc/rtp_sender.cc
+++ b/pc/rtp_sender.cc
@@ -383,6 +383,14 @@ void RtpSenderBase::SetParametersAsync(const RtpParameters& parameters,
       false);
 }
 
+void RtpSenderBase::set_stream_ids(const std::vector<std::string>& stream_ids) {
+  stream_ids_.clear();
+  absl::c_copy_if(stream_ids, std::back_inserter(stream_ids_),
+                  [this](const std::string& stream_id) {
+                    return !absl::c_linear_search(stream_ids_, stream_id);
+                  });
+}
+
 void RtpSenderBase::SetStreams(const std::vector<std::string>& stream_ids) {
   set_stream_ids(stream_ids);
   if (set_streams_observer_)
diff --git a/pc/rtp_sender.h b/pc/rtp_sender.h
index 29e5f16cfe..fdeedd5e5a 100644
--- a/pc/rtp_sender.h
+++ b/pc/rtp_sender.h
@@ -164,9 +164,9 @@ class RtpSenderBase : public RtpSenderInternal, public ObserverInterface {
     RTC_DCHECK_RUN_ON(signaling_thread_);
     return stream_ids_;
   }
-  void set_stream_ids(const std::vector<std::string>& stream_ids) override {
-    stream_ids_ = stream_ids;
-  }
+
+  // Set stream ids, eliminating duplicates in the process.
+  void set_stream_ids(const std::vector<std::string>& stream_ids) override;
   void SetStreams(const std::vector<std::string>& stream_ids) override;
 
   std::string id() const override { return id_; }
diff --git a/pc/rtp_sender_receiver_unittest.cc b/pc/rtp_sender_receiver_unittest.cc
index e51b058210..04043437a7 100644
--- a/pc/rtp_sender_receiver_unittest.cc
+++ b/pc/rtp_sender_receiver_unittest.cc
@@ -1888,6 +1888,15 @@ TEST_F(RtpSenderReceiverTest,
   EXPECT_TRUE(video_rtp_sender_->SetParameters(parameters).ok());
 }
 
+// Checks that the senders SetStreams eliminates duplicate stream ids.
+TEST_F(RtpSenderReceiverTest, SenderSetStreamsEliminatesDuplicateIds) {
+  AddVideoTrack();
+  video_rtp_sender_ =
+      VideoRtpSender::Create(worker_thread_, video_track_->id(), nullptr);
+  video_rtp_sender_->SetStreams({"1", "2", "1"});
+  EXPECT_EQ(video_rtp_sender_->stream_ids().size(), 2u);
+}
+
 // Helper method for syntactic sugar for accepting a vector with '{}' notation.
 std::pair<RidList, RidList> CreatePairOfRidVectors(
     const std::vector<std::string>& first,
diff --git a/test/pc/e2e/media/media_helper.cc b/test/pc/e2e/media/media_helper.cc
index 3f6d069429..e945bd4dae 100644
--- a/test/pc/e2e/media/media_helper.cc
+++ b/test/pc/e2e/media/media_helper.cc
@@ -36,7 +36,7 @@ void MediaHelper::MaybeAddAudio(TestPeer* peer) {
                                            source.get());
   std::string sync_group = audio_config.sync_group
                                ? audio_config.sync_group.value()
-                               : audio_config.stream_label.value();
+                               : audio_config.stream_label.value() + "-sync";
   peer->AddTrack(track, {sync_group, *audio_config.stream_label});
 }
 
@@ -71,7 +71,7 @@ MediaHelper::MaybeAddVideo(TestPeer* peer) {
     }
     std::string sync_group = video_config.sync_group
                                  ? video_config.sync_group.value()
-                                 : video_config.stream_label.value();
+                                 : video_config.stream_label.value() + "-sync";
     RTCErrorOr<rtc::scoped_refptr<RtpSenderInterface>> sender =
         peer->AddTrack(track, {sync_group, *video_config.stream_label});
     RTC_CHECK(sender.ok());