Chromium Code Reviews
Help | Chromium Project | Sign in
(215)

Issue 176883018: Remove additional protection of kDisableAcceleratedVideoDecode in bpf_gpu_policy_linux.cc. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
1 year, 4 months ago by dshwang
Modified:
1 year, 4 months ago
CC:
chromium-reviews, fischman+watch_chromium.org, jam, mcasas+watch_chromium.org, joi+watch-content_chromium.org, feature-media-reviews_chromium.org, darin-cc_chromium.org, piman+watch_chromium.org, wjia+watch_chromium.org, jln+watch_chromium.org, Pawel Osciak
Base URL:
https://git.chromium.org/chromium/src.git@master
Visibility:
Public.

Description

Remove additional protection of kDisableAcceleratedVideoDecode in bpf_gpu_policy_linux.cc. The blacklist is the way we make features available or not. The control of accelerated video decode should rely on GPU blacklist. After this CL, Linux and Mac will output following error message when using --ignore-gpu-blacklist. "[(pid):ERROR:gpu_video_decode_accelerator.cc(303)] Not implemented reached in void content::GpuVideoDecodeAccelerator::Initialize(media::VideoCodecProfile, IPC::Message*) HW video decode acceleration not available." In addition, match macro style in video_decode_accelerator_unittest.cc to gpu_video_decode_accelerator.cc BUG=N/A Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=255968

Patch Set 1 #

Total comments: 1

Patch Set 2 : Change the goal: just clean up #

Patch Set 3 : Change the goal: just clean up #

Patch Set 4 : submit again due to Upload got a 500 response: 500 #

Patch Set 5 : Upload got a 500 response: 500 #

Total comments: 1

Patch Set 6 : set kDisableAcceleratedVideoDecode no matther what, unless defined(OS_CHROMEOS) || defined(OS_WIN) … #

Total comments: 2

Patch Set 7 : Keep IsAcceleratedVideoDecodeEnabled() #

Patch Set 8 : Remove additional protection in bpf_gpu_policy_linux.cc #

Messages

