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

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 ago by dshwang
Modified:
11 months, 4 weeks 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 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 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 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 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 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 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 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 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 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 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 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 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 ...
12 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: ...
12 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 ...
12 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): > > ...
12 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: > > ...
12 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 ...
12 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. ...
12 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 ...
12 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 ...
12 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 ...
12 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 ...
12 months ago (2014-03-06 19:33:01 UTC) #23
dshwang
I update code and description as your comment. Could you review again?
12 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 ...
12 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 ...
11 months, 4 weeks ago (2014-03-10 11:43:06 UTC) #26
dshwang
The CQ bit was checked by dongseong.hwang@intel.com
11 months, 4 weeks 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
11 months, 4 weeks 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
11 months, 4 weeks 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
11 months, 4 weeks 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
11 months, 4 weeks 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
11 months, 4 weeks ago (2014-03-10 17:33:35 UTC) #32
I haz the power (commit-bot)
11 months, 4 weeks 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 dd99357-tainted