M120 #173

jmillan · 2024-02-13T15:20:11Z

No description provided.

ibc

wow

src/Handler.cpp

jmillan · 2024-02-13T15:23:56Z

@fedulvtubudul,

Would you be so kind to test this branch out within your app?

I'm still fighting with the inability to run the tests, nor the demo we have (which relies on the libwebrtc test code).

If you can confirm this is working we would release it.

Thanks.

src/sdp/Utils.cpp

jmillan · 2024-02-13T17:08:59Z

Asking for help in order to make m120 work.

This is the error I'm getting when enabling tests (-DMEDIASOUPCLIENT_BUILD_TESTS:BOOLEAN=ON):

For some reason the resulting libwebrtc.a against which libmediasoupclient links does not contain the CreateBuiltinVideoDecoderFactory symbol nor the analogous encoder one.

The gn args I'm using are:
gn gen out/m120 --args='is_debug=true is_component_build=false is_clang=true rtc_include_tests=true rtc_use_h264=true use_rtti=true use_custom_libcxx=false'

I've dived into the code and theoretically they should be present if rtc_include_tests is true, which it is.

A hand here would be appreciated. CC-ing those who commented on the m112 upgrade PR:
@copiltembel @Poldraunic @mikepulaski @fedulvtubudul

Poldraunic · 2024-02-13T17:36:02Z

@jmillan, sure! I am happy to help!

One question though: is this issue present on all OSes or still specific to MacOS?

jmillan · 2024-02-13T17:44:28Z

@jmillan, sure! I am happy to help!

One question though: is this issue present on all OSes or still specific to MacOS?

Thanks! I'm experiencing it myself in MacOS concretely. Not sure if it will repro in other OS.

Poldraunic · 2024-02-13T18:25:41Z

@jmillan, alright. I'll check it tomorrow on Windows and MacOS both and report back.

copiltembel · 2024-02-13T20:32:26Z

I want to help! Will try to have a look in the weekend, can't promise anything though!

copiltembel · 2024-02-18T12:05:57Z

I can't even seem to be able to build libwebrtc m120 on Ubuntu 22.04:

../../modules/audio_processing/agc2/adaptive_digital_gain_controller_unittest.cc:107:41: error: no member named 'log10f' in namespace 'std'; did you mean simply 'log10f'?
  107 |   const float applied_gain_db = 20.0f * std::log10f(applied_gain);
      |                                         ^~~~~~~~~~~
      |                                         log10f
../../build/linux/debian_bullseye_amd64-sysroot/usr/include/x86_64-linux-gnu/bits/mathcalls.h:107:1: note: 'log10f' declared here
  107 | __MATHCALL (log10,, (_Mdouble_ __x));
      | ^
../../build/linux/debian_bullseye_amd64-sysroot/usr/include/math.h:273:3: note: expanded from macro '__MATHCALL'
  273 |   __MATHDECL (_Mdouble_,function,suffix, args)
      |   ^
../../build/linux/debian_bullseye_amd64-sysroot/usr/include/math.h:275:3: note: expanded from macro '__MATHDECL'
  275 |   __MATHDECL_1(type, function,suffix, args); \
      |   ^
../../build/linux/debian_bullseye_amd64-sysroot/usr/include/math.h:283:15: note: expanded from macro '__MATHDECL_1'
  283 |   extern type __MATH_PRECNAME(function,suffix) args __THROW
      |               ^
../../build/linux/debian_bullseye_amd64-sysroot/usr/include/math.h:303:34: note: expanded from macro '__MATH_PRECNAME'
  303 | # define __MATH_PRECNAME(name,r) name##f##r
      |                                  ^
<scratch space>:211:1: note: expanded from here
  211 | log10f
      | ^
1 error generated.
[650/7524] CXX obj/modules/audio_processing/agc2/fixed_digital_unittests/limiter_unittest.o
ninja: build stopped: subcommand failed.

😕

ibc · 2024-02-18T12:13:28Z

std::log10f(applied_gain)

According to docs:

log10f is included in math.h
std::log10f is included in cmath.h

Include mismatch?

fedulvtubudul · 2024-02-18T15:09:53Z

I might be on the right track though, what do you think?

@copiltembel your idea looks valid considering these comments in BUILD.gn

