Improve stability (#142)

* Add logs. Fix signaling-server. * Add callAll * Prevent creating a new peer connection after the desctuctor is called. * Use bool and callSync instead of atomic_bool for m_destructorCalled. * Add client connectin failed callback. Set ICE timeout. * Add onClientConnectionFail to the JS API. * Remove invalid changes. * Fix * Add print to debug the deadlock. * Add print to debug the deadlock. * Add print to debug the deadlock. * Add debug print in StreamClientPython.cpp. * Update pybind11. * Remove debug prints. * Update cpp-python-tests.yml * Update VERSION --------- Co-authored-by: Dominic Létourneau <[email protected]>
introlab · Jul 10, 2024 · b6a53b2 · b6a53b2
1 parent bef7e2d
commit b6a53b2
Show file tree

Hide file tree

Showing 23 changed files with 229 additions and 30 deletions.
diff --git a/.github/workflows/cpp-python-tests.yml b/.github/workflows/cpp-python-tests.yml
@@ -20,7 +20,7 @@ jobs:
     strategy:
       fail-fast: false
       matrix:
-        os: [ubuntu-20.04, ubuntu-22.04, macos-11, macos-12]
+        os: [ubuntu-20.04, ubuntu-22.04, macos-12]
         python-version: ["3.8", "3.9", "3.10"]
         build-type: [Debug, Release]
         enable-gstreamer: [ON, OFF]
@@ -180,7 +180,7 @@ jobs:
         working-directory: opentera-webrtc-native-client/OpenteraWebrtcNativeClient/python/test
         run: |
           if [ -f requirements.txt ]; then python -m pip install --user -r requirements.txt; fi
-          python -m pytest -v
+          python -m pytest -vv -s
 
       # With stdout
       - name: Run Python signaling-server Tests with STDOUT

diff --git a/VERSION b/VERSION
@@ -1 +1 @@
-1.1.4
+1.2.0
diff --git a/examples/cpp-data-channel-client/main.cpp b/examples/cpp-data-channel-client/main.cpp
@@ -36,7 +36,7 @@ int main(int argc, char* argv[])
         [](const string& error)
         {
             // This callback is called from the internal client thread.
-            cout << "OnSignalingConnectionClosed:" << endl << "\t" << error;
+            cout << "OnSignalingConnectionError:" << endl << "\t" << error;
         });
 
     client.setOnRoomClientsChanged(
@@ -64,6 +64,13 @@ int main(int argc, char* argv[])
             cout << "OnClientDisconnected:" << endl;
             cout << "\tid=" << client.id() << ", name=" << client.name() << endl;
         });
+    client.setOnClientConnectionFailed(
+        [](const Client& client)
+        {
+            // This callback is called from the internal client thread.
+            cout << "OnClientConnectionFailed:" << endl;
+            cout << "\tid=" << client.id() << ", name=" << client.name() << endl;
+        });
 
     client.setOnError(
         [](const string& error)
@@ -73,6 +80,14 @@ int main(int argc, char* argv[])
             cout << "\t" << error << endl;
         });
 
+    client.setLogger(
+        [](const string& message)
+        {
+            // This callback is called from the internal client thread.
+            cout << "log:" << endl;
+            cout << "\t" << message << endl;
+        });
+
     client.setOnDataChannelOpened(
         [](const Client& client)
         {

diff --git a/examples/cpp-stream-client/main.cpp b/examples/cpp-stream-client/main.cpp
@@ -109,7 +109,7 @@ class SinAudioSource : public AudioSource
         for (size_t i = 0; i < data.size(); i++)
         {
             data[i] = static_cast<int16_t>(SinAudioSourceAmplitude * sin(t));
-            t += 2 * M_PI / data.size();
+            t += 2 * M_PI / static_cast<double>(data.size());
         }
 
         while (!m_stopped.load())
@@ -118,8 +118,7 @@ class SinAudioSource : public AudioSource
 
             auto start = chrono::steady_clock::now();
             this_thread::sleep_for(SinAudioSourceFrameDuration - SinAudioSourceSleepBuffer);
-            while ((chrono::steady_clock::now() - start) < SinAudioSourceFrameDuration)
-                ;
+            while ((chrono::steady_clock::now() - start) < SinAudioSourceFrameDuration);
         }
     }
 };
