Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(28)

Issue 1728453002: Enable H.264 video WebRTC behind run-time flag and add WebRtcBrowserTest for H.264 (Closed)

Created:
4 years, 10 months ago by hbos_chromium
Modified:
4 years, 10 months ago
Reviewers:
CC:
chromium-reviews, mlamouri+watch-content_chromium.org, posciak+watch_chromium.org, phoglund+watch_chromium.org, jam, mcasas+watch_chromium.org, feature-media-reviews_chromium.org, darin-cc_chromium.org, mkwst+moarreviews-renderer_chromium.org, tnakamura+watch_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Re-upload of https://codereview.chromium.org/1718883002/ but using #defines instead of BUILDFLAG to avoid problem with generated header file described in comment #10: https://codereview.chromium.org/1718883002/#msg10. - This is Plan B. Enable H.264 video WebRTC behind run-time flag and add WebRtcBrowserTest for H.264. The compile flag |rtc_use_h264| defaults to |proprietary_codecs| which is used on some Chromium bots and in the official Chrome build. The new run-time flag |kWebRtcH264WithOpenH264FFmpeg| determines if the |rtc_use_h264| encoder/decoder will be used at runtime. The run-time flag is off by default. This CL: - Adds run-time flag and a new build target for it. It is placed in content/public/common/ because it can't/shouldn't be placed in the webrtc repo and peer_connection_dependency_factory.cc lives in content/renderer/media/webrtc/. - Removed common_features (previously renderer_features) and replaced its BUILDFLAG stuff with a #defines macro. - Based on the run-time flag, enables or disables H.264 in peer_connection_dependency_factory.cc. - If flag is on, runs a new H.264 browser test. BUG=chromium:500605, chromium:468365

Patch Set 1 : Re-upload of Patch Set 7 from CL 1718883002 #

Total comments: 2

Patch Set 2 : #define macro instead of BUILDFLAG. common_features (aka renderer_features) deleted. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+141 lines, -45 lines) Patch
M chrome/browser/media/chrome_webrtc_browsertest.cc View 1 2 chunks +17 lines, -0 lines 0 comments Download
M chrome/chrome_tests.gypi View 1 3 chunks +10 lines, -1 line 0 comments Download
M chrome/test/BUILD.gn View 1 3 chunks +7 lines, -1 line 0 comments Download
M content/content.gyp View 1 3 chunks +12 lines, -17 lines 0 comments Download
M content/content_common.gypi View 1 1 chunk +10 lines, -0 lines 0 comments Download
M content/content_renderer.gypi View 1 2 chunks +11 lines, -1 line 0 comments Download
M content/public/common/BUILD.gn View 1 2 chunks +11 lines, -0 lines 0 comments Download
A content/public/common/feature_h264_with_openh264_ffmpeg.h View 1 1 chunk +21 lines, -0 lines 0 comments Download
A content/public/common/feature_h264_with_openh264_ffmpeg.cc View 1 1 chunk +17 lines, -0 lines 0 comments Download
M content/public/renderer/BUILD.gn View 1 3 chunks +9 lines, -1 line 0 comments Download
M content/renderer/BUILD.gn View 1 5 chunks +4 lines, -8 lines 0 comments Download
M content/renderer/media/webrtc/peer_connection_dependency_factory.cc View 1 3 chunks +12 lines, -16 lines 0 comments Download

Messages

Total messages: 5 (3 generated)
hbos_chromium
https://codereview.chromium.org/1728453002/diff/1/content/content.gyp File content/content.gyp (right): https://codereview.chromium.org/1728453002/diff/1/content/content.gyp#newcode281 content/content.gyp:281: 'common_features', The corresponding define is pulled in from content_renderer.gypi. ...
4 years, 10 months ago (2016-02-23 11:45:21 UTC) #4
hbos_chromium
4 years, 10 months ago (2016-02-26 08:27:45 UTC) #5
"Plan A" CL (https://codereview.chromium.org/1718883002/) landed without getting
reverted, closing this one.

Powered by Google App Engine
This is Rietveld 408576698