Bug 189976

Summary: Add VP8 support to WebRTC
Product: WebKit Reporter: youenn fablet <youennf>
Component: WebRTCAssignee: youenn fablet <youennf>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, adam, annulen, ben.browitt, commit-queue, eric.carlson, ews-watchlist, oscar, rniwa, ryanhaddad, webkit-bug-importer, youennf
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 190025    
Bug Blocks:    
Attachments:
Description Flags
patch
none
Patch
none
Patch
none
Patch
none
Patch for landing
none
Rebased patch
none
Rebasing
none
Fixing iOS
none
Archive of layout-test-results from ews106 for mac-sierra-wk2
none
Fixing iOS
none
Patch for landing none

Description youenn fablet 2018-09-25 15:30:20 PDT
Add libvpx support
Comment 1 youenn fablet 2018-09-25 15:40:38 PDT
Created attachment 350805 [details]
patch
Comment 2 youenn fablet 2018-09-25 20:46:08 PDT
Created attachment 350843 [details]
Patch
Comment 3 EWS Watchlist 2018-09-25 20:50:28 PDT
Attachment 350843 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/mediastream/libwebrtc/LibWebRTCProvider.h:112:  Inline functions should not be in classes annotated with WEBCORE_EXPORT. Remove the macro from the class and apply it to each appropriate method, or move the inline function definition out-of-line.  [build/webcore_export] [4]
Total errors found: 1 in 28 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 4 youenn fablet 2018-09-26 12:51:50 PDT
Created attachment 350885 [details]
Patch
Comment 5 EWS Watchlist 2018-09-26 12:54:24 PDT
Attachment 350885 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/mediastream/libwebrtc/LibWebRTCProvider.h:112:  Inline functions should not be in classes annotated with WEBCORE_EXPORT. Remove the macro from the class and apply it to each appropriate method, or move the inline function definition out-of-line.  [build/webcore_export] [4]
Total errors found: 1 in 28 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 6 youenn fablet 2018-09-26 13:52:43 PDT
Created attachment 350898 [details]
Patch
Comment 7 EWS Watchlist 2018-09-26 13:57:03 PDT
Attachment 350898 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/mediastream/libwebrtc/LibWebRTCProvider.h:112:  Inline functions should not be in classes annotated with WEBCORE_EXPORT. Remove the macro from the class and apply it to each appropriate method, or move the inline function definition out-of-line.  [build/webcore_export] [4]
Total errors found: 1 in 28 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 8 Alex Christensen 2018-09-27 08:13:05 PDT
Comment on attachment 350898 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=350898&action=review

> Source/ThirdParty/libwebrtc/Source/third_party/libvpx/source/config/mac/x64/vpx_config.h:66
> -#define CONFIG_VP9 1
> +#define CONFIG_VP9 0

No more VP9?

> Source/ThirdParty/libwebrtc/libwebrtc.xcodeproj/project.pbxproj:-14743
> -				41F411BF1EF8DBA500343C26 /* vp8_noop.cc in Sources */,

