Bug 179076 - Use VCP H264 encoder for platforms supporting it
Summary: Use VCP H264 encoder for platforms supporting it
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebRTC (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: youenn fablet
URL:
Keywords: InRadar
Depends on: 179274 179335
Blocks:
  Show dependency treegraph
 
Reported: 2017-10-31 13:59 PDT by youenn fablet
Modified: 2017-12-14 20:25 PST (History)
7 users (show)

See Also:


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

Note You need to log in before you can comment on or make changes to this bug.
Description youenn fablet 2017-10-31 13:59:54 PDT
This will be more efficient
Comment 1 youenn fablet 2017-10-31 14:01:27 PDT
Created attachment 325487 [details]
Patch
Comment 2 Eric Carlson 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.
Comment 3 youenn fablet 2017-11-02 15:06:37 PDT
Created attachment 325773 [details]
Patch
Comment 4 Jon Lee 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.
Comment 5 Jon Lee 2017-11-02 17:06:34 PDT
rdar://problem/35180773
Comment 6 youenn fablet 2017-11-02 17:38:55 PDT
Created attachment 325803 [details]
Patch
Comment 7 youenn fablet 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.
Comment 8 youenn fablet 2017-11-03 09:49:59 PDT
Created attachment 325908 [details]
Patch
Comment 9 youenn fablet 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.
Comment 10 Eric Carlson 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.
Comment 11 youenn fablet 2017-11-03 13:53:19 PDT
Created attachment 325956 [details]
Patch for landing
Comment 12 youenn fablet 2017-11-03 13:53:53 PDT
Thanks for the review.
Patch updated accordingly.
Comment 13 WebKit Commit Bot 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>
Comment 14 WebKit Commit Bot 2017-11-03 14:13:41 PDT
All reviewed patches have been landed.  Closing bug.
Comment 15 youenn fablet 2017-11-03 14:47:40 PDT
Reopening to attach new patch.
Comment 16 youenn fablet 2017-11-03 14:47:40 PDT
Created attachment 325966 [details]
Build fix
Comment 17 WebKit Commit Bot 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>
Comment 18 WebKit Commit Bot 2017-11-03 15:03:36 PDT
All reviewed patches have been landed.  Closing bug.
Comment 19 youenn fablet 2017-11-03 15:37:15 PDT
Reopening to attach new patch.
Comment 20 youenn fablet 2017-11-03 15:37:16 PDT
Created attachment 325974 [details]
Build fix
Comment 21 youenn fablet 2017-11-03 15:48:19 PDT
Created attachment 325977 [details]
Build fix
Comment 22 WebKit Commit Bot 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>
Comment 23 WebKit Commit Bot 2017-11-03 16:21:52 PDT
All reviewed patches have been landed.  Closing bug.
Comment 24 WebKit Commit Bot 2017-11-03 16:30:17 PDT
Re-opened since this is blocked by bug 179274
Comment 25 youenn fablet 2017-11-03 17:18:36 PDT
Created attachment 325992 [details]
Trying to fix build again
Comment 26 WebKit Commit Bot 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>
Comment 27 WebKit Commit Bot 2017-11-06 09:48:50 PST
All reviewed patches have been landed.  Closing bug.
Comment 28 WebKit Commit Bot 2017-11-06 11:45:57 PST
Re-opened since this is blocked by bug 179335
Comment 29 youenn fablet 2017-12-11 13:16:38 PST
Created attachment 329025 [details]
Patch
Comment 30 youenn fablet 2017-12-11 14:07:58 PST
Created attachment 329032 [details]
Patch
Comment 31 youenn fablet 2017-12-11 15:03:54 PST
Created attachment 329040 [details]
Patch
Comment 32 WebKit Commit Bot 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>
Comment 33 WebKit Commit Bot 2017-12-11 15:37:44 PST
All reviewed patches have been landed.  Closing bug.
Comment 34 Matt Lewis 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