@@ -164,7 +163,7 @@ int main(int argc, char* argv[])
         [](const string& error)
         {
             // This callback is called from the internal client thread.
-            cout << "OnSignalingConnectionClosed:" << endl << "\t" << error;
+            cout << "OnSignalingConnectionError:" << endl << "\t" << error;
         });
 
     client.setOnRoomClientsChanged(
@@ -194,6 +193,13 @@ int main(int argc, char* argv[])
             cout << "\tid=" << client.id() << ", name=" << client.name() << endl;
             cv::destroyWindow(client.id());
         });
+    client.setOnClientConnectionFailed(
+        [](const Client& client)
+        {
+            // This callback is called from the internal client thread.
+            cout << "OnClientConnectionFailed:" << endl;
+            cout << "\tid=" << client.id() << ", name=" << client.name() << endl;
+        });
 
     client.setOnError(
         [](const string& error)
@@ -203,6 +209,14 @@ int main(int argc, char* argv[])
             cout << "\t" << error << endl;
         });
 
+    client.setLogger(
+        [](const string& message)
+        {
+            // This callback is called from the internal client thread.
+            cout << "log:" << endl;
+            cout << "\t" << message << endl;
+        });
+
     client.setOnAddRemoteStream(
         [](const Client& client)
         {
@@ -221,7 +235,7 @@ int main(int argc, char* argv[])
         [](const Client& client, const cv::Mat& bgrImg, uint64_t timestampUs)
         {
             // This callback is called from a WebRTC processing thread.
-            // cout << "OnVideoFrameReceived:" << endl;
+            cout << "OnVideoFrameReceived:" << endl;
             cv::imshow(client.id(), bgrImg);
             cv::waitKey(1);
         });

diff --git a/examples/python-data-channel-client/python_data_channel_client.py b/examples/python-data-channel-client/python_data_channel_client.py
@@ -42,6 +42,13 @@ def on_client_disconnected(client):
         client.id, client.name, client.data))
 
 
+def on_client_connection_failed(client):
+    # This callback is called from the internal client thread.
+    print('on_client_connection_failed:')
+    print('\tid={}, name={}, data={}\n'.format(
+        client.id, client.name, client.data))
+
+
 def on_error(error):
     # This callback is called from the internal client thread.
     print('error:')
@@ -95,6 +102,7 @@ def on_data_channel_message_string(client, message):
 
     client.on_client_connected = on_client_connected
     client.on_client_disconnected = on_client_disconnected
+    client.on_client_connection_failed = on_client_connection_failed
 
     client.on_error = on_error
 

diff --git a/examples/python-stream-client/python_stream_client.py b/examples/python-stream-client/python_stream_client.py
@@ -46,6 +46,13 @@ def on_client_disconnected(client):
         client.id, client.name, client.data))
 
 
+def on_client_connection_failed(client):
+    # This callback is called from the internal client thread.
+    print('on_client_connection_failed:')
+    print('\tid={}, name={}, data={}\n'.format(
+        client.id, client.name, client.data))
+
+
 def on_add_remote_stream(client):
     # This callback is called from the internal client thread.
     print('on_add_remote_stream:')
@@ -116,6 +123,7 @@ def on_error(error):
 
     client.on_client_connected = on_client_connected
     client.on_client_disconnected = on_client_disconnected
+    client.on_client_connection_failed = on_client_connection_failed
 
     client.on_add_remote_stream = on_add_remote_stream
     client.on_remove_remote_stream = on_remove_remote_stream

diff --git a/examples/web-data-channel-client/client.js b/examples/web-data-channel-client/client.js
@@ -60,6 +60,10 @@
       alert('The call is rejected (' + name + ')');
     };
 
+    dataChannelClient.onClientConnectionFail = (id, name, clientData) => {
+      console.log('The connect with the client ' + name + '(' + id + ') failed.');
+    }
+
     dataChannelClient.onDataChannelOpen = (id, name, clientData) => {
       sendButton.disabled = false;
       callAllButton.disabled = true;

diff --git a/examples/web-stream-client/client.js b/examples/web-stream-client/client.js
@@ -62,6 +62,10 @@
       });
     };
 
