Bug 167256

Summary:

[WebRTC] Update build system to make G711 optional in libwebrtc

Product:

WebKit

Reporter:

youenn fablet <youennf>

Component:

WebCore Misc.

Assignee:

youenn fablet <youennf>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

achristensen, agouaillard, commit-queue

Priority:

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
Patch	none
Patch	none
Patch	none

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	Details Formatted Diff Diff
Patch (37.85 KB, patch) 2017-01-20 14:41 PST, youenn fablet	no flags	Details Formatted Diff Diff
Patch (5.28 KB, patch) 2017-01-20 18:01 PST, youenn fablet	no flags	Details Formatted Diff Diff
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.