# Heavy but optional helper for unittests and webrtc users who prefer to use
# defaults factories or do not worry about extra dependencies and binary size.
rtc_library("rtc_media_engine_defaults") {
...

copiltembel · 2024-02-18T15:57:19Z

OK, so my previous comment was not correct, the missing symbols are in differents libs:

Linking test_mediasoupclient with

	${LIBWEBRTC_BINARY_PATH}/api/video_codecs/libbuiltin_video_encoder_factory${CMAKE_STATIC_LIBRARY_SUFFIX}
	${LIBWEBRTC_BINARY_PATH}/api/video_codecs/libbuiltin_video_decoder_factory${CMAKE_STATIC_LIBRARY_SUFFIX}

stops it from complaining about the missing enc and dec factories.

fedulvtubudul · 2024-02-19T04:55:52Z

Next missing symbols

webrtc::SimulcastEncoderAdapter::SimulcastEncoderAdapter(webrtc::VideoEncoderFactory*, webrtc::SdpVideoFormat const&)
webrtc::InternalDecoderFactory
webrtc::InternalEncoderFactory

are fixed by linking this libs:

${LIBWEBRTC_BINARY_PATH}/media/librtc_simulcast_encoder_adapter${CMAKE_STATIC_LIBRARY_SUFFIX}
${LIBWEBRTC_BINARY_PATH}/media/librtc_internal_video_codecs${CMAKE_STATIC_LIBRARY_SUFFIX}

And this one is also missing:

webrtc::webrtc_sequence_checker_internal::SequenceCheckerImpl::ExpectationToString()

but it is not even compiled in normal (release) WebRTC builds. On source-code level it is implemented under preprocessor condition #if RTC_DCHECK_IS_ON. To make it available we should set dcheck_always_on=true flag when building WebRTC for mediasoup tests.

After that, tests are built without errors (on my machine, as we used to say 🤣). However, they are not green:



#
# Fatal error in: ../../../../Mediasoup/dependencies/webrtc/src/call/call.cc, line 719
# last system error: 0
# Check failed: (worker_thread_)->IsCurrent()
#
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
test_mediasoupclient is a Catch v2.13.10 host application.
Run with -? for options

-------------------------------------------------------------------------------
SendHandler
-------------------------------------------------------------------------------
/Users/neznajka/vl/projects/mediasoup-client-swift/Mediasoup/dependencies/libmediasoupclient/test/src/Handler.test.cpp:38
...............................................................................

/Users/neznajka/vl/projects/mediasoup-client-swift/Mediasoup/dependencies/libmediasoupclient/test/src/Handler.test.cpp:38: FAILED:
due to a fatal error condition:
  SIGABRT - Abort (abnormal termination) signal

===============================================================================
test cases:  3 |  2 passed | 1 failed
assertions: 20 | 19 passed | 1 failed

zsh: abort      ./test_mediasoupclient.app/Contents/MacOS/test_mediasoupclient

copiltembel · 2024-02-19T09:57:25Z

(On Ubuntu 22.04)

Re libwebrtc m120, I'm a bit lost because:

I cannot build with gcc (i.e. is_clang=false)
When building with clang, I cannot link against system C++ library for C++ standard library support (i.e. use_custom_libcxx=false)

This is the only thing that works:

$ gn gen out/clang/m120 --args='is_clang=true rtc_include_tests=true is_component_build=false use_rtti=true is_debug=true use_custom_libcxx=true treat_warnings_as_errors=false'

But then when linking it against test_mediasoupclient, I get alot of errors like these:

[100%] Linking CXX executable test_mediasoupclient
/usr/bin/ld: /mnt/large4/src/google/libwebrtc/m120_webrtc/src/out/clang/m120/obj/libwebrtc.a(generated_message_util.o): in function `google::protobuf::internal::DestroyString(void const*)':
./../../../third_party/protobuf/src/google/protobuf/generated_message_util.cc:64: undefined reference to `std::__Cr::basic_string<char, std::__Cr::char_traits<char>, std::__Cr::allocator<char> >::~basic_string()'
/usr/bin/ld: CMakeFiles/test_mediasoupclient.dir/src/Device.test.cpp.o: in function `std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > cricket::ToStringIfSet<bool>(char const*, std::optional<bool> const&)':
Device.test.cpp:(.text+0x3169): undefined reference to `rtc::ToString[abi:cxx11](bool)'
/usr/bin/ld: CMakeFiles/test_mediasoupclient.dir/src/Device.test.cpp.o: in function `std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > cricket::ToStringIfSet<int>(char const*, std::optional<int> const&)':
Device.test.cpp:(.text+0x32f2): undefined reference to `rtc::ToString[abi:cxx11](int)'
/usr/bin/ld: CMakeFiles/test_mediasoupclient.dir/src/Device.test.cpp.o: in function `std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > cricket::VectorToString<cricket::Codec>(std::vector<cricket::Codec, std::allocator<cricket::Codec> > const&)':
Device.test.cpp:(.text+0x34a6): undefined reference to `cricket::Codec::ToString[abi:cxx11]() const'
/usr/bin/ld: CMakeFiles/test_mediasoupclient.dir/src/Device.test.cpp.o: in function `std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > cricket::VectorToString<webrtc::RtpExtension>(std::vector<webrtc::RtpExtension, std::allocator<webrtc::RtpExtension> > const&)':
Device.test.cpp:(.text+0x366b): undefined reference to `webrtc::RtpExtension::ToString[abi:cxx11]() const'
/usr/bin/ld: CMakeFiles/test_mediasoupclient.dir/src/Handler.test.cpp.o: in function `std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > cricket::ToStringIfSet<bool>(char const*, std::optional<bool> const&)':
Handler.test.cpp:(.text+0x52b3): undefined reference to `rtc::ToString[abi:cxx11](bool)'
/usr/bin/ld: CMakeFiles/test_mediasoupclient.dir/src/Handler.test.cpp.o: in function `std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > cricket::ToStringIfSet<int>(char const*, std::optional<int> const&)':
Handler.test.cpp:(.text+0x543c): undefined reference to `rtc::ToString[abi:cxx11](int)'
/usr/bin/ld: CMakeFiles/test_mediasoupclient.dir/src/Handler.test.cpp.o: in function `std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > cricket::VectorToString<cricket::Codec>(std::vector<cricket::Codec, std::allocator<cricket::Codec> > const&)':
Handler.test.cpp:(.text+0x55f0): undefined reference to `cricket::Codec::ToString[abi:cxx11]() const'
/usr/bin/ld: CMakeFiles/test_mediasoupclient.dir/src/Handler.test.cpp.o: in function `std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > cricket::VectorToString<webrtc::RtpExtension>(std::vector<webrtc::RtpExtension, std::allocator<webrtc::RtpExtension> > const&)':
Handler.test.cpp:(.text+0x57b5): undefined reference to `webrtc::RtpExtension::ToString[abi:cxx11]() const'
/usr/bin/ld: CMakeFiles/test_mediasoupclient.dir/src/PeerConnection.test.cpp.o: in function `std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > cricket::ToStringIfSet<bool>(char const*, std::optional<bool> const&)':
PeerConnection.test.cpp:(.text+0x1c41): undefined reference to `rtc::ToString[abi:cxx11](bool)'
(truncated for brevity)

fedulvtubudul · 2024-02-19T12:08:17Z

@copiltembel this part abi:cxx11 looks suspicious in your logs. We should use same language version for building all components. Both WebRTC and libmediasoupclient switched C++ 17 for current versions.

copiltembel · 2024-02-19T12:41:35Z

Got it, but it's not like I'm doing it on purpose. I'm just checking out libwebrtc and libmediasoupclient and trying to build them. Not changing anything.

jmillan · 2024-02-21T16:15:52Z

I applied the missing link statements and also use a single thread for RtcPeerConnection worker and signaling an I got to the point where most of the tests pass, but I get an abort here:

Something must have changed regarding to the internal threads.

jmillan · 2024-02-21T16:28:04Z

These errors come from calling track->set[ena|disa]bled() from Producer, when pausing and resuming it.

Meaning those methods are called by the test process but now libwebrtc expects it to come from the provided signaling thread...

jmillan · 2024-02-21T17:04:00Z

Email posted to discuss-webrtc https://groups.google.com/g/discuss-webrtc/c/q0srUpz1rbQ

jmillan · 2024-02-22T13:30:51Z

This is very probably only affecting the tests since we are using these internal encoder/decoder factories etc.

Can anyone please try this version within you own app?

copiltembel · 2024-02-23T10:27:48Z

I'm having a look at mediasoup-broadcaster-demo. I think it would be useful to have that build and run before moving forward. Here's the updates that I made so far: https://github.com/copiltembel/mediasoup-broadcaster-demo/tree/m120

Unfortunately I have the same issue as above at link time.

jmillan · 2024-02-23T12:22:44Z

I'm having a look at mediasoup-broadcaster-demo. I think it would be useful to have that build and run before moving forward. Here's the updates that I made so far: https://github.com/copiltembel/mediasoup-broadcaster-demo/tree/m120

Broadcaster demo uses the same internal encoder/decoders as the tests.., this is why it's not helpful.

This is why I asked that anyone who has an app based on libmediasoupclient to test this branch, since such app will get the tracks from a device or file and not from libwebrtc sources.

copiltembel · 2024-02-23T13:15:43Z

Broadcaster demo uses the same internal encoder/decoders as the tests.., this is why it's not helpful.

Got it.

This is why I asked that anyone who has an app based on libmediasoupclient to test this branch, since such app will get the tracks from a device or file and not from libwebrtc sources.

I have such an app, but as said before, I cannot build it using this branch. I guess because I can only build
libwebrtc m120 with use_custom_libcxx=true, I get tons of link errors like these when building my app:

/usr/bin/ld: int128.cc:(.text+0x10a): undefined reference to `VTT for std::__Cr::basic_ostringstream<char, std::__Cr::char_traits<char>, std::__Cr::allocator<char> >'
/usr/bin/ld: int128.cc:(.text+0x143): undefined reference to `std::__Cr::ios_base::init(void*)'
/usr/bin/ld: int128.cc:(.text+0x175): undefined reference to `std::__Cr::basic_streambuf<char, std::__Cr::char_traits<char> >::basic_streambuf()'
/usr/bin/ld: int128.cc:(.text+0x17c): undefined reference to `vtable for std::__Cr::basic_stringbuf<char, std::__Cr::char_traits<char>, std::__Cr::allocator<char> >'
/usr/bin/ld: int128.cc:(.text+0x245): undefined reference to `std::__Cr::basic_ostream<char, std::__Cr::char_traits<char> >::operator<<(unsigned long)'
/usr/bin/ld: int128.cc:(.text+0x28a): undefined reference to `std::__Cr::ios_base::getloc() const'
/usr/bin/ld: int128.cc:(.text+0x291): undefined reference to `std::__Cr::ctype<char>::id'
/usr/bin/ld: int128.cc:(.text+0x299): undefined reference to `std::__Cr::locale::use_facet(std::__Cr::locale::id&) const'
/usr/bin/ld: int128.cc:(.text+0x2af): undefined reference to `std::__Cr::locale::~locale()'
/usr/bin/ld: int128.cc:(.text+0x2e3): undefined reference to `std::__Cr::basic_ostream<char, std::__Cr::char_traits<char> >::operator<<(unsigned long)'
/usr/bin/ld: int128.cc:(.text+0x30c): undefined reference to `std::__Cr::basic_ostream<char, std::__Cr::char_traits<char> >::operator<<(unsigned long)'
/usr/bin/ld: int128.cc:(.text+0x351): undefined reference to `std::__Cr::ios_base::getloc() const'
/usr/bin/ld: int128.cc:(.text+0x358): undefined reference to `std::__Cr::ctype<char>::id'
/usr/bin/ld: int128.cc:(.text+0x360): undefined reference to `std::__Cr::locale::use_facet(std::__Cr::locale::id&) const'
/usr/bin/ld: int128.cc:(.text+0x376): undefined reference to `std::__Cr::locale::~locale()'
/usr/bin/ld: int128.cc:(.text+0x3aa): undefined reference to `std::__Cr::basic_ostream<char, std::__Cr::char_traits<char> >::operator<<(unsigned long)'
/usr/bin/ld: int128.cc:(.text+0x3d6): undefined reference to `std::__Cr::basic_stringbuf<char, std::__Cr::char_traits<char>, std::__Cr::allocator<char> >::str() const'
/usr/bin/ld: int128.cc:(.text+0x432): undefined reference to `std::__Cr::ios_base::getloc() const'
/usr/bin/ld: int128.cc:(.text+0x439): undefined reference to `std::__Cr::ctype<char>::id'
/usr/bin/ld: int128.cc:(.text+0x442): undefined reference to `std::__Cr::locale::use_facet(std::__Cr::locale::id&) const'
/usr/bin/ld: int128.cc:(.text+0x45c): undefined reference to `std::__Cr::locale::~locale()'
/usr/bin/ld: int128.cc:(.text+0x475): undefined reference to `std::__Cr::basic_string<char, std::__Cr::char_traits<char>, std::__Cr::allocator<char> >::append(unsigned long, char)'
/usr/bin/ld: int128.cc:(.text+0x488): undefined reference to `std::__Cr::ios_base::getloc() const'
/usr/bin/ld: int128.cc:(.text+0x48f): undefined reference to `std::__Cr::ctype<char>::id'
/usr/bin/ld: int128.cc:(.text+0x498): undefined reference to `std::__Cr::locale::use_facet(std::__Cr::locale::id&) const'
/usr/bin/ld: int128.cc:(.text+0x4b2): undefined reference to `std::__Cr::locale::~locale()'
/usr/bin/ld: int128.cc:(.text+0x4cd): undefined reference to `std::__Cr::basic_string<char, std::__Cr::char_traits<char>, std::__Cr::allocator<char> >::insert(unsigned long, unsigned long, char)'
/usr/bin/ld: int128.cc:(.text+0x508): undefined reference to `VTT for std::__Cr::basic_ostringstream<char, std::__Cr::char_traits<char>, std::__Cr::allocator<char> >'
/usr/bin/ld: int128.cc:(.text+0x54c): undefined reference to `std::__Cr::basic_streambuf<char, std::__Cr::char_traits<char> >::~basic_streambuf()'
/usr/bin/ld: int128.cc:(.text+0x55f): undefined reference to `std::__Cr::basic_ostream<char, std::__Cr::char_traits<char> >::~basic_ostream()'
/usr/bin/ld: int128.cc:(.text+0x567): undefined reference to `std::__Cr::basic_ios<char, std::__Cr::char_traits<char> >::~basic_ios()'

fedulvtubudul · 2024-02-23T16:17:24Z

iOS app I'm working on has switched to WerRTC m120 and libmediasoupclient 3.4.1. This version is in public for three weeks now and I don't see any anomalies in Crashlytics reports since then. I've already merged newer libmediasoupclient changes but can't say when it will roll to production yet.

Our Android team has also adopted WebRTC m120 and libmediasoupclient 3.4.1. They've faced assertion failures when WebRTC is built in debug mode when stopping a producer. Looked like calling something on a wrong thread. On release build it works stable though, but it's still in QA and not verified by real-world users yet.

copiltembel · 2024-02-27T08:01:39Z

After some libwebrtc code torturing, I was able to build, run and ... crash (my own app). Here is the call stack:

(on sig_thread)

libc.so.6!pthread_kill (Unknown Source:0)
libc.so.6!raise (Unknown Source:0)
libc.so.6!abort (Unknown Source:0)
libc.so.6![Unknown/Just-In-Time compiled code] (Unknown Source:0)
libc.so.6!__assert_fail (Unknown Source:0)
libc.so.6!pthread_mutex_lock (Unknown Source:0)
webrtc::webrtc_sequence_checker_internal::SequenceCheckerImpl::IsCurrent() const (Unknown Source:0)
webrtc::SequenceChecker::IsCurrent(const webrtc::SequenceChecker * const this) (/mnt/large4/src/google/libwebrtc/m120_webrtc/src/api/sequence_checker.h:67)
webrtc::Notifier<webrtc::VideoTrackSourceInterface>::RegisterObserver(webrtc::Notifier<webrtc::VideoTrackSourceInterface> * const this, webrtc::ObserverInterface * observer) (/mnt/large4/src/google/libwebrtc/m120_webrtc/src/api/notifier.h:31)
webrtc::VideoTrack::VideoTrack(std::basic_string_view<char, std::char_traits<char> >, webrtc::scoped_refptr<webrtc::VideoTrackSourceProxyWithInternal<webrtc::VideoTrackSourceInterface> >, rtc::Thread*) (Unknown Source:0)
webrtc::VideoTrack::Create(std::basic_string_view<char, std::char_traits<char> >, webrtc::scoped_refptr<webrtc::VideoTrackSourceInterface>, rtc::Thread*) (Unknown Source:0)
webrtc::VideoRtpReceiver::VideoRtpReceiver(rtc::Thread*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::vector<webrtc::scoped_refptr<webrtc::MediaStreamInterface>, std::allocator<webrtc::scoped_refptr<webrtc::MediaStreamInterface> > > const&) (Unknown Source:0)
webrtc::VideoRtpReceiver::VideoRtpReceiver(rtc::Thread*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::vector<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::allocator<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > >) (Unknown Source:0)
webrtc::RtpTransmissionManager::CreateReceiver(cricket::MediaType, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) (Unknown Source:0)
webrtc::PeerConnection::AddTransceiver(cricket::MediaType, webrtc::scoped_refptr<webrtc::MediaStreamTrackInterface>, webrtc::RtpTransceiverInit const&, bool) (Unknown Source:0)
webrtc::PeerConnection::AddTransceiver(cricket::MediaType, webrtc::RtpTransceiverInit const&) (Unknown Source:0)
webrtc::PeerConnection::AddTransceiver(cricket::MediaType) (Unknown Source:0)
void absl::internal_any_invocable::LocalInvoker<false, void, webrtc::MethodCall<webrtc::PeerConnectionInterface, webrtc::RTCErrorOr<webrtc::scoped_refptr<webrtc::RtpTransceiverInterface> >, cricket::MediaType>::Marshal(rtc::Thread*)::{lambda()#1}&&>(absl::internal_any_invocable::TypeErasedState*) (Unknown Source:0)
rtc::Thread::Dispatch(absl::AnyInvocable<void () &&>) (Unknown Source:0)
rtc::Thread::ProcessMessages(int) (Unknown Source:0)
rtc::Thread::PreRun(void*) (Unknown Source:0)
libc.so.6![Unknown/Just-In-Time compiled code] (Unknown Source:0)

webrtc/src/api/sequence_checker.h

  // Returns true if sequence checker is attached to the current sequence.
  bool IsCurrent() const { return Impl::IsCurrent(); }

The weird thing is:

// In Release mode, IsCurrent will always return true.
class RTC_LOCKABLE SequenceChecker
#if RTC_DCHECK_IS_ON
    : public webrtc_sequence_checker_internal::SequenceCheckerImpl {
  using Impl = webrtc_sequence_checker_internal::SequenceCheckerImpl;
#else
    : public webrtc_sequence_checker_internal::SequenceCheckerDoNothing {
  using Impl = webrtc_sequence_checker_internal::SequenceCheckerDoNothing;
#endif

Tried a bit with libwebrtc in debug mode, I get crashes when initializing the audio device module:

(webrtc_voice_engine.cc:354): WebRtcVoiceEngine::WebRtcVoiceEngine
(webrtc_video_engine.cc:771): WebRtcVideoEngine::WebRtcVideoEngine()
(webrtc_voice_engine.cc:376): WebRtcVoiceEngine::Init
(audio_device_impl.cc:292): Init
(audio_device_impl.cc:636): SetPlayoutDevice(0)


#
# Fatal error in: ../../../../modules/audio_device/linux/audio_device_pulse_linux.cc, line 660
# last system error: 0
# Check failed: thread_checker_.IsCurrent()
#

jmillan · 2024-02-27T08:49:32Z

@copiltembel,

The method calls of the track should hop on over to the signaling thread when invoking on a proxy object, which is what you should have, rather than having a direct pointer to the underlaying track object.

So something has changed lately in the way a track is obtained that we (including in libmediasoupclient tests) are not getting the track within the corresponding proxy.

That is why the thread check does not succeed.

copiltembel · 2024-02-27T09:33:51Z

@jmillan

Unfortunately I see there's no help coming from the discuss-webrtc forum.

jmillan · 2024-02-27T09:56:09Z

Unfortunately I see there's no help coming from the discuss-webrtc forum.

I got a private message (?) telling exactly what I said here.

copiltembel · 2024-02-27T14:31:35Z

Maybe I am misunderstanding something, but the crash I am seeing (with libwebrtc in release) is on the sig_thread.

ibc · 2024-02-27T14:47:51Z

Maybe I am misunderstanding something, but the crash I am seeing (with libwebrtc in release) is on the sig_thread.

Maybe the crash is in the signaling thread as a result of calling a method on an object using another thread. When in that scenario it's hard to anticipate how the whole app will crash. Imagine that the non-signaling thread (without any thread lock mechanism in place) changes some memory that the signaling thread is reading or writing to. Then the crash will indeed happen in the signaling thread.

copiltembel · 2024-02-28T14:06:46Z

I had an issue when building in debug, I wasn't using the workerThread to create the audioDeviceModule. With libwebrtc in debug, my app works now, without crashing.

But running with libwebrtc built in release mode crashes on device->Load(rtpCapabilities, &peerConnectionOptions);

pthread_mutex_lock.c:94: ___pthread_mutex_lock: Assertion `mutex->__data.__owner == 0' failed.

As @ibc said, there might be some concurrency issue, which occurs more often in release mode than in debug. Seems difficult to identify and fix.

mrosu90 · 2024-04-25T13:44:47Z

is there a plan to merge this into v3 anytime soon?

ibc · 2024-04-25T14:09:59Z

is there a plan to merge this into v3 anytime soon?

As you can see in comments above yours there are unresolved items before this PR can be merged. You are welcome to help with them.

Poldraunic · 2024-04-27T22:29:43Z

Hi! I have promised to look into the issues and didn't help at all! And for that I am sorry - life got in a way.

From what I can see in this thread and from what I've observed on my local machine, the issue here lies in the threads. LibWebRTC is very particular on how objects are created, used and destroyed. In order to enforce correct usage of these resources, they have all these RTC_DCHECK, RTC_DCHECK_RUN_ON assertion macros. Sometimes they simply check if you're calling all the functions on the same thread, other times they explicitly expect certain threads to be used.

For example, for the test case transport.produce() succeeds fail comes from the fact that the thread used to create webrtc::AudioRtpSender (and its base webrtc::RtpSenderBase) is not the same that used for notification dispatch.

On the screenshot below, you can see call stack for the current thread and its name. The thread name is indeed signaling_thread, but for whatever reason check fails. What gives?

Current Thread and Callstack

This doesn't make any sense, so let's patch LibWebRTC and add variable to store current thread. This way, we will be able to use debugger to inspect all variables. We will use this to compare current thread variable and class local thread variable. And what do we get? This is not the same thread at all! They share the same thread name yes, these are different objects.

Same name, but different address

So, how did we get into this situation? Well, the reason is how tests are structured. In the test case transport.produce() succeeds we create audio/video tracks using free functions createAudioTrack()/createVideoTrack(). These functions check for the presence of local for this translation unit webrtc::PeerConnectionFactoryInterface. If factory interface doesn't exist, we will create it along with the threads required for it to run. Tracks (and all underlying LibWebRTC objects) will be "bound" to the threads of this factory. However, this is not the only place where we create webrtc::PeerConnectionFactoryInterface and threads for it. Couple of tests cases earlier, we have device.load() succeeds test case with the following function call stack:

mediasoupclient::Device::Load()
mediasoupclient::Handler::GetNativeRtpCapabilities()
mediasoupclient::PeerConnection::PeerConnection()
webrtc::CreatePeerConnectionFactory() and threads for it.

And so, with every test case ran, we indirectly create more and more factory interfaces and threads for it until LibWebRTC blows up somewhere. Bonus picture showing amount of threads created up until transport.produce() succeeds fails:

27 LibWebRTC threads in a single test run

Poldraunic · 2024-04-27T22:31:13Z

src/PeerConnection.cpp

-			  this->workerThread.get(),
+			  // TMP: Use the same  thread for signaling and worker.
+			  // this->workerThread.get(),
+			  this->signalingThread.get(),


I also want to note that this looks extremely sketchy. If LibWebRTC truly wanted you to use signaling thread as both worker and signaling, they'd do so in other ways (such as taking only 2 pointers).

Yep, this is a temporal work around that needs to be removed.

Poldraunic · 2024-04-27T22:35:19Z

I also tested this on Windows/MSVC and to build mediasoupclient tests I had to patch LibWebRTC with the following patch. It is taken from LibWebRTC Gerrit.

diff --git a/build/config/win/BUILD.gn b/build/config/win/BUILD.gn
index 59caa7b42..7518fff68 100644
--- a/build/config/win/BUILD.gn
+++ b/build/config/win/BUILD.gn
@@ -41,6 +41,10 @@ declare_args() {
   #  and with this switch, clang emits it like this:
   #    foo/bar.cc:12:34: error: something went wrong
   use_clang_diagnostics_format = false
+
+  # Force dynamic crt build. Setting to true compiles targets using dynamic_crt (/MD).
+  # When false, default_crt will be used.
+  use_dynamic_crt = false
 }
 
 # This is included by reference in the //build/config/compiler config that
@@ -472,7 +476,7 @@ config("default_crt") {
     # exceptions on.
     configs = [ ":dynamic_crt" ]
   } else {
-    if (current_os == "winuwp") {
+    if (current_os == "winuwp" || use_dynamic_crt) {
       # https://blogs.msdn.microsoft.com/vcblog/2014/06/10/the-great-c-runtime-crt-refactoring/
       # contains a details explanation of what is happening with the Windows
       # CRT in Visual Studio releases related to Windows store applications.

During GN run you'd also need to pass use_dynamic_crt=true for it to work.

After that, linking would fail because no Windows system libraries where provided. I had to add following patch to CMakeLists.txt:

target_link_libraries(test_mediasoupclient PRIVATE
	Secur32
	Winmm
	wmcodecdspuuid
	Msdmo
	dmoguids
	Iphlpapi
)

jmillan · 2024-04-29T15:43:58Z

Thanks @Poldraunic! I'll create a proper Factory class to avoid that and try.

Poldraunic · 2024-04-29T16:34:27Z

@jmillan, let me know if you need help!

jmillan added 2 commits February 13, 2024 15:28

m120

7473167

tests

940bf4c

ibc reviewed Feb 13, 2024

View reviewed changes

src/Handler.cpp Show resolved Hide resolved

add missing new line

5f8c6e2

jmillan commented Feb 13, 2024

View reviewed changes

src/sdp/Utils.cpp Show resolved Hide resolved

ibc approved these changes Feb 13, 2024

View reviewed changes

jmillan mentioned this pull request Feb 20, 2024

Upgrade to m112 #154

Closed

jmillan added 2 commits February 21, 2024 17:01

cont.

2afd242

cont.

f8c73fc

jmillan mentioned this pull request Apr 3, 2024

vp8 and h264 only support L1Tx, but c++ code #175

Open

Poldraunic reviewed Apr 27, 2024

View reviewed changes

M120 #173

Are you sure you want to change the base?

M120 #173

Conversation

jmillan commented Feb 13, 2024

ibc left a comment

Choose a reason for hiding this comment

jmillan commented Feb 13, 2024

jmillan commented Feb 13, 2024

Poldraunic commented Feb 13, 2024

jmillan commented Feb 13, 2024

Poldraunic commented Feb 13, 2024

copiltembel commented Feb 13, 2024

copiltembel commented Feb 18, 2024

ibc commented Feb 18, 2024

fedulvtubudul commented Feb 18, 2024

copiltembel commented Feb 18, 2024

fedulvtubudul commented Feb 19, 2024

copiltembel commented Feb 19, 2024

fedulvtubudul commented Feb 19, 2024

copiltembel commented Feb 19, 2024

jmillan commented Feb 21, 2024

jmillan commented Feb 21, 2024

jmillan commented Feb 21, 2024

jmillan commented Feb 22, 2024 • edited

copiltembel commented Feb 23, 2024

jmillan commented Feb 23, 2024

copiltembel commented Feb 23, 2024 • edited

fedulvtubudul commented Feb 23, 2024

copiltembel commented Feb 27, 2024 • edited

jmillan commented Feb 27, 2024

copiltembel commented Feb 27, 2024

jmillan commented Feb 27, 2024

copiltembel commented Feb 27, 2024

ibc commented Feb 27, 2024

copiltembel commented Feb 28, 2024

mrosu90 commented Apr 25, 2024

ibc commented Apr 25, 2024

Poldraunic commented Apr 27, 2024 • edited

Poldraunic Apr 27, 2024

Choose a reason for hiding this comment

jmillan Apr 29, 2024

Choose a reason for hiding this comment

Poldraunic commented Apr 27, 2024 • edited

jmillan commented Apr 29, 2024

Poldraunic commented Apr 29, 2024

jmillan commented Feb 22, 2024 •

edited

copiltembel commented Feb 23, 2024 •

edited

copiltembel commented Feb 27, 2024 •

edited

Poldraunic commented Apr 27, 2024 •

edited

Poldraunic commented Apr 27, 2024 •

edited