+    streamClient.onClientConnectionFail = (id, name, clientData) => {
+      console.log('The connect with the client ' + name + '(' + id + ') failed.');
+    }
+
     streamClient.onAddRemoteStream = (id, name, clientData, stream) => {
       callAllButton.disabled = true;
       hangUpAllButton.disabled = false;

diff --git a/examples/web-stream-data-channel-client/client.js b/examples/web-stream-data-channel-client/client.js
@@ -66,6 +66,10 @@
       });
     };
 
+    streamDataChannelClient.onClientConnectionFail = (id, name, clientData) => {
+      console.log('The connect with the client ' + name + '(' + id + ') failed.');
+    }
+
     streamDataChannelClient.onAddRemoteStream = (id, name, clientData, stream) => {
       sendButton.disabled = false;
       callAllButton.disabled = true;

diff --git a/opentera-webrtc-native-client/3rdParty/pybind11 b/opentera-webrtc-native-client/3rdParty/pybind11
diff --git a/...tiveClient/include/OpenteraWebrtcNativeClient/Handlers/DataChannelPeerConnectionHandler.h b/...tiveClient/include/OpenteraWebrtcNativeClient/Handlers/DataChannelPeerConnectionHandler.h
@@ -33,6 +33,7 @@ namespace opentera
             std::function<void(const std::string&)> onError,
             std::function<void(const Client&)> onClientConnected,
             std::function<void(const Client&)> onClientDisconnected,
+            std::function<void(const Client&)> onClientConnectionFailed,
             DataChannelConfiguration dataChannelConfiguration,
             std::function<void(const Client&)> onDataChannelOpen,
             std::function<void(const Client&)> onDataChannelClosed,

diff --git a/...eraWebrtcNativeClient/include/OpenteraWebrtcNativeClient/Handlers/PeerConnectionHandler.h b/...eraWebrtcNativeClient/include/OpenteraWebrtcNativeClient/Handlers/PeerConnectionHandler.h
@@ -42,6 +42,7 @@ namespace opentera
         std::function<void(const std::string&)> m_onError;
         std::function<void(const Client&)> m_onClientConnected;
         std::function<void(const Client&)> m_onClientDisconnected;
+        std::function<void(const Client&)> m_onClientConnectionFailed;
 
         rtc::scoped_refptr<webrtc::PeerConnectionInterface> m_peerConnection;
 
@@ -55,7 +56,8 @@ namespace opentera
             SignalingClient& m_signalingClient,
             std::function<void(const std::string&)>&& onError,
             std::function<void(const Client&)>&& onClientConnected,
-            std::function<void(const Client&)>&& onClientDisconnected);
+            std::function<void(const Client&)>&& onClientDisconnected,
+            std::function<void(const Client&)>&& onClientConnectionFailed);
         ~PeerConnectionHandler() override;
 
         virtual void setPeerConnection(const rtc::scoped_refptr<webrtc::PeerConnectionInterface>& peerConnection);

diff --git a/...rtcNativeClient/include/OpenteraWebrtcNativeClient/Handlers/StreamPeerConnectionHandler.h b/...rtcNativeClient/include/OpenteraWebrtcNativeClient/Handlers/StreamPeerConnectionHandler.h
@@ -55,6 +55,7 @@ namespace opentera
             std::function<void(const std::string&)> onError,
             std::function<void(const Client&)> onClientConnected,
             std::function<void(const Client&)> onClientDisconnected,
+            std::function<void(const Client&)> onClientConnectionFailed,
             rtc::scoped_refptr<webrtc::VideoTrackInterface> videoTrack,
             rtc::scoped_refptr<webrtc::AudioTrackInterface> audioTrack,
             std::function<void(const Client&)> onAddRemoteStream,

diff --git a/...ative-client/OpenteraWebrtcNativeClient/include/OpenteraWebrtcNativeClient/WebrtcClient.h b/...ative-client/OpenteraWebrtcNativeClient/include/OpenteraWebrtcNativeClient/WebrtcClient.h
@@ -45,6 +45,7 @@ namespace opentera
 
         std::function<void(const Client&)> m_onClientConnected;
         std::function<void(const Client&)> m_onClientDisconnected;