We could remove this file.
Comment 9 youenn fablet 2018-09-27 08:33:13 PDT
(In reply to Alex Christensen from comment #8)
> Comment on attachment 350898 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=350898&action=review
> 
> > Source/ThirdParty/libwebrtc/Source/third_party/libvpx/source/config/mac/x64/vpx_config.h:66
> > -#define CONFIG_VP9 1
> > +#define CONFIG_VP9 0
>
> No more VP9?

This vpx_config.h is the default one and we need to update it for WebKit.
 
> > Source/ThirdParty/libwebrtc/libwebrtc.xcodeproj/project.pbxproj:-14743
> > -				41F411BF1EF8DBA500343C26 /* vp8_noop.cc in Sources */,
> 
> We could remove this file.

Right.
Comment 10 Eric Carlson 2018-09-27 09:10:43 PDT
Comment on attachment 350898 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=350898&action=review

> Source/WebCore/platform/mediastream/libwebrtc/LibWebRTCProvider.h:130
> +    bool m_isSupportingVP8 { false };

Nit: I think "m_supportsVP8" would be better.
Comment 11 youenn fablet 2018-09-27 09:33:27 PDT
Created attachment 350970 [details]
Patch for landing
Comment 12 EWS Watchlist 2018-09-27 09:36:50 PDT
Attachment 350970 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/mediastream/libwebrtc/LibWebRTCProvider.h:112:  Inline functions should not be in classes annotated with WEBCORE_EXPORT. Remove the macro from the class and apply it to each appropriate method, or move the inline function definition out-of-line.  [build/webcore_export] [4]
Total errors found: 1 in 28 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 13 WebKit Commit Bot 2018-09-27 10:16:55 PDT
Comment on attachment 350970 [details]
Patch for landing

Clearing flags on attachment: 350970

Committed r236557: <https://trac.webkit.org/changeset/236557>
Comment 14 WebKit Commit Bot 2018-09-27 10:16:57 PDT
All reviewed patches have been landed.  Closing bug.
Comment 15 Radar WebKit Bug Importer 2018-09-27 10:17:26 PDT
<rdar://problem/44834363>
Comment 16 Konstantin Tokarev 2018-09-27 10:21:04 PDT
You've (I guess accidentally) committed a bunch of object files with this patch (
Source/ThirdParty/libwebrtc/Source/third_party/libvpx/source/libvpx/vp8/common/x86/copy_sse2.asm.o etc)
Comment 17 youenn fablet 2018-09-27 10:23:03 PDT
(In reply to Konstantin Tokarev from comment #16)
> You've (I guess accidentally) committed a bunch of object files with this
> patch (
> Source/ThirdParty/libwebrtc/Source/third_party/libvpx/source/libvpx/vp8/
> common/x86/copy_sse2.asm.o etc)

These asm object files will be generated by yasm dynamically as part of https://bugs.webkit.org/show_bug.cgi?id=190025
Comment 18 Ryan Haddad 2018-09-27 11:30:55 PDT
Rolled out this change in https://trac.webkit.org/r236561 because it broke internal builds.
Comment 19 youenn fablet 2018-09-27 21:25:07 PDT
Created attachment 351049 [details]
Rebased patch
Comment 20 youenn fablet 2018-09-29 11:19:42 PDT
Created attachment 351189 [details]
Rebasing
Comment 21 EWS Watchlist 2018-09-29 11:22:31 PDT
Attachment 351189 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/mediastream/libwebrtc/LibWebRTCProvider.h:112:  Inline functions should not be in classes annotated with WEBCORE_EXPORT. Remove the macro from the class and apply it to each appropriate method, or move the inline function definition out-of-line.  [build/webcore_export] [4]
Total errors found: 1 in 29 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 22 youenn fablet 2018-09-29 11:45:53 PDT
Created attachment 351191 [details]
Fixing iOS
Comment 23 EWS Watchlist 2018-09-29 11:47:54 PDT
Attachment 351191 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/mediastream/libwebrtc/LibWebRTCProvider.h:112:  Inline functions should not be in classes annotated with WEBCORE_EXPORT. Remove the macro from the class and apply it to each appropriate method, or move the inline function definition out-of-line.  [build/webcore_export] [4]
Total errors found: 1 in 29 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 24 EWS Watchlist 2018-09-29 13:06:35 PDT
Comment on attachment 351191 [details]
Fixing iOS

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

New failing tests:
media/range-extract-contents-crash.html
Comment 25 EWS Watchlist 2018-09-29 13:06:37 PDT
Created attachment 351194 [details]
Archive of layout-test-results from ews106 for mac-sierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews106  Port: mac-sierra-wk2  Platform: Mac OS X 10.12.6
Comment 26 youenn fablet 2018-09-29 13:47:19 PDT
Created attachment 351195 [details]
Fixing iOS
Comment 27 EWS Watchlist 2018-09-29 13:49:19 PDT
Attachment 351195 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/mediastream/libwebrtc/LibWebRTCProvider.h:112:  Inline functions should not be in classes annotated with WEBCORE_EXPORT. Remove the macro from the class and apply it to each appropriate method, or move the inline function definition out-of-line.  [build/webcore_export] [4]
Total errors found: 1 in 29 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 28 youenn fablet 2018-10-03 18:12:15 PDT
Created attachment 351569 [details]
Patch for landing
Comment 29 EWS Watchlist 2018-10-03 18:14:00 PDT
Attachment 351569 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/mediastream/libwebrtc/LibWebRTCProvider.h:112:  Inline functions should not be in classes annotated with WEBCORE_EXPORT. Remove the macro from the class and apply it to each appropriate method, or move the inline function definition out-of-line.  [build/webcore_export] [4]
Total errors found: 1 in 29 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 30 WebKit Commit Bot 2018-10-03 18:53:37 PDT
Comment on attachment 351569 [details]
Patch for landing

Clearing flags on attachment: 351569

Committed r236821: <https://trac.webkit.org/changeset/236821>
Comment 31 WebKit Commit Bot 2018-10-03 18:53:39 PDT
All reviewed patches have been landed.  Closing bug.