Bug 218436 - Update WebRTC libwebrtc to M87
Summary: Update WebRTC libwebrtc to M87
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebRTC (show other bugs)
Version: WebKit Local Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: youenn fablet
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-11-02 03:44 PST by youenn fablet
Modified: 2020-11-10 21:24 PST (History)
15 users (show)

See Also:


Attachments
Patch (11.37 MB, patch)
2020-11-04 03:44 PST, youenn fablet
no flags Details | Formatted Diff | Diff
Patch (10.83 MB, patch)
2020-11-04 10:57 PST, youenn fablet
no flags Details | Formatted Diff | Diff
Patch (10.82 MB, patch)
2020-11-04 11:13 PST, youenn fablet
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (69.57 MB, patch)
2020-11-04 12:59 PST, youenn fablet
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (10.82 MB, patch)
2020-11-05 00:04 PST, youenn fablet
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
WIP of changes for gtk/wpe (7.97 KB, patch)
2020-11-09 01:16 PST, Víctor M. Jáquez L.
no flags Details | Formatted Diff | Diff
Changes on wpe/gtk for M89 update (16.75 KB, patch)
2020-11-10 00:13 PST, Víctor M. Jáquez L.
no flags Details | Formatted Diff | Diff
Patch (10.83 MB, patch)
2020-11-10 06:16 PST, youenn fablet
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (10.83 MB, patch)
2020-11-10 06:46 PST, youenn fablet
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description youenn fablet 2020-11-02 03:44:24 PST
Update WebRTC liwebrtc to M87
Comment 1 youenn fablet 2020-11-04 03:44:37 PST
Created attachment 413153 [details]
Patch
Comment 2 youenn fablet 2020-11-04 10:57:23 PST
Created attachment 413180 [details]
Patch
Comment 3 youenn fablet 2020-11-04 11:13:06 PST
Created attachment 413185 [details]
Patch
Comment 4 youenn fablet 2020-11-04 12:59:23 PST
Created attachment 413197 [details]
Patch
Comment 5 Eric Carlson 2020-11-04 13:05:35 PST
rs=me once the bots are happy
Comment 6 youenn fablet 2020-11-05 00:04:02 PST
Created attachment 413265 [details]
Patch
Comment 7 Víctor M. Jáquez L. 2020-11-05 09:23:50 PST
This patch fix the use shown by the bots:

