RESOLVED FIXED 167256
[WebRTC] Update build system to make G711 optional in libwebrtc 
https://bugs.webkit.org/show_bug.cgi?id=167256
Summary [WebRTC] Update build system to make G711 optional in libwebrtc
youenn fablet
Reported 2017-01-20 14:18:26 PST
[WebRTC] Update build system to make G711 optional in libwebrtc
Attachments
Patch (46.07 KB, patch)
2017-01-20 14:34 PST, youenn fablet 		no flags
DetailsFormatted DiffDiff
Patch (37.85 KB, patch)
2017-01-20 14:41 PST, youenn fablet 		no flags
DetailsFormatted DiffDiff
Patch (5.28 KB, patch)
2017-01-20 18:01 PST, youenn fablet 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
youenn fablet
Comment 1 2017-01-20 14:34:13 PST
Created attachment 299386 [details] Patch
WebKit Commit Bot
Comment 2 2017-01-20 14:37:32 PST
Attachment 299386 [details] did not pass style-queue: ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/modules/video_coding/codecs/vp8/include/vp8.h:22: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/modules/video_coding/codecs/vp8/include/vp8.h:30: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Total errors found: 2 in 9 files If any of these errors are false positives, please file a bug against check-webkit-style.
youenn fablet
Comment 3 2017-01-20 14:41:06 PST
Created attachment 299390 [details] Patch
Alex Christensen
Comment 4 2017-01-20 17:45:42 PST
Comment on attachment 299390 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=299390&action=review This patch looks like we are adding support for g711. It shouldn't. > Source/ThirdParty/libwebrtc/CMakeLists.txt:128 > +set_target_properties(WebrtcModulesAudio_CodingG711 PROPERTIES LINK_FLAGS "-stdlib=libc++ -arch x86_64 -isysroot /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.12.sdk -mmacosx-version-min=10.9") shouldn't hard-code path.
youenn fablet
Comment 5 2017-01-20 18:01:59 PST
Created attachment 299415 [details] Patch
WebKit Commit Bot
Comment 6 2017-01-20 18:37:16 PST
Comment on attachment 299415 [details] Patch Clearing flags on attachment: 299415 Committed r211009: <http://trac.webkit.org/changeset/211009>
WebKit Commit Bot
Comment 7 2017-01-20 18:37:20 PST
All reviewed patches have been landed. Closing bug.
Alex. Gouaillard
Comment 8 2017-01-20 19:12:25 PST
The title is ambiguous, G711 is Mandatory to Implement in WebRTC, are you planning NOT to support it, or are you just planning to support it a different way than the one libwebrtc is?
youenn fablet
Comment 9 2017-01-20 20:54:52 PST
(In reply to comment #8) > The title is ambiguous, G711 is Mandatory to Implement in WebRTC, are you > planning NOT to support it, or are you just planning to support it a > different way than the one libwebrtc is? It would be nice to support it. The direction is still unclear though.
Alex. Gouaillard
Comment 10 2017-01-20 21:11:30 PST
OK. 1. It's impossible to be compliant with the IETF specs without it. 2. A lot of mobile solutions/SDKs shy away from Opus because of resource constraints and fall back to G711 for webRTC compliance / interoperability. Let's wait and see what the direction will be ;-)
Note You need to log in before you can comment on or make changes to this bug.