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

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, 1 month ago by dshwang
Modified:
1 year, 1 month 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, 1 month 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, 1 month 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, 1 month 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, 1 month 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, 1 month 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, 1 month 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, 1 month 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, 1 month 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, 1 month 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, 1 month 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, 1 month 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, 1 month 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, 1 month 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, 1 month 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, 1 month 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, 1 month 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, 1 month 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, 1 month 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, 1 month 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, 1 month 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, 1 month 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, 1 month 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, 1 month ago (2014-03-06 19:33:01 UTC) #23
dshwang
I update code and description as your comment. Could you review again?
1 year, 1 month 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, 1 month 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, 1 month ago (2014-03-10 11:43:06 UTC) #26
dshwang
The CQ bit was checked by dongseong.hwang@intel.com
1 year, 1 month ago (2014-03-10 11:43:11 UTC) #27
I haz the power (commit-bot)
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/dongseong.hwang@intel.com/176883018/130001
1 year, 1 month ago (2014-03-10 11:43:23 UTC) #28
I haz the power (commit-bot)
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/dongseong.hwang@intel.com/176883018/130001
1 year, 1 month ago (2014-03-10 13:00:57 UTC) #29
I haz the power (commit-bot)
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/dongseong.hwang@intel.com/176883018/130001
1 year, 1 month ago (2014-03-10 14:18:20 UTC) #30
I haz the power (commit-bot)
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/dongseong.hwang@intel.com/176883018/130001
1 year, 1 month ago (2014-03-10 15:24:21 UTC) #31
I haz the power (commit-bot)
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/dongseong.hwang@intel.com/176883018/130001
1 year, 1 month ago (2014-03-10 17:33:35 UTC) #32
I haz the power (commit-bot)
1 year, 1 month 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 e0e3771