Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1886363002:
Reland: Tie kWebRtcH264WithOpenH264FFmpeg and kEnableWebRtcHWH264Encoding flags on Mac (Closed)
|
Created:
4 years, 8 months ago by emircan Modified:
4 years, 8 months ago CC:
chromium-reviews, darin-cc_chromium.org, feature-media-reviews_chromium.org, jam, mkwst+moarreviews-renderer_chromium.org, mlamouri+watch-content_chromium.org, posciak+watch_chromium.org Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionReland: Tie kWebRtcH264WithOpenH264FFmpeg and kEnableWebRtcHWH264Encoding flags on Mac
Additional to the original, this CL checks for >=OSX10.9
in content/common/gpu/media/vt_video_encode_accelerator_mac.cc
since VideoToolbox HW features are introduced there.
The original CL got reverted because of a failure on a bot running 10.8.5.
https://developer.apple.com/library/mac/releasenotes/General/APIDiffsMacOSX10_9/VideoToolbox.html
Original CL description:
https://codereview.chromium.org/1852523002/
This CL enables using VideoToolbox encoder when
kWebRtcH264WithOpenH264FFmpeg flag is on.
BUG=597334
TBR=mcasas@chromium.org, hbos@chromium.org
Committed: https://crrev.com/3e6e8e36cbd14975a4ff345dd153b24db6e4c444
Cr-Commit-Position: refs/heads/master@{#388611}
Patch Set 1 : #Patch Set 2 : Change to DLOGs. #
Total comments: 1
Messages
Total messages: 27 (18 generated)
Description was changed from ========== new flags tied. BUG= ========== to ========== Reland: Tie kWebRtcH264WithOpenH264FFmpeg and kEnableWebRtcHWH264Encoding flags on Mac This CL enables using VideoToolbox encoder when kWebRtcH264WithOpenH264FFmpeg flag is on. BUG=597334 ==========
Patchset #1 (id:1) has been deleted
Patchset #1 (id:20001) has been deleted
Description was changed from ========== Reland: Tie kWebRtcH264WithOpenH264FFmpeg and kEnableWebRtcHWH264Encoding flags on Mac This CL enables using VideoToolbox encoder when kWebRtcH264WithOpenH264FFmpeg flag is on. BUG=597334 ========== to ========== Reland: Tie kWebRtcH264WithOpenH264FFmpeg and kEnableWebRtcHWH264Encoding flags on Mac This CL enables using VideoToolbox encoder when kWebRtcH264WithOpenH264FFmpeg flag is on. Additionally, this CL checks for >=OSX10.9 since VideoToolbox HW features are introduced there. https://developer.apple.com/library/mac/releasenotes/General/APIDiffsMacOSX10... BUG=597334 ==========
emircan@chromium.org changed reviewers: + hbos@chromium.org, mcasas@chromium.org, pbos@chromium.org
PTAL.
Description was changed from ========== Reland: Tie kWebRtcH264WithOpenH264FFmpeg and kEnableWebRtcHWH264Encoding flags on Mac This CL enables using VideoToolbox encoder when kWebRtcH264WithOpenH264FFmpeg flag is on. Additionally, this CL checks for >=OSX10.9 since VideoToolbox HW features are introduced there. https://developer.apple.com/library/mac/releasenotes/General/APIDiffsMacOSX10... BUG=597334 ========== to ========== Reland: Tie kWebRtcH264WithOpenH264FFmpeg and kEnableWebRtcHWH264Encoding flags on Mac Additional to the original, this CL checks for >=OSX10.9 since VideoToolbox HW features are introduced there. https://developer.apple.com/library/mac/releasenotes/General/APIDiffsMacOSX10... OriginalCL description: This CL enables using VideoToolbox encoder when kWebRtcH264WithOpenH264FFmpeg flag is on. BUG=597334 ==========
Description was changed from ========== Reland: Tie kWebRtcH264WithOpenH264FFmpeg and kEnableWebRtcHWH264Encoding flags on Mac Additional to the original, this CL checks for >=OSX10.9 since VideoToolbox HW features are introduced there. https://developer.apple.com/library/mac/releasenotes/General/APIDiffsMacOSX10... OriginalCL description: This CL enables using VideoToolbox encoder when kWebRtcH264WithOpenH264FFmpeg flag is on. BUG=597334 ========== to ========== Reland: Tie kWebRtcH264WithOpenH264FFmpeg and kEnableWebRtcHWH264Encoding flags on Mac Additional to the original, this CL checks for >=OSX10.9 since VideoToolbox HW features are introduced there. https://developer.apple.com/library/mac/releasenotes/General/APIDiffsMacOSX10... Original CL description: This CL enables using VideoToolbox encoder when kWebRtcH264WithOpenH264FFmpeg flag is on. BUG=597334 ==========
emircan@chromium.org changed reviewers: - pbos@chromium.org
Description was changed from ========== Reland: Tie kWebRtcH264WithOpenH264FFmpeg and kEnableWebRtcHWH264Encoding flags on Mac Additional to the original, this CL checks for >=OSX10.9 since VideoToolbox HW features are introduced there. https://developer.apple.com/library/mac/releasenotes/General/APIDiffsMacOSX10... Original CL description: This CL enables using VideoToolbox encoder when kWebRtcH264WithOpenH264FFmpeg flag is on. BUG=597334 ========== to ========== Reland: Tie kWebRtcH264WithOpenH264FFmpeg and kEnableWebRtcHWH264Encoding flags on Mac Additional to the original, this CL checks for >=OSX10.9 since VideoToolbox HW features are introduced there. https://developer.apple.com/library/mac/releasenotes/General/APIDiffsMacOSX10... Original CL description: https://codereview.chromium.org/1852523002/ This CL enables using VideoToolbox encoder when kWebRtcH264WithOpenH264FFmpeg flag is on. BUG=597334 ==========
emircan@chromium.org changed reviewers: + dalecurtis@chromium.org
dalecurtis@, can you review the changes in content/common/gpu/media/vt_video_encode_accelerator_mac.cc ?
dalecurtis@chromium.org changed reviewers: + sandersd@chromium.org - dalecurtis@chromium.org
=>sandersd
Description was changed from ========== Reland: Tie kWebRtcH264WithOpenH264FFmpeg and kEnableWebRtcHWH264Encoding flags on Mac Additional to the original, this CL checks for >=OSX10.9 since VideoToolbox HW features are introduced there. https://developer.apple.com/library/mac/releasenotes/General/APIDiffsMacOSX10... Original CL description: https://codereview.chromium.org/1852523002/ This CL enables using VideoToolbox encoder when kWebRtcH264WithOpenH264FFmpeg flag is on. BUG=597334 ========== to ========== Reland: Tie kWebRtcH264WithOpenH264FFmpeg and kEnableWebRtcHWH264Encoding flags on Mac Additional to the original, this CL checks for >=OSX10.9 in content/common/gpu/media/vt_video_encode_accelerator_mac.cc since VideoToolbox HW features are introduced there. https://developer.apple.com/library/mac/releasenotes/General/APIDiffsMacOSX10... Original CL description: https://codereview.chromium.org/1852523002/ This CL enables using VideoToolbox encoder when kWebRtcH264WithOpenH264FFmpeg flag is on. BUG=597334 ==========
Description was changed from ========== Reland: Tie kWebRtcH264WithOpenH264FFmpeg and kEnableWebRtcHWH264Encoding flags on Mac Additional to the original, this CL checks for >=OSX10.9 in content/common/gpu/media/vt_video_encode_accelerator_mac.cc since VideoToolbox HW features are introduced there. https://developer.apple.com/library/mac/releasenotes/General/APIDiffsMacOSX10... Original CL description: https://codereview.chromium.org/1852523002/ This CL enables using VideoToolbox encoder when kWebRtcH264WithOpenH264FFmpeg flag is on. BUG=597334 ========== to ========== Reland: Tie kWebRtcH264WithOpenH264FFmpeg and kEnableWebRtcHWH264Encoding flags on Mac Additional to the original, this CL checks for >=OSX10.9 in content/common/gpu/media/vt_video_encode_accelerator_mac.cc since VideoToolbox HW features are introduced there. The original CL got reverted because of a failure on a bot running 10.8.5. https://developer.apple.com/library/mac/releasenotes/General/APIDiffsMacOSX10... Original CL description: https://codereview.chromium.org/1852523002/ This CL enables using VideoToolbox encoder when kWebRtcH264WithOpenH264FFmpeg flag is on. BUG=597334 ==========
lgtm
Description was changed from ========== Reland: Tie kWebRtcH264WithOpenH264FFmpeg and kEnableWebRtcHWH264Encoding flags on Mac Additional to the original, this CL checks for >=OSX10.9 in content/common/gpu/media/vt_video_encode_accelerator_mac.cc since VideoToolbox HW features are introduced there. The original CL got reverted because of a failure on a bot running 10.8.5. https://developer.apple.com/library/mac/releasenotes/General/APIDiffsMacOSX10... Original CL description: https://codereview.chromium.org/1852523002/ This CL enables using VideoToolbox encoder when kWebRtcH264WithOpenH264FFmpeg flag is on. BUG=597334 ========== to ========== Reland: Tie kWebRtcH264WithOpenH264FFmpeg and kEnableWebRtcHWH264Encoding flags on Mac Additional to the original, this CL checks for >=OSX10.9 in content/common/gpu/media/vt_video_encode_accelerator_mac.cc since VideoToolbox HW features are introduced there. The original CL got reverted because of a failure on a bot running 10.8.5. https://developer.apple.com/library/mac/releasenotes/General/APIDiffsMacOSX10... Original CL description: https://codereview.chromium.org/1852523002/ This CL enables using VideoToolbox encoder when kWebRtcH264WithOpenH264FFmpeg flag is on. BUG=597334 TBR=mcasas@chromium.org, hbos@chromium.org ==========
On 2016/04/20 21:39:02, sandersd wrote: > lgtm Assuming the only new code is that in content/common/gpu/media/vt_video_encode_accelerator_mac.cc, RS LGTM
The CQ bit was checked by emircan@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1886363002/60001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1886363002/60001
Message was sent while issue was closed.
Description was changed from ========== Reland: Tie kWebRtcH264WithOpenH264FFmpeg and kEnableWebRtcHWH264Encoding flags on Mac Additional to the original, this CL checks for >=OSX10.9 in content/common/gpu/media/vt_video_encode_accelerator_mac.cc since VideoToolbox HW features are introduced there. The original CL got reverted because of a failure on a bot running 10.8.5. https://developer.apple.com/library/mac/releasenotes/General/APIDiffsMacOSX10... Original CL description: https://codereview.chromium.org/1852523002/ This CL enables using VideoToolbox encoder when kWebRtcH264WithOpenH264FFmpeg flag is on. BUG=597334 TBR=mcasas@chromium.org, hbos@chromium.org ========== to ========== Reland: Tie kWebRtcH264WithOpenH264FFmpeg and kEnableWebRtcHWH264Encoding flags on Mac Additional to the original, this CL checks for >=OSX10.9 in content/common/gpu/media/vt_video_encode_accelerator_mac.cc since VideoToolbox HW features are introduced there. The original CL got reverted because of a failure on a bot running 10.8.5. https://developer.apple.com/library/mac/releasenotes/General/APIDiffsMacOSX10... Original CL description: https://codereview.chromium.org/1852523002/ This CL enables using VideoToolbox encoder when kWebRtcH264WithOpenH264FFmpeg flag is on. BUG=597334 TBR=mcasas@chromium.org, hbos@chromium.org ==========
Message was sent while issue was closed.
Committed patchset #2 (id:60001)
Message was sent while issue was closed.
Description was changed from ========== Reland: Tie kWebRtcH264WithOpenH264FFmpeg and kEnableWebRtcHWH264Encoding flags on Mac Additional to the original, this CL checks for >=OSX10.9 in content/common/gpu/media/vt_video_encode_accelerator_mac.cc since VideoToolbox HW features are introduced there. The original CL got reverted because of a failure on a bot running 10.8.5. https://developer.apple.com/library/mac/releasenotes/General/APIDiffsMacOSX10... Original CL description: https://codereview.chromium.org/1852523002/ This CL enables using VideoToolbox encoder when kWebRtcH264WithOpenH264FFmpeg flag is on. BUG=597334 TBR=mcasas@chromium.org, hbos@chromium.org ========== to ========== Reland: Tie kWebRtcH264WithOpenH264FFmpeg and kEnableWebRtcHWH264Encoding flags on Mac Additional to the original, this CL checks for >=OSX10.9 in content/common/gpu/media/vt_video_encode_accelerator_mac.cc since VideoToolbox HW features are introduced there. The original CL got reverted because of a failure on a bot running 10.8.5. https://developer.apple.com/library/mac/releasenotes/General/APIDiffsMacOSX10... Original CL description: https://codereview.chromium.org/1852523002/ This CL enables using VideoToolbox encoder when kWebRtcH264WithOpenH264FFmpeg flag is on. BUG=597334 TBR=mcasas@chromium.org, hbos@chromium.org Committed: https://crrev.com/3e6e8e36cbd14975a4ff345dd153b24db6e4c444 Cr-Commit-Position: refs/heads/master@{#388611} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/3e6e8e36cbd14975a4ff345dd153b24db6e4c444 Cr-Commit-Position: refs/heads/master@{#388611}
Message was sent while issue was closed.
pbos@chromium.org changed reviewers: + pbos@chromium.org
Message was sent while issue was closed.
https://codereview.chromium.org/1886363002/diff/60001/content/renderer/media/... File content/renderer/media/rtc_video_encoder_factory.cc (right): https://codereview.chromium.org/1886363002/diff/60001/content/renderer/media/... content/renderer/media/rtc_video_encoder_factory.cc:42: webrtc_h264_enabled) { I don't think we should hook this into whether SW H264 is enabled or not, pushing both of these at the same time seems like it could be problematic to me. It also makes it harder to test OpenH264/FFmpeg on platforms where hardware is supported. Isn't kEnableWebRtcHWH264Encoding sufficient? Otherwise, could you add a separate flag? |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
