chrome/browser/about_flags.cc - Issue 2573933002: Restore switch kDisableWebRtcHWEncoding

Unified Diff: chrome/browser/about_flags.cc

Issue 2573933002: Restore switch kDisableWebRtcHWEncoding (Closed)

Patch Set: check kDisableWebRtcHWEncoding only at creating encoder_factory Created 4 years ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

View side-by-side diff with in-line comments

« no previous file with comments | « chrome/app/generated_resources.grd ('k') | chrome/browser/chromeos/login/chrome_restart_request.cc » ('j') | content/browser/gpu/gpu_data_manager_impl_private.cc » ('J')
Expand Comments ('e') | Collapse Comments ('c') | Hide Comments ('s')

Index: chrome/browser/about_flags.cc

diff --git a/chrome/browser/about_flags.cc b/chrome/browser/about_flags.cc

index dc77e84d4d3cee3c587f53b80b3f62545c49578b..df5ec96fcc13579880b9c1f26b159a2612ea6312 100644

--- a/chrome/browser/about_flags.cc

+++ b/chrome/browser/about_flags.cc

@@ -703,11 +703,11 @@ const FeatureEntry kFeatureEntries[] = {

{"disable-webrtc-hw-decoding", IDS_FLAGS_WEBRTC_HW_DECODING_NAME,

IDS_FLAGS_WEBRTC_HW_DECODING_DESCRIPTION, kOsAndroid | kOsCrOS,

SINGLE_DISABLE_VALUE_TYPE(switches::kDisableWebRtcHWDecoding)},

- {"disable-webrtc-hw-vp8-encoding", IDS_FLAGS_WEBRTC_HW_VP8_ENCODING_NAME,

- IDS_FLAGS_WEBRTC_HW_VP8_ENCODING_DESCRIPTION, kOsAndroid | kOsCrOS,

- SINGLE_DISABLE_VALUE_TYPE(switches::kDisableWebRtcHWVP8Encoding)},

+ {"disable-webrtc-hw-encoding", IDS_FLAGS_WEBRTC_HW_ENCODING_NAME,

+ IDS_FLAGS_WEBRTC_HW_ENCODING_DESCRIPTION, kOsAndroid | kOsCrOS,

+ SINGLE_DISABLE_VALUE_TYPE(switches::kDisableWebRtcHWEncoding)},

{"enable-webrtc-hw-h264-encoding", IDS_FLAGS_WEBRTC_HW_H264_ENCODING_NAME,

- IDS_FLAGS_WEBRTC_HW_H264_ENCODING_DESCRIPTION, kOsAndroid | kOsCrOS,

+ IDS_FLAGS_WEBRTC_HW_H264_ENCODING_DESCRIPTION, kOsAndroid,

Pawel Osciak 2016/12/15 05:45:53 This flag is marked here, as well as described in

braveyao 2016/12/15 20:46:24 #enable-webrtc-hw-h264-encoding is ENABLED by defa

Pawel Osciak 2016/12/20 01:49:17 Yes, I agree with that. What I meant was that this

braveyao 2016/12/20 19:09:14 Yes I changed it back to kOsAndroid only since kDi

Pawel Osciak 2016/12/21 01:12:57 I believe before your previous change we could pas

I believe before your previous change we could pass a codec name to kDisableWebRtcHWEncoding to selectively disable codecs... I'm ok using explicit flags for this instead of a value, but it would be great if that worked not only on Android...

braveyao 2016/12/21 02:01:39 Done. I can make the H264 flag also available on

On 2016/12/21 01:12:57, Pawel Osciak wrote: > On 2016/12/20 19:09:14, braveyao wrote: > > On 2016/12/20 01:49:17, Pawel Osciak wrote: > > > On 2016/12/15 20:46:24, braveyao wrote: > > > > On 2016/12/15 05:45:53, Pawel Osciak wrote: > > > > > This flag is marked here, as well as described in content_features.cc, > as > > > only > > > > > for Android (although it is actually enabled and being used outside #if > > > > > defined(OS_ANDROID) in this CL as far as I can see). > > > > > > > > > > We need to use it outside of Android to keep H264 enabled for CrOS as > > well. > > > > > > > > > > On the other hand, I think your original intention was to disable only > > VP8, > > > so > > > > > perhaps this flag is not needed at all and you should just be passing > > > > > kDisableWebRtcHWVP8Encoding for your use case? > > > > > > > > #enable-webrtc-hw-h264-encoding is ENABLED by default on CrOS and > > Android(and > > > > other platforms). So it won't affect CrOS H264. Here we only add it in > > > > about_flag page on Android to offer a way to disable it manually. > > > > > > > > We need this feature flag. Because we need to control the percentage of > > > > deployment if anything goes very wrong. > > > > > > Yes, I agree with that. What I meant was that this change changes > "kOsAndroid > > | > > > kOsCrOS" to "kOsAndroid" only, even though it's not Android-only. > > > > > > As for the deployment switch, if you needed to disable H264 encode, would > you > > > still want to use VP8 HW encode? If not, you could use > > kDisableWebRtcHWEncoding > > > as a disable flag instead of this... > > > > Yes I changed it back to kOsAndroid only since kDisableWebRtcHWEncoding is > > restored here. > > Now on CrOS, nothing is changed. > > On Android, VP8 and H264 can be enable/disable respectively (which is by > > design), also can be both disabled by kDisableWebRtcHWEncoding as before. > > I believe before your previous change we could pass a codec name to > kDisableWebRtcHWEncoding to selectively disable codecs... I'm ok using explicit > flags for this instead of a value, but it would be great if that worked not only > on Android...

Done. I can make the H264 flag also available on CrOS. But I wondering how you can "disable codecs selectively using kDisableWebRtcHWEncoding=codecname" on CrOS before. I didn't see such logic in the codes.

FEATURE_VALUE_TYPE(features::kWebRtcHWH264Encoding)},

{"enable-webrtc-stun-origin", IDS_FLAGS_WEBRTC_STUN_ORIGIN_NAME,

IDS_FLAGS_WEBRTC_STUN_ORIGIN_DESCRIPTION, kOsAll,