Bug 218436 – Update WebRTC libwebrtc to M87

• Component: WebRTC
• Status: RESOLVED FIXED

[youenn fablet]

Update WebRTC liwebrtc to M87

[youenn fablet]

Created attachment 413153 [details]
Patch

[youenn fablet]

Created attachment 413180 [details]
Patch

[youenn fablet]

Created attachment 413185 [details]
Patch

[youenn fablet]

Created attachment 413197 [details]
Patch

[Eric Carlson]

rs=me once the bots are happy

[youenn fablet]

Created attachment 413265 [details]
Patch

[Víctor M. Jáquez L.]

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

[Víctor M. Jáquez L.]

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.

[youenn fablet]

(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

[youenn fablet]

s/read/readd

[youenn fablet]

It seems it is missing the following changes: https://bugs.webkit.org/show_bug.cgi?id=214806

[Víctor M. Jáquez L.]

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.

[Víctor M. Jáquez L.]

(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.

[youenn fablet]

(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.

[Víctor M. Jáquez L.]

(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

[youenn fablet]

(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.

[youenn fablet]

Best thing might be to wrap rtp_format_h265.h with #ifndef DISABLE_H265

[Víctor M. Jáquez L.]

(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.

[youenn fablet]

(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

[Víctor M. Jáquez L.]

(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.

[Víctor M. Jáquez L.]

It could be ported to use VideoFrameBufferPool

https://webrtc-review.googlesource.com/c/src/+/184510

[Víctor M. Jáquez L.]

(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 :(

[Víctor M. Jáquez L.]

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.

[Víctor M. Jáquez L.]

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.

[Radar WebKit Bug Importer]

<rdar://problem/71185126>

[Víctor M. Jáquez L.]

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.

[youenn fablet]

(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.

[Víctor M. Jáquez L.]

(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.

[youenn fablet]

Created attachment 413689 [details]
Patch

[youenn fablet]

Created attachment 413691 [details]
Patch

[youenn fablet]

(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.

[EWS]

Committed r269642: <https://trac.webkit.org/changeset/269642>

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