Bug 179076

Summary:

Use VCP H264 encoder for platforms supporting it

Product:

WebKit

Reporter:

youenn fablet <youennf>

Component:

WebRTC

Assignee:

youenn fablet <youennf>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

commit-queue, eric.carlson, jlewis3, jonlee, ryanhaddad, webkit-bug-importer, youennf

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

See Also:

https://bugs.webkit.org/show_bug.cgi?id=180858

Bug Depends on:

179274, 179335

Bug Blocks:

Attachments:

Description	Flags
Patch	none
Patch	none
Patch	none
Patch	none
Patch for landing	none
Build fix	none
Build fix	none
Build fix	none
Trying to fix build again	none
Patch	none
Patch	none
Patch	none

youenn fablet

Reported 2017-10-31 13:59:54 PDT

This will be more efficient

Attachments
Patch (67.42 KB, patch) 2017-10-31 14:01 PDT, youenn fablet	no flags	Details Formatted Diff Diff
Patch (59.07 KB, patch) 2017-11-02 15:06 PDT, youenn fablet	no flags	Details Formatted Diff Diff
Patch (260.22 KB, patch) 2017-11-02 17:38 PDT, youenn fablet	no flags	Details Formatted Diff Diff
Patch (260.91 KB, patch) 2017-11-03 09:49 PDT, youenn fablet	no flags	Details Formatted Diff Diff
Patch for landing (260.98 KB, patch) 2017-11-03 13:53 PDT, youenn fablet	no flags	Details Formatted Diff Diff
Build fix (2.29 KB, patch) 2017-11-03 14:47 PDT, youenn fablet	no flags	Details Formatted Diff Diff
Build fix (2.10 KB, patch) 2017-11-03 15:37 PDT, youenn fablet	no flags	Details Formatted Diff Diff
Build fix (2.27 KB, patch) 2017-11-03 15:48 PDT, youenn fablet	no flags	Details Formatted Diff Diff
Trying to fix build again (260.77 KB, patch) 2017-11-03 17:18 PDT, youenn fablet	no flags	Details Formatted Diff Diff
Patch (260.75 KB, patch) 2017-12-11 13:16 PST, youenn fablet	no flags	Details Formatted Diff Diff
Patch (260.59 KB, patch) 2017-12-11 14:07 PST, youenn fablet	no flags	Details Formatted Diff Diff
Patch (260.70 KB, patch) 2017-12-11 15:03 PST, youenn fablet	no flags	Details Formatted Diff Diff
Show Obsolete (11) View All Add attachment proposed patch, testcase, etc.

youenn fablet

Comment 1 2017-10-31 14:01:27 PDT

Created attachment 325487 [details] Patch

Eric Carlson

Comment 2 2017-11-01 09:50:38 PDT

Comment on attachment 325487 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=325487&action=review > Source/ThirdParty/libwebrtc/Configurations/libwebrtc.xcconfig:33 > + Nit: extra blank line > Source/ThirdParty/libwebrtc/Source/webrtc/sdk/objc/Framework/Classes/VideoProcessing/encoder_vcp.h:8 > + * Copyright (c) 2015 The WebRTC project authors. All Rights Reserved. > + * > + * Use of this source code is governed by a BSD-style license > + * that can be found in the LICENSE file in the root of the source > + * tree. An additional intellectual property rights grant can be found > + * in the file PATENTS. All contributing project authors may > + * be found in the AUTHORS file in the root of the source tree. Is this the correct header? > Source/ThirdParty/libwebrtc/Source/webrtc/sdk/objc/Framework/Classes/VideoProcessing/encoder_vcp.h:43 > +// This file provides a H264 encoder implementation using the VideoToolbox > +// APIs. Since documentation is almost non-existent, this is largely based on > +// the information in the VideoToolbox header files, a talk from WWDC 2014 and > +// experimentation. This is incorrect.

youenn fablet

Comment 3 2017-11-02 15:06:37 PDT

Created attachment 325773 [details] Patch

Jon Lee

Comment 4 2017-11-02 17:06:20 PDT

Comment on attachment 325773 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=325773&action=review > Source/ThirdParty/libwebrtc/ChangeLog:4 > + https://bugs.webkit.org/show_bug.cgi?id=179076 please add reference to radar.

Jon Lee

Comment 5 2017-11-02 17:06:34 PDT

rdar://problem/35180773

youenn fablet

Comment 6 2017-11-02 17:38:55 PDT

Created attachment 325803 [details] Patch

youenn fablet

Comment 7 2017-11-02 17:39:26 PDT