Total messages: 33 (0 generated)
dshwang
1 year, 4 months ago (2014-03-03 15:57:35 UTC) #1
arekm
On 2014/03/03 15:57:35, dshwang wrote: + I965DrvVideoPath = "/usr/lib/x86_64-linux-gnu/dri/i965_drv_video.so"; will these stay hardcoded? (invalid here ...
1 year, 4 months ago (2014-03-03 17:07:35 UTC) #2
dshwang
On 2014/03/03 17:07:35, arekm wrote: > On 2014/03/03 15:57:35, dshwang wrote: > > + I965DrvVideoPath ...
1 year, 4 months ago (2014-03-03 17:14:47 UTC) #3
Ami GONE FROM CHROMIUM
Does not lgtm. The rationale listed in the CL description fails to address the reason ...
1 year, 4 months ago (2014-03-03 17:28:11 UTC) #4
dshwang
On 2014/03/03 17:28:11, Ami Fischman wrote: > Does not lgtm. > The rationale listed in ...
1 year, 4 months ago (2014-03-03 18:15:44 UTC) #5
Ami GONE FROM CHROMIUM
There is a history of users disabling the blacklist (entirely) because they want a feature ...
1 year, 4 months ago (2014-03-03 18:26:27 UTC) #6
Jorge Lucangeli Obes
On 2014/03/03 18:26:27, Ami Fischman wrote: > There is a history of users disabling the ...
1 year, 4 months ago (2014-03-03 18:42:48 UTC) #7
dshwang
On 2014/03/03 18:42:48, Jorge Lucangeli Obes wrote: > On 2014/03/03 18:26:27, Ami Fischman wrote: > ...
1 year, 4 months ago (2014-03-03 19:16:50 UTC) #8
Ami GONE FROM CHROMIUM
https://codereview.chromium.org/176883018/diff/70001/content/browser/gpu/gpu_data_manager_impl_private.cc File content/browser/gpu/gpu_data_manager_impl_private.cc (right): https://codereview.chromium.org/176883018/diff/70001/content/browser/gpu/gpu_data_manager_impl_private.cc#newcode325 content/browser/gpu/gpu_data_manager_impl_private.cc:325: #if defined(OS_CHROMEOS) || defined(OS_WIN) || defined(OS_ANDROID) Why is this ...
1 year, 4 months ago (2014-03-03 20:38:37 UTC) #9
dshwang
I have a good compromise plan. Summary: introduce gpu-video-decode-whitelist. don't export it on chrome://flags until ...
1 year, 4 months ago (2014-03-03 20:45:30 UTC) #10
Ami GONE FROM CHROMIUM
> > Summary: introduce gpu-video-decode-whitelist. don't export it on > chrome://flags > until we have ...
1 year, 4 months ago (2014-03-03 21:15:59 UTC) #11
dshwang
On 2014/03/03 21:15:59, Ami Fischman wrote: > I don't see what future developments would give ...
1 year, 4 months ago (2014-03-04 08:43:42 UTC) #12
Ami GONE FROM CHROMIUM
The BUG= line in the CL description should probably be dropped (since the CL has ...
1 year, 4 months ago (2014-03-05 02:03:37 UTC) #13
dshwang
Thank you for review and sorry for delaying reply. https://codereview.chromium.org/176883018/diff/90001/content/browser/gpu/gpu_data_manager_impl_private.cc File content/browser/gpu/gpu_data_manager_impl_private.cc (right): https://codereview.chromium.org/176883018/diff/90001/content/browser/gpu/gpu_data_manager_impl_private.cc#newcode328 content/browser/gpu/gpu_data_manager_impl_private.cc:328: ...
1 year, 4 months ago (2014-03-06 12:44:37 UTC) #14
Jorge Lucangeli Obes
https://codereview.chromium.org/176883018/diff/90001/content/common/sandbox_linux/bpf_gpu_policy_linux.cc File content/common/sandbox_linux/bpf_gpu_policy_linux.cc (left): https://codereview.chromium.org/176883018/diff/90001/content/common/sandbox_linux/bpf_gpu_policy_linux.cc#oldcode76 content/common/sandbox_linux/bpf_gpu_policy_linux.cc:76: // Accelerated video decode is currently enabled on Chrome ...
1 year, 4 months ago (2014-03-06 15:53:59 UTC) #15
dshwang
On 2014/03/06 15:53:59, Jorge Lucangeli Obes wrote: > https://codereview.chromium.org/176883018/diff/90001/content/common/sandbox_linux/bpf_gpu_policy_linux.cc > File content/common/sandbox_linux/bpf_gpu_policy_linux.cc (left): > > ...
1 year, 4 months ago (2014-03-06 16:15:06 UTC) #16
Jorge Lucangeli Obes
On 2014/03/06 16:15:06, dshwang wrote: > On 2014/03/06 15:53:59, Jorge Lucangeli Obes wrote: > > ...
1 year, 4 months ago (2014-03-06 16:28:33 UTC) #17
dshwang
On 2014/03/06 16:28:33, Jorge Lucangeli Obes wrote: > bpf_gpu_policy_linux.cc lgtm, but wait for Ami for ...
1 year, 4 months ago (2014-03-06 16:35:16 UTC) #18
Ami GONE FROM CHROMIUM
On 2014/03/06 12:44:37, dshwang wrote: > Thank you for review and sorry for delaying reply. ...
1 year, 4 months ago (2014-03-06 17:29:05 UTC) #19
dshwang
On 2014/03/06 17:29:05, Ami Fischman wrote: > On 2014/03/06 12:44:37, dshwang wrote: > > Thank ...
1 year, 4 months ago (2014-03-06 18:33:29 UTC) #20
Ami GONE FROM CHROMIUM
> > First of all, it declares Linux and Mac does not support Accelerated Video ...
1 year, 4 months ago (2014-03-06 19:18:21 UTC) #21
dshwang
On 2014/03/06 19:18:21, Ami Fischman wrote: > We don't usually put in multiple layers of ...
1 year, 4 months ago (2014-03-06 19:32:30 UTC) #22
dshwang
On 2014/03/06 19:32:30, dshwang wrote: > Thank you for good explanation. I'll review #if. Linux ...
1 year, 4 months ago (2014-03-06 19:33:01 UTC) #23
dshwang
I update code and description as your comment. Could you review again?
1 year, 4 months ago (2014-03-07 14:43:45 UTC) #24
Ami GONE FROM CHROMIUM
LGTM assuming on linux the sandbox doesn't try to load the cros-only libs; i.e. that ...
1 year, 4 months ago (2014-03-07 22:29:44 UTC) #25
dshwang
On 2014/03/07 22:29:44, Ami Fischman wrote: > LGTM assuming on linux the sandbox doesn't try ...
1 year, 4 months ago (2014-03-10 11:43:06 UTC) #26
dshwang
The CQ bit was checked by dongseong.hwang@intel.com
1 year, 4 months ago (2014-03-10 11:43:11 UTC) #27
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/dongseong.hwang@intel.com/176883018/130001
1 year, 4 months ago (2014-03-10 11:43:23 UTC) #28
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/dongseong.hwang@intel.com/176883018/130001
1 year, 4 months ago (2014-03-10 13:00:57 UTC) #29
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/dongseong.hwang@intel.com/176883018/130001
1 year, 4 months ago (2014-03-10 14:18:20 UTC) #30
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/dongseong.hwang@intel.com/176883018/130001
1 year, 4 months ago (2014-03-10 15:24:21 UTC) #31
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/dongseong.hwang@intel.com/176883018/130001
1 year, 4 months ago (2014-03-10 17:33:35 UTC) #32
commit-bot: I haz the power
1 year, 4 months ago (2014-03-10 17:59:07 UTC) #33
Message was sent while issue was closed.
Change committed as 255968
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 5fa3ca5