MEDIA-2252: fix removing neighbours (finos#353)

Author: reddvl1980

Diff for: api/EndpointDescription.h

@@ -15,7 +15,7 @@ struct EndpointDescription
     utils::Optional<api::Video> video;
     utils::Optional<api::Data> data;
 
-    std::vector<std::string> neighbours; // group(s) that should not be heard
+    utils::Optional<std::vector<std::string>> neighbours; // group(s) that should not be heard
 };
 
 } // namespace api

Diff for: api/Parser.cpp

@@ -422,10 +422,12 @@ EndpointDescription parsePatchEndpoint(const nlohmann::json& data, const std::st
 
     if (data.find("neighbours") != data.end())
     {
+        std::vector<std::string> neighbours;
         for (auto& group : requiredJsonArray(data["neighbours"], "groups"))
         {
-            endpointDescription.neighbours.push_back(group);
+            neighbours.push_back(group);
         }
+        endpointDescription.neighbours.set(neighbours);
     }
     return endpointDescription;
 }

Diff for: bridge/endpointActions/ConferenceActions.cpp

@@ -533,7 +533,12 @@ void configureAudioEndpoint(const api::EndpointDescription& endpointDescription,
         remoteSsrc.set(audio.ssrcs.front());
     }
 
-    auto neighbours = convertGroupIds(endpointDescription.neighbours);
+    std::vector<uint32_t> neighbours;
+    if (endpointDescription.neighbours.isSet())
+    {
+        neighbours = convertGroupIds(endpointDescription.neighbours.get());
+    }
+
     if (!mixer.configureAudioStream(endpointId, rtpMap, remoteSsrc, neighbours))
     {
         throw httpd::RequestErrorException(httpd::StatusCode::BAD_REQUEST,
@@ -836,7 +841,7 @@ httpd::Response reconfigureEndpoint(ActionContext* context,
 
     const bool isAudioSet = endpointDescription.audio.isSet();
     const bool isVideoSet = endpointDescription.video.isSet();
-    const bool isNeighboursSet = !endpointDescription.neighbours.empty();
+    const bool isNeighboursSet = endpointDescription.neighbours.isSet();
 
     if (isAudioSet && !mixer->isAudioStreamConfigured(endpointId))
     {
@@ -874,7 +879,7 @@ httpd::Response reconfigureEndpoint(ActionContext* context,
 
     if (isNeighboursSet)
     {
-        auto neighbours = convertGroupIds(endpointDescription.neighbours);
+        auto neighbours = convertGroupIds(endpointDescription.neighbours.get());
         if (!mixer->reconfigureAudioStreamNeighbours(endpointId, neighbours))
         {
             throw httpd::RequestErrorException(httpd::StatusCode::INTERNAL_SERVER_ERROR,

Diff for: bridge/engine/EngineMixerAudio.cpp

@@ -181,6 +181,7 @@ void EngineMixer::reconfigureNeighbours(const transport::RtcTransport& transport
     if (neighbourIt != _neighbourMemberships.end())
     {
         auto& neighbourList = neighbourIt->second.memberships;
+        neighbourList.clear();
         for (auto& it : engineAudioStream->neighbours)
         {
             neighbourList.push_back(it.first);

Diff for: test/integration/ConfIntegrationTest.cpp

@@ -723,7 +723,116 @@ TEST_F(IntegrationTest, neighbours)
     });
 }
 
-TEST_F(IntegrationTest, dynamicNeighbours)
+TEST_F(IntegrationTest, dynamicNeighbours_removeNeighbours)
+{
+    runTestInThread(expectedTestThreadCount(1), [this]() {
+        _config.readFromString(_defaultSmbConfig);
+
+        initBridge(_config);
+        const auto baseUrl = "http://127.0.0.1:8080";
+
+        GroupCall<SfuClient<Channel>> group(_httpd,
+            _instanceCounter,
+            *_mainPoolAllocator,
+            _audioAllocator,
+            *_clientTransportFactory,
+            *_publicTransportFactory,
+            *_sslDtls,
+            4);
+
+        Conference conf(_httpd);
+
+        ScopedFinalize finalize(std::bind(&IntegrationTest::finalizeSimulation, this));
+        startSimulation();
+
+        group.startConference(conf, baseUrl);
+
+        std::string neighbours[] = {"gid1"};
+
+        CallConfigBuilder cfgBuilder(conf.getId());
+        cfgBuilder.url(baseUrl).withOpus().withVideo();
+
+        CallConfigBuilder cfgNeighbours(cfgBuilder);
+        cfgNeighbours.neighbours(utils::Span<std::string>(neighbours));
+
+        group.clients[0]->initiateCall(cfgBuilder.build());
+        group.clients[1]->joinCall(cfgNeighbours.build());
+        group.clients[2]->joinCall(cfgNeighbours.build());
+        group.clients[3]->joinCall(cfgNeighbours.mixed().build());
+
+        // 600, 1300, 2100, 3200
+        ASSERT_TRUE(group.connectAll(utils::Time::sec * _clientsConnectionTimeout));
+
+        nlohmann::json responseBody;
+        const auto conferenceId = conf.getId();
+
+        for (int i = 1; i < 4; i++)
+        {
+            auto endpointId = group.clients[i]->getEndpointId();
+
+            nlohmann::json body = {{"action", "reconfigure"}};
+            body["neighbours"] = {{"groups", nlohmann::json::array()}};
+
+            auto neighboursSet = emulator::awaitResponse<HttpPutRequest>(_httpd,
+                std::string(baseUrl) + "/conferences/" + conferenceId + "/" + endpointId,
+                body.dump(),
+                1.5 * utils::Time::sec,
+                responseBody);
+            EXPECT_TRUE(neighboursSet);
+        }
+
+        make5secCallWithDefaultAudioProfile(group);
+
+        auto statsSuccess = emulator::awaitResponse<HttpGetRequest>(_httpd,
+            std::string(baseUrl) + "/stats",
+            1500 * utils::Time::ms,
+            responseBody);
+        EXPECT_TRUE(statsSuccess);
+
+        auto confRequest = emulator::awaitResponse<HttpGetRequest>(_httpd,
+            std::string(baseUrl) + "/conferences",
+            1500 * utils::Time::ms,
+            responseBody);
+        EXPECT_TRUE(confRequest);
+
+        group.stopTransports();
+
+        group.awaitPendingJobs(utils::Time::sec * 4);
+        finalizeSimulation();
+
+        const size_t chMixed[] = {0, 0, 0, 3};
+        AudioAnalysisData results[4];
+        for (size_t id = 0; id < 4; ++id)
+        {
+            results[id] = analyzeRecording<SfuClient<Channel>>(group.clients[id].get(), 5, true, chMixed[id]);
+
+            std::unordered_map<uint32_t, transport::ReportSummary> transportSummary;
+            std::string clientName = "client_" + std::to_string(id);
+            group.clients[id]->getReportSummary(transportSummary);
+            logTransportSummary(clientName.c_str(), transportSummary);
+
+            logVideoSent(clientName.c_str(), *group.clients[id]);
+            logVideoReceive(clientName.c_str(), *group.clients[id]);
+        }
+
+        EXPECT_EQ(results[0].audioSsrcCount, 3u);
+        EXPECT_EQ(results[1].audioSsrcCount, 3u);
+        EXPECT_EQ(results[2].audioSsrcCount, 3u);
+        EXPECT_EQ(results[3].audioSsrcCount, 1u);
+
+        EXPECT_EQ(results[0].dominantFrequencies.size(), 3);
+        EXPECT_EQ(results[1].dominantFrequencies.size(), 3);
+        EXPECT_EQ(results[2].dominantFrequencies.size(), 3);
+        EXPECT_EQ(results[3].dominantFrequencies.size(), 3);
+
+        if (results[3].dominantFrequencies.size() > 0)
+        {
+            EXPECT_NEAR(results[3].dominantFrequencies[0], 600.0, 50.0);
+        }
+    });
+}
+
+TEST_F(IntegrationTest, dynamicNeighbours_addNeighbours)
 {
     runTestInThread(expectedTestThreadCount(1), [this]() {
         _config.readFromString(_defaultSmbConfig);