(In reply to youenn fablet from comment #6) > Created attachment 325803 [details] > Patch Fixing macOS version. Need to handle iOS sim/iOS missing private framework.

youenn fablet

Comment 8 2017-11-03 09:49:59 PDT

Created attachment 325908 [details] Patch

youenn fablet

Comment 9 2017-11-03 10:46:03 PDT

Comment on attachment 325908 [details] Patch Patch only activates VCP for MacOS right now. It should be extended to iOS at some point.

Eric Carlson

Comment 10 2017-11-03 12:05:55 PDT

Comment on attachment 325908 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=325908&action=review > Source/ThirdParty/libwebrtc/Source/webrtc/sdk/objc/Framework/Classes/VideoProcessing/encoder_vcp.h:3 > + * Copyright (c) 2017 Apple INC. All Rights Reserved. Nit: Apple INC => Apple Inc > Source/ThirdParty/libwebrtc/Source/webrtc/sdk/objc/Framework/Classes/VideoProcessing/encoder_vcp.mm:3 > + * Copyright (c) 2017 Apple INC. All Rights Reserved. Ditto.

youenn fablet

Comment 11 2017-11-03 13:53:19 PDT

Created attachment 325956 [details] Patch for landing

youenn fablet

Comment 12 2017-11-03 13:53:53 PDT

Thanks for the review. Patch updated accordingly.

WebKit Commit Bot

Comment 13 2017-11-03 14:13:39 PDT

Comment on attachment 325956 [details] Patch for landing Clearing flags on attachment: 325956 Committed r224428: <https://trac.webkit.org/changeset/224428>

WebKit Commit Bot

Comment 14 2017-11-03 14:13:41 PDT

All reviewed patches have been landed. Closing bug.

youenn fablet

Comment 15 2017-11-03 14:47:40 PDT

Reopening to attach new patch.

youenn fablet

Comment 16 2017-11-03 14:47:40 PDT

Created attachment 325966 [details] Build fix

WebKit Commit Bot

Comment 17 2017-11-03 15:03:34 PDT

Comment on attachment 325966 [details] Build fix Clearing flags on attachment: 325966 Committed r224435: <https://trac.webkit.org/changeset/224435>

WebKit Commit Bot

Comment 18 2017-11-03 15:03:36 PDT

All reviewed patches have been landed. Closing bug.

youenn fablet

Comment 19 2017-11-03 15:37:15 PDT

Reopening to attach new patch.

youenn fablet

Comment 20 2017-11-03 15:37:16 PDT

Created attachment 325974 [details] Build fix

youenn fablet

Comment 21 2017-11-03 15:48:19 PDT

Created attachment 325977 [details] Build fix

WebKit Commit Bot

Comment 22 2017-11-03 16:21:51 PDT

Comment on attachment 325977 [details] Build fix Clearing flags on attachment: 325977 Committed r224440: <https://trac.webkit.org/changeset/224440>

WebKit Commit Bot

Comment 23 2017-11-03 16:21:52 PDT

All reviewed patches have been landed. Closing bug.

WebKit Commit Bot

Comment 24 2017-11-03 16:30:17 PDT

Re-opened since this is blocked by bug 179274

youenn fablet

Comment 25 2017-11-03 17:18:36 PDT

Created attachment 325992 [details] Trying to fix build again

WebKit Commit Bot

Comment 26 2017-11-06 09:48:49 PST

Comment on attachment 325992 [details] Trying to fix build again Clearing flags on attachment: 325992 Committed r224497: <https://trac.webkit.org/changeset/224497>

WebKit Commit Bot

Comment 27 2017-11-06 09:48:50 PST

All reviewed patches have been landed. Closing bug.

WebKit Commit Bot

Comment 28 2017-11-06 11:45:57 PST

Re-opened since this is blocked by bug 179335

youenn fablet

Comment 29 2017-12-11 13:16:38 PST

Created attachment 329025 [details] Patch

youenn fablet

Comment 30 2017-12-11 14:07:58 PST

Created attachment 329032 [details] Patch

youenn fablet

Comment 31 2017-12-11 15:03:54 PST

Created attachment 329040 [details] Patch

WebKit Commit Bot

Comment 32 2017-12-11 15:37:42 PST

Comment on attachment 329040 [details] Patch Clearing flags on attachment: 329040 Committed r225761: <https://trac.webkit.org/changeset/225761>

WebKit Commit Bot

Comment 33 2017-12-11 15:37:44 PST

All reviewed patches have been landed. Closing bug.

Matt Lewis

Comment 34 2017-12-12 10:47:04 PST

This caused an API failure with test ContentFiltering.LazilyLoadPlatformFrameworks https://build.webkit.org/builders/Apple%20High%20Sierra%20Release%20WK2%20%28Tests%29/builds/1689 https://build.webkit.org/builders/Apple%20High%20Sierra%20Release%20WK2%20%28Tests%29/builds/1689/steps/run-api-tests/logs/stdio Failure: FAIL ContentFiltering.LazilyLoadPlatformFrameworks /Volumes/Data/slave/highsierra-release/build/Tools/TestWebKitAPI/Tests/WebKitCocoa/ContentFiltering.mm:366 Value of: static_cast<bool>(networkExtensionLoaded) Actual: true Expected: static_cast<bool>(networkExtensionShouldBeLoaded) Which is: false /Volumes/Data/slave/highsierra-release/build/Tools/TestWebKitAPI/Tests/WebKitCocoa/ContentFiltering.mm:366 Value of: static_cast<bool>(networkExtensionLoaded) Actual: true Expected: static_cast<bool>(networkExtensionShouldBeLoaded) Which is: false /Volumes/Data/slave/highsierra-release/build/Tools/TestWebKitAPI/Tests/WebKitCocoa/ContentFiltering.mm:366 Value of: static_cast<bool>(networkExtensionLoaded) Actual: true Expected: static_cast<bool>(networkExtensionShouldBeLoaded) Which is: false /Volumes/Data/slave/highsierra-release/build/Tools/TestWebKitAPI/Tests/WebKitCocoa/ContentFiltering.mm:366 Value of: static_cast<bool>(networkExtensionLoaded) Actual: true Expected: static_cast<bool>(networkExtensionShouldBeLoaded) Which is: false /Volumes/Data/slave/highsierra-release/build/Tools/TestWebKitAPI/Tests/WebKitCocoa/ContentFiltering.mm:366 Value of: static_cast<bool>(networkExtensionLoaded) Actual: true Expected: static_cast<bool>(networkExtensionShouldBeLoaded) Which is: false

Note You need to log in before you can comment on or make changes to this bug.