Bug 190361 – Refresh libwebrtc up to 343f4144be

• Status: RESOLVED
• Resolution: FIXED
• Component: WebRTC

[youenn fablet]

This will allow getting improvements related to addTransceiver and MDNS candidate handling.

[youenn fablet]

Created attachment 351793 [details]
patch

[EWS Watchlist]

Attachment 351793 [details] did not pass style-queue:


ERROR: Source/WebCore/testing/MockLibWebRTCPeerConnection.cpp:333:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Total errors found: 1 in 1666 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[youenn fablet]

Created attachment 351821 [details]
patch

[EWS Watchlist]

Attachment 351821 [details] did not pass style-queue:


ERROR: Source/WebCore/testing/MockLibWebRTCPeerConnection.cpp:333:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Total errors found: 1 in 1668 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[youenn fablet]

Created attachment 351839 [details]
patch

[EWS Watchlist]

Attachment 351839 [details] did not pass style-queue:


ERROR: Source/WebCore/testing/MockLibWebRTCPeerConnection.cpp:333:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Total errors found: 1 in 1668 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[youenn fablet]

Created attachment 351842 [details]
patch

[EWS Watchlist]

Attachment 351842 [details] did not pass style-queue:


ERROR: Source/WebCore/testing/MockLibWebRTCPeerConnection.cpp:333:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Total errors found: 1 in 1674 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[youenn fablet]

Created attachment 351847 [details]
patch

[EWS Watchlist]

Attachment 351847 [details] did not pass style-queue:


ERROR: Source/WebCore/testing/MockLibWebRTCPeerConnection.cpp:333:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Total errors found: 1 in 1674 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[youenn fablet]

Created attachment 351855 [details]
patch

[EWS Watchlist]

Attachment 351855 [details] did not pass style-queue:


ERROR: Source/WebCore/testing/MockLibWebRTCPeerConnection.cpp:333:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Total errors found: 1 in 1675 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[EWS Watchlist]

Comment on attachment 351855 [details]
patch

Attachment 351855 [details] did not pass mac-ews (mac):
Output: https://webkit-queues.webkit.org/results/9499967

New failing tests:
platform/mac/webrtc/captureCanvas-webrtc-software-encoder.html

[EWS Watchlist]

Created attachment 351862 [details]
Archive of layout-test-results from ews102 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews102  Port: mac-sierra  Platform: Mac OS X 10.12.6

[youenn fablet]

Created attachment 351881 [details]
patch

[youenn fablet]

Created attachment 351896 [details]
patch

[EWS Watchlist]

Attachment 351896 [details] did not pass style-queue:


ERROR: Source/WebCore/testing/MockLibWebRTCPeerConnection.cpp:333:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Total errors found: 1 in 1675 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Alejandro G. Castro]

Created attachment 351956 [details]
GTK and WPE compilation patch

Basically the changes are:
   - add and remove files
   - a couple of additions we always do for libevent external and stringutils
   - modifications of the code for gstreamer factories (timestamps continue to change: thibault check this)
   - rtc_base/logging.cc apparently was old in the patch, got the version of the file for 343f4144be to make it compile
   - absl/strings/string_view.h gcc does not allow multiple lines for constexpr in the version that I use, it is just a assert so I basically leave it for clang

[Thibault Saunier]

