Summary: | Update WebRTC libwebrtc to M87 | ||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | youenn fablet <youennf> | ||||||||||||||||||||
Component: | WebRTC | Assignee: | youenn fablet <youennf> | ||||||||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||||||||
Severity: | Normal | CC: | annulen, eric.carlson, ews-watchlist, glenn, gyuyoung.kim, hta, jer.noble, philipj, pnormand, ryuan.choi, sergio, tommyw, vjaquez, webkit-bug-importer, youennf | ||||||||||||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||||||||||||
Version: | WebKit Local Build | ||||||||||||||||||||||
Hardware: | Unspecified | ||||||||||||||||||||||
OS: | Unspecified | ||||||||||||||||||||||
See Also: | https://bugs.webkit.org/show_bug.cgi?id=218787 | ||||||||||||||||||||||
Attachments: |
|
Description
youenn fablet
2020-11-02 03:44:24 PST
Created attachment 413153 [details]
Patch
Created attachment 413180 [details]
Patch
Created attachment 413185 [details]
Patch
Created attachment 413197 [details]
Patch
rs=me once the bots are happy Created attachment 413265 [details]
Patch
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 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. (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 s/read/readd It seems it is missing the following changes: https://bugs.webkit.org/show_bug.cgi?id=214806 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. (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. (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. (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 (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. Best thing might be to wrap rtp_format_h265.h with #ifndef DISABLE_H265 (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. (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 (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. It could be ported to use VideoFrameBufferPool https://webrtc-review.googlesource.com/c/src/+/184510 (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 :( 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. 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.
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.
(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. (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. Created attachment 413689 [details]
Patch
Created attachment 413691 [details]
Patch
(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. Committed r269642: <https://trac.webkit.org/changeset/269642> All reviewed patches have been landed. Closing bug and clearing flags on attachment 413691 [details]. |