--- a/Source/ThirdParty/libwebrtc/CMakeLists.txt
+++ b/Source/ThirdParty/libwebrtc/CMakeLists.txt
@@ -1452,6 +1452,7 @@ target_compile_definitions(webrtc PRIVATE
   WEBRTC_CODEC_ISAC
   WEBRTC_CODEC_OPUS
   WEBRTC_CODEC_RED
+  WEBRTC_ENABLE_LINUX_ALSA
   WEBRTC_INCLUDE_INTERNAL_AUDIO_DEVICE
   WEBRTC_INTELLIGIBILITY_ENHANCER=0
   WEBRTC_LINUX
@@ -1463,7 +1464,6 @@ target_compile_definitions(webrtc PRIVATE
   WEBRTC_USE_BUILTIN_ISAC_FIX=1
   WEBRTC_USE_BUILTIN_ISAC_FLOAT=0
   WTF_USE_DYNAMIC_ANNOTATIONS=1
-  LINUX_ALSA
   RTC_DISABLE_VP9
   _GNU_SOURCE
   __Userspace__


But more scary ones appear afterwards
Comment 8 Víctor M. Jáquez L. 2020-11-06 05:53:58 PST
Oddly, the first errors appear after fixing WEBRTC_ENABLE_LINUX_ALSA, are stdint.h related: 

In file included from ../../Source/ThirdParty/libwebrtc/Source/webrtc/modules/congestion_controller/rtp/transport_feedback_demuxer.cc:10:                                                                [130/5701]
In file included from ../../Source/ThirdParty/libwebrtc/Source/webrtc/modules/congestion_controller/rtp/transport_feedback_demuxer.h:17:
../../Source/ThirdParty/libwebrtc/Source/webrtc/modules/include/module_common_types_public.h:43:41: error: use of undeclared identifier 'uint32_t'
                    std::numeric_limits<uint32_t>::max(),
                                        ^
../../Source/ThirdParty/libwebrtc/Source/webrtc/modules/include/module_common_types_public.h:83:43: error: use of undeclared identifier 'uint16_t'
using SequenceNumberUnwrapper = Unwrapper<uint16_t>; 
                                          ^


As if an include is missing somewhere.
Comment 9 youenn fablet 2020-11-06 06:02:07 PST
(In reply to Víctor M. Jáquez L. from comment #8)
> Oddly, the first errors appear after fixing WEBRTC_ENABLE_LINUX_ALSA, are
> stdint.h related: 
> 
> In file included from
> ../../Source/ThirdParty/libwebrtc/Source/webrtc/modules/
> congestion_controller/rtp/transport_feedback_demuxer.cc:10:                 
> [130/5701]
> In file included from
> ../../Source/ThirdParty/libwebrtc/Source/webrtc/modules/
> congestion_controller/rtp/transport_feedback_demuxer.h:17:
> ../../Source/ThirdParty/libwebrtc/Source/webrtc/modules/include/
> module_common_types_public.h:43:41: error: use of undeclared identifier
> 'uint32_t'
>                     std::numeric_limits<uint32_t>::max(),
>                                         ^
> ../../Source/ThirdParty/libwebrtc/Source/webrtc/modules/include/
> module_common_types_public.h:83:43: error: use of undeclared identifier
> 'uint16_t'
> using SequenceNumberUnwrapper = Unwrapper<uint16_t>; 
>                                           ^
> 
> 
> As if an include is missing somewhere.

Right, this is probably the issue.
I might have overwritten an #include <cstdint> when resyncing.
Best thing would be to read it as follow:

#if defined(WEBRTC_WEBKIT_BUILD)
#include <cstdint>
#endif
Comment 10 youenn fablet 2020-11-06 06:02:28 PST
s/read/readd
Comment 11 youenn fablet 2020-11-06 06:36:13 PST
It seems it is missing the following changes: https://bugs.webkit.org/show_bug.cgi?id=214806
Comment 12 Víctor M. Jáquez L. 2020-11-06 06:48:34 PST
I just see that M87 was released today (4 hours ago) https://groups.google.com/g/discuss-webrtc/c/6VmKkCjRK0k/m/YyOTQyQ5AAAJ (commit id 75b9ab6751f3f49bbdd0a260fcbcbc8135dd567a)

So I wonder if your sync is updated O:) or if you preferred some previous commit behind the release.
Comment 13 Víctor M. Jáquez L. 2020-11-06 06:51:48 PST
(In reply to Víctor M. Jáquez L. from comment #12)
> I just see that M87 was released today (4 hours ago)
> https://groups.google.com/g/discuss-webrtc/c/6VmKkCjRK0k/m/YyOTQyQ5AAAJ
> (commit id 75b9ab6751f3f49bbdd0a260fcbcbc8135dd567a)
> 
> So I wonder if your sync is updated O:) or if you preferred some previous
> commit behind the release.

The last commit is from September 30. The release announce was afterwards. Sorry.
Comment 14 youenn fablet 2020-11-06 06:53:20 PST
(In reply to Víctor M. Jáquez L. from comment #13)
> (In reply to Víctor M. Jáquez L. from comment #12)
> > I just see that M87 was released today (4 hours ago)
> > https://groups.google.com/g/discuss-webrtc/c/6VmKkCjRK0k/m/YyOTQyQ5AAAJ
> > (commit id 75b9ab6751f3f49bbdd0a260fcbcbc8135dd567a)
> > 
> > So I wonder if your sync is updated O:) or if you preferred some previous
> > commit behind the release.
> 
> The last commit is from September 30. The release announce was afterwards.
> Sorry.

This patch is taking this branch plus a few cherry-picks.
It might be that some additional cherry-picks happen in the M87.
I will probably try to cherry-pick them later in the month, the patch should be much smaller this time.
Comment 15 Víctor M. Jáquez L. 2020-11-06 07:00:04 PST
(In reply to youenn fablet from comment #11)
> It seems it is missing the following changes:
> https://bugs.webkit.org/show_bug.cgi?id=214806

Cool. Cherry-picking that patch fixed those errors (I've modified them to guard them with WEBRTC_WEBKIT_BUILD).

Now there are some H265 includes missing:

../../Source/ThirdParty/libwebrtc/Source/webrtc/modules/rtp_rtcp/source/rtp_format_h265.h:30:21: error: unknown type name 'H265PacketizationMode'; did you mean 'H264PacketizationMode'?                           
                    H265PacketizationMode packetization_mode);                                                                                                                                                     
                    ^~~~~~~~~~~~~~~~~~~~~                                                                                                                                                                          
                    H264PacketizationMode                 

Digging it
Comment 16 youenn fablet 2020-11-06 07:02:16 PST
(In reply to Víctor M. Jáquez L. from comment #15)
> (In reply to youenn fablet from comment #11)
> > It seems it is missing the following changes:
> > https://bugs.webkit.org/show_bug.cgi?id=214806
> 
> Cool. Cherry-picking that patch fixed those errors (I've modified them to
> guard them with WEBRTC_WEBKIT_BUILD).
> 
> Now there are some H265 includes missing:
> 
> ../../Source/ThirdParty/libwebrtc/Source/webrtc/modules/rtp_rtcp/source/
> rtp_format_h265.h:30:21: error: unknown type name 'H265PacketizationMode';
> did you mean 'H264PacketizationMode'?                           
>                     H265PacketizationMode packetization_mode);              
> 
>                     ^~~~~~~~~~~~~~~~~~~~~                                   
> 
>                     H264PacketizationMode                 
> 
> Digging it

My fault probably, I had to merge things back and there are some #ifndef DISABLE_H265
that I might have missed.
Comment 17 youenn fablet 2020-11-06 07:03:21 PST
Best thing might be to wrap rtp_format_h265.h with #ifndef DISABLE_H265
Comment 18 Víctor M. Jáquez L. 2020-11-06 07:20:31 PST
(In reply to youenn fablet from comment #17)
> Best thing might be to wrap rtp_format_h265.h with #ifndef DISABLE_H265

Fixed there and in other part.

Now I get this 

In file included from ../../Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCPeerConnectionBackend.cpp:33:
In file included from ../../Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCMediaEndpoint.h:31:
In file included from ../../Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCRtpSenderBackend.h:33:
../../Source/WebCore/platform/mediastream/RealtimeOutgoingVideoSource.h:40:10: fatal error: 'webrtc/common_video/include/i420_buffer_pool.h' file not found
#include <webrtc/common_video/include/i420_buffer_pool.h>
         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Digging.
Comment 19 youenn fablet 2020-11-06 07:22:34 PST
(In reply to Víctor M. Jáquez L. from comment #18)
> (In reply to youenn fablet from comment #17)
> > Best thing might be to wrap rtp_format_h265.h with #ifndef DISABLE_H265
> 
> Fixed there and in other part.
> 
> Now I get this 
> 
> In file included from
> ../../Source/WebCore/Modules/mediastream/libwebrtc/
> LibWebRTCPeerConnectionBackend.cpp:33:
> In file included from
> ../../Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCMediaEndpoint.h:
> 31:
> In file included from
> ../../Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCRtpSenderBackend.
> h:33:
> ../../Source/WebCore/platform/mediastream/RealtimeOutgoingVideoSource.h:40:
> 10: fatal error: 'webrtc/common_video/include/i420_buffer_pool.h' file not
> found
> #include <webrtc/common_video/include/i420_buffer_pool.h>
>          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> Digging.

This include should probably be removed
Comment 20 Víctor M. Jáquez L. 2020-11-06 07:32:53 PST
(In reply to youenn fablet from comment #19)
> (In reply to Víctor M. Jáquez L. from comment #18)
> > (In reply to youenn fablet from comment #17)
> > > Best thing might be to wrap rtp_format_h265.h with #ifndef DISABLE_H265
> > 
> > Fixed there and in other part.
> > 
> > Now I get this 
> > 
> > In file included from
> > ../../Source/WebCore/Modules/mediastream/libwebrtc/
> > LibWebRTCPeerConnectionBackend.cpp:33:
> > In file included from
> > ../../Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCMediaEndpoint.h:
> > 31:
> > In file included from
> > ../../Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCRtpSenderBackend.
> > h:33:
> > ../../Source/WebCore/platform/mediastream/RealtimeOutgoingVideoSource.h:40:
> > 10: fatal error: 'webrtc/common_video/include/i420_buffer_pool.h' file not
> > found
> > #include <webrtc/common_video/include/i420_buffer_pool.h>
> >          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > 
> > Digging.
> 
> This include should probably be removed

Indeed. And it's used also in the GStreamer implementation. Let me see how it can be replaced.
Comment 21 Víctor M. Jáquez L. 2020-11-06 07:45:47 PST
It could be ported to use VideoFrameBufferPool

https://webrtc-review.googlesource.com/c/src/+/184510
Comment 22 Víctor M. Jáquez L. 2020-11-06 08:14:15 PST
(In reply to Víctor M. Jáquez L. from comment #21)
> It could be ported to use VideoFrameBufferPool
> 
> https://webrtc-review.googlesource.com/c/src/+/184510

Fixed.

RTPFragmentationHeader was also removed :(
Comment 23 Víctor M. Jáquez L. 2020-11-06 08:49:02 PST
The OnEncodedImage callback has changed too. This might take more time than expected adapting the API usage. Now I need to go. I back on in later today or some time this weekend.
Comment 24 Víctor M. Jáquez L. 2020-11-09 01:16:31 PST
Created attachment 413567 [details]
WIP of changes for gtk/wpe

There has been changes in libwebrtc api that affects our encoders implementation. I'm still working on it.
Comment 25 Radar WebKit Bug Importer 2020-11-09 03:45:40 PST
<rdar://problem/71185126>
Comment 26 Víctor M. Jáquez L. 2020-11-10 00:13:12 PST
Created attachment 413673 [details]
Changes on wpe/gtk for M89 update

With this patch, the build  compiles cleanly, though several tests became broken. I'll review them one by one.

I have doubts about adding video_rtp_depacketizer_vp9.cc in the build. Other than that I think with these modifications the update is ready to merge.
Comment 27 youenn fablet 2020-11-10 00:23:00 PST
(In reply to Víctor M. Jáquez L. from comment #26)
> Created attachment 413673 [details]
> Changes on wpe/gtk for M89 update
> 
> With this patch, the build  compiles cleanly, though several tests became
> broken. I'll review them one by one.

Great, thanks!
Let me know if you need help for the tests.

I'll merge the patch today and reupload in bugzilla.
I'll land it today, please let me know if you would prefer to wait for tomorrow.

> I have doubts about adding video_rtp_depacketizer_vp9.cc in the build. Other
> than that I think with these modifications the update is ready to merge.

It is now needed in Cocoa ports since VP9 support is runtime enabled.
GTK/WPE ports should think about what the plan should be for VP9.

Adding VP9 support should be controlled by the WebRTC encoder and decoder factories.
See LibWebRTCProvider::createDecoderFactory() for instance.
Comment 28 Víctor M. Jáquez L. 2020-11-10 01:27:05 PST
(In reply to youenn fablet from comment #27)
> (In reply to Víctor M. Jáquez L. from comment #26)
> > Created attachment 413673 [details]
> > Changes on wpe/gtk for M89 update
> > 
> > With this patch, the build  compiles cleanly, though several tests became
> > broken. I'll review them one by one.
> 
> Great, thanks!
> Let me know if you need help for the tests.
> 
> I'll merge the patch today and reupload in bugzilla.
> I'll land it today, please let me know if you would prefer to wait for
> tomorrow.

It's OK today. I don't want to slow you down.

> 
> > I have doubts about adding video_rtp_depacketizer_vp9.cc in the build. Other
> > than that I think with these modifications the update is ready to merge.
> 
> It is now needed in Cocoa ports since VP9 support is runtime enabled.
> GTK/WPE ports should think about what the plan should be for VP9.
> 
> Adding VP9 support should be controlled by the WebRTC encoder and decoder
> factories.
> See LibWebRTCProvider::createDecoderFactory() for instance.
Comment 29 youenn fablet 2020-11-10 06:16:35 PST
Created attachment 413689 [details]
Patch
Comment 30 youenn fablet 2020-11-10 06:46:32 PST
Created attachment 413691 [details]
Patch
Comment 31 youenn fablet 2020-11-10 07:08:16 PST
(In reply to youenn fablet from comment #30)
> Created attachment 413691 [details]
> Patch

Victor, I removed rtp_generator.cc from CMakeLists.txt.
I believe this is ok since this is a tool only.
Comment 32 EWS 2020-11-10 12:50:40 PST
Committed r269642: <https://trac.webkit.org/changeset/269642>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 413691 [details].