(In reply to Alejandro G. Castro from comment #18)
> Created attachment 351956 [details]
> GTK and WPE compilation patch
> 
> Basically the changes are:
>    - add and remove files
>    - a couple of additions we always do for libevent external and stringutils
>    - modifications of the code for gstreamer factories (timestamps continue
> to change: thibault check this)

No problem from what I can see.

[youenn fablet]

Some comments below.
We need logging.cc to keep these changes until libwebrtc updates them to remove static initializers warning. Is there a way to update the version I uploaded to make it compile in GTK?

(In reply to Alejandro G. Castro from comment #18)
> Created attachment 351956 [details]
> GTK and WPE compilation patch
> 
> Basically the changes are:
>    - add and remove files

Ideally, we would just have that to do.

>    - a couple of additions we always do for libevent external and stringutils

Can they be upstreamed to libwebrtc?

>    - modifications of the code for gstreamer factories (timestamps continue
> to change: thibault check this)
>    - rtc_base/logging.cc apparently was old in the patch, got the version of
> the file for 343f4144be to make it compile

I kept some of WebKit specific logging.cc changes as this is required to remove some static initializer warnings in Mac/iOS ports. Libwebrtc is working on fixing these but has not fully finalized.

>    - absl/strings/string_view.h gcc does not allow multiple lines for
> constexpr in the version that I use, it is just a assert so I basically
> leave it for clang

Is there a diff of these changes somewhere?
Can we upstream them to libwebrtc?

[Alejandro G. Castro]

(In reply to youenn fablet from comment #20)
> Some comments below.
> We need logging.cc to keep these changes until libwebrtc updates them to
> remove static initializers warning. Is there a way to update the version I
> uploaded to make it compile in GTK?
> 

Yep, you can basically add an ifdef to these 2 lines:

114   static std::once_flag callLogCriticalScopeOnce;
115   std::call_once(callLogCriticalScopeOnce,[] { logCriticalScope(); });


> (In reply to Alejandro G. Castro from comment #18)
> > Created attachment 351956 [details]
> > GTK and WPE compilation patch
> > 
> > Basically the changes are:
> >    - add and remove files
> 
> Ideally, we would just have that to do.
> 

I agree, changes are really small now, hopefully we can remove all of them sooner than later.

> >    - a couple of additions we always do for libevent external and stringutils
> 
> Can they be upstreamed to libwebrtc?
> 

I doubt we can, as you can see in the patch: one is to allow using an external libevent and the other is because libwebrtc uses tolower, we disallow that function in WebKit. I proposed to add more conditions to the ifdef in the previous patch for WebKit but it seems too much just to compile the library.

> >    - modifications of the code for gstreamer factories (timestamps continue
> > to change: thibault check this)
> >    - rtc_base/logging.cc apparently was old in the patch, got the version of
> > the file for 343f4144be to make it compile
> 
> I kept some of WebKit specific logging.cc changes as this is required to
> remove some static initializer warnings in Mac/iOS ports. Libwebrtc is
> working on fixing these but has not fully finalized.
> 

I understand, we can just remove two of the lines removed upstream or add an ifdef to them.

> >    - absl/strings/string_view.h gcc does not allow multiple lines for
> > constexpr in the version that I use, it is just a assert so I basically
> > leave it for clang
> 
> Is there a diff of these changes somewhere?
> Can we upstream them to libwebrtc?

The change is in the patch, I guess they don't support compiling with gcc, or maybe there is a way to allow this in the compilation. This is the new abseil library, not even libwebrtc, we can check with them in more detail.

[Alejandro G. Castro]

(In reply to Alejandro G. Castro from comment #21)
> (In reply to youenn fablet from comment #20)
> > >    - absl/strings/string_view.h gcc does not allow multiple lines for
> > > constexpr in the version that I use, it is just a assert so I basically
> > > leave it for clang
> > 
> > Is there a diff of these changes somewhere?
> > Can we upstream them to libwebrtc?
> 
> The change is in the patch, I guess they don't support compiling with gcc,
> or maybe there is a way to allow this in the compilation. This is the new
> abseil library, not even libwebrtc, we can check with them in more detail.

This is actually a bug in the library :-), we got a version with the problem in it, it explains clearly how to use it in the code:

// ABSL_ASSERT()
//
// In C++11, `assert` can't be used portably within constexpr functions.
// ABSL_ASSERT functions as a runtime assert but works in C++11 constexpr
// functions.  Example:
//
// constexpr double Divide(double a, double b) {
//   return ABSL_ASSERT(b != 0), a / b;
// }
//
// This macro is inspired by
// https://akrzemi1.wordpress.com/2017/05/18/asserts-in-constexpr-functions/

It is already fixed in abseil-cpp master so I'm going to upload the exact code to the patch.

[Alejandro G. Castro]

Created attachment 352033 [details]
GTK and WPE compilation patch

New version of the compilation patch for GTK and WPE:
   - Solves the ABSEIL_ASSERT with the code upstream.
   - rtc_base/logging.cc removes just the 2 lines that cause problems in the compilation, it seems safe until we can update.

[youenn fablet]

Created attachment 352061 [details]
Merged patch

[youenn fablet]

Created attachment 352064 [details]
patch

[EWS Watchlist]

Attachment 352064 [details] did not pass style-queue:


ERROR: Source/WebCore/testing/MockLibWebRTCPeerConnection.cpp:333:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Total errors found: 1 in 1677 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[youenn fablet]

Created attachment 352067 [details]
patch

[EWS Watchlist]

Attachment 352067 [details] did not pass style-queue:


ERROR: Source/WebCore/testing/MockLibWebRTCPeerConnection.cpp:333:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Total errors found: 1 in 1677 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Chris Dumez]

Comment on attachment 352067 [details]
patch

rs=me

[WebKit Commit Bot]

Comment on attachment 352067 [details]
patch

Rejecting attachment 352067 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-01', 'land-attachment', '--force-clean', '--non-interactive', '--parent-command=commit-queue', 352067, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit

Last 5000 characters of output:
ce/webrtc/video/end_to_end_tests/multi_stream_tester.cc
	M	Source/ThirdParty/libwebrtc/Source/webrtc/video/end_to_end_tests/multi_stream_tests.cc
	M	Source/ThirdParty/libwebrtc/Source/webrtc/video/end_to_end_tests/network_state_tests.cc
	M	Source/ThirdParty/libwebrtc/Source/webrtc/video/end_to_end_tests/probing_tests.cc
	M	Source/ThirdParty/libwebrtc/Source/webrtc/video/end_to_end_tests/receive_time_tests.cc
	M	Source/ThirdParty/libwebrtc/Source/webrtc/video/end_to_end_tests/retransmission_tests.cc
	M	Source/ThirdParty/libwebrtc/Source/webrtc/video/end_to_end_tests/rtp_rtcp_tests.cc
	M	Source/ThirdParty/libwebrtc/Source/webrtc/video/end_to_end_tests/ssrc_tests.cc
	M	Source/ThirdParty/libwebrtc/Source/webrtc/video/end_to_end_tests/stats_tests.cc
	M	Source/ThirdParty/libwebrtc/Source/webrtc/video/end_to_end_tests/transport_feedback_tests.cc
	M	Source/ThirdParty/libwebrtc/Source/webrtc/video/full_stack_tests.cc
	M	Source/ThirdParty/libwebrtc/Source/webrtc/video/overuse_frame_detector.cc
	M	Source/ThirdParty/libwebrtc/Source/webrtc/video/picture_id_tests.cc
	M	Source/ThirdParty/libwebrtc/Source/webrtc/video/quality_scaling_tests.cc
	M	Source/ThirdParty/libwebrtc/Source/webrtc/video/quality_threshold.cc
	M	Source/ThirdParty/libwebrtc/Source/webrtc/video/quality_threshold.h
	M	Source/ThirdParty/libwebrtc/Source/webrtc/video/receive_statistics_proxy.cc
	M	Source/ThirdParty/libwebrtc/Source/webrtc/video/receive_statistics_proxy.h
	M	Source/ThirdParty/libwebrtc/Source/webrtc/video/receive_statistics_proxy_unittest.cc
	M	Source/ThirdParty/libwebrtc/Source/webrtc/video/replay.cc
	M	Source/ThirdParty/libwebrtc/Source/webrtc/video/rtp_streams_synchronizer.cc
	M	Source/ThirdParty/libwebrtc/Source/webrtc/video/rtp_streams_synchronizer.h
	M	Source/ThirdParty/libwebrtc/Source/webrtc/video/rtp_video_stream_receiver.cc
	M	Source/ThirdParty/libwebrtc/Source/webrtc/video/rtp_video_stream_receiver.h
	M	Source/ThirdParty/libwebrtc/Source/webrtc/video/rtp_video_stream_receiver_unittest.cc
	M	Source/ThirdParty/libwebrtc/Source/webrtc/video/screenshare_loopback.cc
	M	Source/ThirdParty/libwebrtc/Source/webrtc/video/send_delay_stats.h
	M	Source/ThirdParty/libwebrtc/Source/webrtc/video/send_delay_stats_unittest.cc
	M	Source/ThirdParty/libwebrtc/Source/webrtc/video/send_statistics_proxy.cc
	M	Source/ThirdParty/libwebrtc/Source/webrtc/video/send_statistics_proxy.h
	M	Source/ThirdParty/libwebrtc/Source/webrtc/video/send_statistics_proxy_unittest.cc
	M	Source/ThirdParty/libwebrtc/Source/webrtc/video/stats_counter.cc
	M	Source/ThirdParty/libwebrtc/Source/webrtc/video/sv_loopback.cc
	M	Source/ThirdParty/libwebrtc/Source/webrtc/video/transport_adapter.cc
	M	Source/ThirdParty/libwebrtc/Source/webrtc/video/transport_adapter.h
	M	Source/ThirdParty/libwebrtc/Source/webrtc/video/video_analyzer.cc
	M	Source/ThirdParty/libwebrtc/Source/webrtc/video/video_analyzer.h
	M	Source/ThirdParty/libwebrtc/Source/webrtc/video/video_loopback.cc
	M	Source/ThirdParty/libwebrtc/Source/webrtc/video/video_quality_observer.cc
	M	Source/ThirdParty/libwebrtc/Source/webrtc/video/video_quality_test.cc
	M	Source/ThirdParty/libwebrtc/Source/webrtc/video/video_quality_test.h
	M	Source/ThirdParty/libwebrtc/Source/webrtc/video/video_receive_stream.cc
	M	Source/ThirdParty/libwebrtc/Source/webrtc/video/video_receive_stream.h
	M	Source/ThirdParty/libwebrtc/Source/webrtc/video/video_receive_stream_unittest.cc
	M	Source/ThirdParty/libwebrtc/Source/webrtc/video/video_send_stream.cc
	M	Source/ThirdParty/libwebrtc/Source/webrtc/video/video_send_stream.h
	M	Source/ThirdParty/libwebrtc/Source/webrtc/video/video_send_stream_impl.cc
	M	Source/ThirdParty/libwebrtc/Source/webrtc/video/video_send_stream_impl.h
	M	Source/ThirdParty/libwebrtc/Source/webrtc/video/video_send_stream_impl_unittest.cc
	M	Source/ThirdParty/libwebrtc/Source/webrtc/video/video_send_stream_tests.cc
	M	Source/ThirdParty/libwebrtc/Source/webrtc/video/video_stream_decoder.h
	M	Source/ThirdParty/libwebrtc/Source/webrtc/video/video_stream_decoder_impl.cc
	M	Source/ThirdParty/libwebrtc/Source/webrtc/video/video_stream_encoder.cc
	M	Source/ThirdParty/libwebrtc/Source/webrtc/video/video_stream_encoder_unittest.cc
	M	Source/ThirdParty/libwebrtc/Source/webrtc/webrtc.gni
	M	Source/ThirdParty/libwebrtc/libwebrtc.xcodeproj/project.pbxproj
	M	Source/WebCore/ChangeLog

ERROR from SVN:
Item is out of date: File '/trunk/Source/WebCore/ChangeLog' is out of date
W: 4081d3cc069a1d4a559a73b15c5995854c4c0af9 and refs/remotes/origin/master differ, using rebase:
:040000 040000 36e803d77c73702312f685641ad4f3961c785f20 7b9daf9ec3bcf63a1a0600e3152d4144df6297c3 M	Source
Current branch master is up to date.
ERROR: Not all changes have been committed into SVN, however the committed
ones (if any) seem to be successfully integrated into the working tree.
Please see the above messages for details.


Failed to run "['git', 'svn', 'dcommit', '--rmdir']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit
Updating OpenSource
Current branch master is up to date.

Full output: https://webkit-queues.webkit.org/results/9554705

[youenn fablet]

Landed manually as https://trac.webkit.org/r237075

[Radar WebKit Bug Importer]

<rdar://problem/45236328>

[youenn fablet]

GTK bot seems to have issues at link time.

../../Source/ThirdParty/libwebrtc/Source/webrtc/api/rtpsenderinterface.h:33: error: undefined reference to 'vtable for webrtc::RtpSenderInterface'
/usr/bin/ld.gold: the vtable symbol may be undefined because the class is missing its key function
../../Source/ThirdParty/libwebrtc/Source/webrtc/api/rtpsenderinterface.h:85: error: undefined reference to 'vtable for webrtc::RtpSenderInterface'
/usr/bin/ld.gold: the vtable symbol may be undefined because the class is missing its key function
lib/libwebrtc.a(lib/../Source/ThirdParty/libwebrtc/CMakeFiles/webrtc.dir/Source/webrtc/pc/rtpsender.cc.o):rtpsender.cc:vtable for rtc::RefCountedObject<webrtc::RtpSenderInternal>: error: undefined reference to 'webrtc::RtpSenderInterface::init_send_encodings() const'
lib/libwebrtc.a(lib/../Source/ThirdParty/libwebrtc/CMakeFiles/webrtc.dir/Source/webrtc/pc/rtpsender.cc.o):rtpsender.cc:vtable for rtc::RefCountedObject<webrtc::RtpSenderInternal>: error: undefined reference to 'webrtc::RtpSenderInterface::SetFrameEncryptor(rtc::scoped_refptr<webrtc::FrameEncryptorInterface>)'
lib/libwebrtc.a(lib/../Source/ThirdParty/libwebrtc/CMakeFiles/webrtc.dir/Source/webrtc/pc/rtpsender.cc.o):rtpsender.cc:vtable for rtc::RefCountedObject<webrtc::RtpSenderInternal>: error: undefined reference to 'webrtc::RtpSenderInterface::GetFrameEncryptor() const'
lib/libwebrtc.a(lib/../Source/ThirdParty/libwebrtc/CMakeFiles/webrtc.dir/Source/webrtc/pc/rtpsender.cc.o):rtpsender.cc:vtable for webrtc::RtpSenderInternal: error: undefined reference to 'webrtc::RtpSenderInterface::init_send_encodings() const'
lib/libwebrtc.a(lib/../Source/ThirdParty/libwebrtc/CMakeFiles/webrtc.dir/Source/webrtc/pc/rtpsender.cc.o):rtpsender.cc:vtable for webrtc::RtpSenderInternal: error: undefined reference to 'webrtc::RtpSenderInterface::SetFrameEncryptor(rtc::scoped_refptr<webrtc::FrameEncryptorInterface>)'
lib/libwebrtc.a(lib/../Source/ThirdParty/libwebrtc/CMakeFiles/webrtc.dir/Source/webrtc/pc/rtpsender.cc.o):rtpsender.cc:vtable for webrtc::RtpSenderInternal: error: undefined reference to 'webrtc::RtpSenderInterface::GetFrameEncryptor() const'
collect2: error: ld returned 1 exit status

[Ryan Haddad]

Committed http://trac.webkit.org/projects/webkit/changeset/237076 to fix internal builds.

[youenn fablet]

Tried to fix it at  https://trac.webkit.org/r237079

[youenn fablet]

(In reply to Ryan Haddad from comment #34)
> Committed http://trac.webkit.org/projects/webkit/changeset/237076 to fix
> internal builds.

Thanks Ryan!

[youenn fablet]

(In reply to youenn fablet from comment #36)
> (In reply to Ryan Haddad from comment #34)
> > Committed http://trac.webkit.org/projects/webkit/changeset/237076 to fix
> > internal builds.
> 
> Thanks Ryan!

I think I need to beef up style checker...