+        std::function<void(const Client&)> m_onClientConnectionFailed;
 
         std::function<void(const std::string& error)> m_onError;
 
@@ -54,6 +55,8 @@ namespace opentera
         std::unique_ptr<rtc::Thread> m_workerThread;
         std::unique_ptr<rtc::Thread> m_signalingThread;
 
+        bool m_destructorCalled;
+
     protected:
         std::unique_ptr<SignalingClient> m_signalingClient;
 
@@ -105,6 +108,7 @@ namespace opentera
 
         void setOnClientConnected(const std::function<void(const Client&)>& callback);
         void setOnClientDisconnected(const std::function<void(const Client&)>& callback);
+        void setOnClientConnectionFailed(const std::function<void(const Client&)>& callback);
 
         void setOnError(const std::function<void(const std::string& error)>& callback);
 
@@ -122,6 +126,7 @@ namespace opentera
         std::function<void(const std::string&)> getOnErrorFunction();
         std::function<void(const Client&)> getOnClientConnectedFunction();
         std::function<void(const Client&)> getOnClientDisconnectedFunction();
+        std::function<void(const Client&)> getOnClientConnectionFailedFunction();
 
         rtc::Thread* getInternalClientThread();
 
@@ -332,6 +337,23 @@ namespace opentera
         callSync(m_internalClientThread.get(), [this, &callback]() { m_onClientDisconnected = callback; });
     }
 
+    /**
+     * @brief Sets the callback that is called when a client peer connection fails.
+     *
+     * The callback is called from the internal client thread. The callback should not block.
+     *
+     * @parblock
+     * Callback parameters:
+     *  - client: The client that has a connection failure
+     * @endparblock
+     *
+     * @param callback The callback
+     */
+    inline void WebrtcClient::setOnClientConnectionFailed(const std::function<void(const Client&)>& callback)
+    {
+        callSync(m_internalClientThread.get(), [this, &callback]() { m_onClientConnectionFailed = callback; });
+    }
+
     /**
      * @brief Sets the callback that is called when an error occurs.
      *

diff --git a/opentera-webrtc-native-client/OpenteraWebrtcNativeClient/python/src/WebrtcClientPython.cpp b/opentera-webrtc-native-client/OpenteraWebrtcNativeClient/python/src/WebrtcClientPython.cpp
@@ -186,6 +186,20 @@ void opentera::initWebrtcClientPython(pybind11::module& m)
             " - client: The client that is disconnected\n"
             "\n"
             ":param callback: The callback")
+        .def_property(
+            "on_client_connection_failed",
+            nullptr,
+            GilScopedRelease<WebrtcClient>::guard(&WebrtcClient::setOnClientConnectionFailed),
+            "Sets the callback that is called when a client peer "
+            "connection fails.\n"
+            "\n"
+            "The callback is called from the internal client thread. "
+            "The callback should not block.\n"
+            "\n"
+            "Callback parameters:\n"
+            " - client: The client that has a connection failure\n"
+            "\n"
+            ":param callback: The callback")
 
         .def_property(
             "on_error",

diff --git a/opentera-webrtc-native-client/OpenteraWebrtcNativeClient/src/DataChannelClient.cpp b/opentera-webrtc-native-client/OpenteraWebrtcNativeClient/src/DataChannelClient.cpp
@@ -87,6 +87,7 @@ unique_ptr<PeerConnectionHandler>
         getOnErrorFunction(),
         getOnClientConnectedFunction(),
         getOnClientDisconnectedFunction(),
+        getOnClientConnectionFailedFunction(),
         m_dataChannelConfiguration,
         onDataChannelOpen,
         onDataChannelClosed,

diff --git a/...ative-client/OpenteraWebrtcNativeClient/src/Handlers/DataChannelPeerConnectionHandler.cpp b/...ative-client/OpenteraWebrtcNativeClient/src/Handlers/DataChannelPeerConnectionHandler.cpp
@@ -11,6 +11,7 @@ DataChannelPeerConnectionHandler::DataChannelPeerConnectionHandler(
     function<void(const string&)> onError,
     function<void(const Client&)> onClientConnected,
     function<void(const Client&)> onClientDisconnected,
+    function<void(const Client&)> onClientConnectionFailed,
     DataChannelConfiguration dataChannelConfiguration,
     function<void(const Client&)> onDataChannelOpen,
     function<void(const Client&)> onDataChannelClosed,
@@ -24,7 +25,8 @@ DataChannelPeerConnectionHandler::DataChannelPeerConnectionHandler(
           m_signalingClient,
           move(onError),
           move(onClientConnected),
-          move(onClientDisconnected)),
+          move(onClientDisconnected),
+          move(onClientConnectionFailed)),
       m_dataChannelConfiguration(move(dataChannelConfiguration)),
       m_onDataChannelOpen(move(onDataChannelOpen)),
       m_onDataChannelClosed(move(onDataChannelClosed)),

diff --git a/...ra-webrtc-native-client/OpenteraWebrtcNativeClient/src/Handlers/PeerConnectionHandler.cpp b/...ra-webrtc-native-client/OpenteraWebrtcNativeClient/src/Handlers/PeerConnectionHandler.cpp
@@ -49,14 +49,16 @@ PeerConnectionHandler::PeerConnectionHandler(
     SignalingClient& m_signalingClient,
     function<void(const string&)>&& onError,
     function<void(const Client&)>&& onClientConnected,
-    function<void(const Client&)>&& onClientDisconnected)
+    function<void(const Client&)>&& onClientDisconnected,
+    function<void(const Client&)>&& onClientConnectionFailed)
     : m_id(move(id)),
       m_peerClient(move(peerClient)),
       m_isCaller(isCaller),
       m_signalingClient(m_signalingClient),
       m_onError(move(onError)),
       m_onClientConnected(move(onClientConnected)),
       m_onClientDisconnected(move(onClientDisconnected)),
+      m_onClientConnectionFailed(move(onClientConnectionFailed)),
       m_onClientDisconnectedCalled(true)
 {
 }
@@ -139,14 +141,14 @@ void PeerConnectionHandler::OnConnectionChange(webrtc::PeerConnectionInterface::
             m_onClientConnected(m_peerClient);
             m_onClientDisconnectedCalled = false;
             break;
-
-        case webrtc::PeerConnectionInterface::PeerConnectionState::kDisconnected:
         case webrtc::PeerConnectionInterface::PeerConnectionState::kFailed:
+            m_onClientConnectionFailed(m_peerClient);
+            break;
+        case webrtc::PeerConnectionInterface::PeerConnectionState::kDisconnected:
         case webrtc::PeerConnectionInterface::PeerConnectionState::kClosed:
             m_onClientDisconnected(m_peerClient);
             m_onClientDisconnectedCalled = true;
             break;
-
         default:
             break;
     }

diff --git a/...rtc-native-client/OpenteraWebrtcNativeClient/src/Handlers/StreamPeerConnectionHandler.cpp b/...rtc-native-client/OpenteraWebrtcNativeClient/src/Handlers/StreamPeerConnectionHandler.cpp
@@ -18,6 +18,7 @@ StreamPeerConnectionHandler::StreamPeerConnectionHandler(
     function<void(const string&)> onError,
     function<void(const Client&)> onClientConnected,
     function<void(const Client&)> onClientDisconnected,
+    function<void(const Client&)> onClientConnectionFailed,
     webrtc::scoped_refptr<VideoTrackInterface> videoTrack,
     webrtc::scoped_refptr<AudioTrackInterface> audioTrack,
     function<void(const Client&)> onAddRemoteStream,
@@ -32,7 +33,8 @@ StreamPeerConnectionHandler::StreamPeerConnectionHandler(
           m_signalingClient,
           move(onError),
           move(onClientConnected),
-          move(onClientDisconnected)),
+          move(onClientDisconnected),
+          move(onClientConnectionFailed)),
       m_offerToReceiveAudio(hasOnMixedAudioFrameReceivedCallback || onAudioFrameReceived),
       m_offerToReceiveVideo(static_cast<bool>(onVideoFrameReceived)),
       m_videoTrack(move(videoTrack)),