New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 597150 link

Starred by 2 users
Status: Fixed
Owner:
posciak@chromium.org
Closed: Aug 2016
Cc:
█ vollick@chromium.org
jam@chromium.org
penghuang@chromium.org
█ fsam...@chromium.org
█ markdittmer@chromium.org
rjkroege@chromium.org
kcwu@chromium.org
vmi...@chromium.org
xhw...@chromium.org
piman@chromium.org
█ owenlin@chromium.org
posciak@chromium.org
mcasas@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 3
Type: Bug



Sign in to add a comment

GpuVideoDecodeAcceleratorFactoryImpl should implement GpuVideoDecodeAcceleratorFactory

Project Member Reported by posciak@chromium.org, Mar 23 2016 
As we can't include content/public from content/common, we cannot have GpuVideoDecodeAcceleratorFactoryImpl in content/common directly implement GpuVideoDecodeAcceleratorFactory. For now we are duplicating that interface in content/common and using a wrapper in content/public.

Once VDA implementations are moved out of content/common ( issue 586388  and related), make GpuVideoDecodeAcceleratorFactoryImpl really implement GpuVideoDecodeAcceleratorFactory.

Comment 1 by fsam...@chromium.org, Mar 23 2016

Cc: rjkroege@chromium.org markdittmer@chromium.org piman@chromium.org vollick@chromium.org vmi...@chromium.org penghuang@chromium.org
Labels: mustash1 mustash gpurefactor OS-All

Comment 2 by fsam...@chromium.org, May 2 2016

Cc: jam@chromium.org
+jam@, +xhwang@

Code has moved to media/gpu. I'm not sure I understand what you're suggesting here though. GpuVideoDecodeAcceleratorFactoryImpl lives in media/gpu.

I'd like to suggest the following:

1. Delete GpuVideoDecodeAcceleratoryFactory in content/public/gpu.
2. Rename GpuVideoDecodeAcceleratorFactoryImpl to GpuVideoDecodeAcceleratorFactory.
3. Replace all usage of content/public/gpu version with media/gpu version.

WDYT?

Comment 3 by fsam...@chromium.org, May 4 2016

Cc: xhw...@chromium.org

Comment 4 by xhw...@chromium.org, May 4 2016 

Since GpuVideoDecodeAcceleratorFactory has only one impl which is GpuVideoDecodeAcceleratorFactoryImpl. The proposal in #2 sgtm.

Comment 5 by posciak@chromium.org, May 11 2016

Status: Started (was: Assigned)

Comment 6 by fsam...@chromium.org, Jul 24 2016 

Has this been completed?

Comment 7 by posciak@chromium.org, Jul 25 2016 

CLs to remove GVDAF and rename GVDAFI to GVDAF by piman@ are under review:
https://codereview.chromium.org/2171473003/
https://codereview.chromium.org/2178603002/
Project Member

Comment 8 by bugdroid1@chromium.org, Jul 26 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e785d25b620665c05bfebedbc354b4989ea90ac9

commit e785d25b620665c05bfebedbc354b4989ea90ac9
Author: piman <piman@chromium.org>
Date: Tue Jul 26 08:59:51 2016

Remove content::GpuVideoDecodeAcceleratorFactory

This can be replaced by media::GpuVideoDecodeAcceleratorFactoryImpl, modulo
properly wiring GpuPreferences.

This in turns allows us to remove GpuChildThread::current() for great justice.

BUG= 597150 

Review-Url: https://codereview.chromium.org/2171473003
Cr-Commit-Position: refs/heads/master@{#407741}

[modify] https://crrev.com/e785d25b620665c05bfebedbc354b4989ea90ac9/android_webview/gpu/aw_content_gpu_client.cc
[modify] https://crrev.com/e785d25b620665c05bfebedbc354b4989ea90ac9/android_webview/gpu/aw_content_gpu_client.h
[modify] https://crrev.com/e785d25b620665c05bfebedbc354b4989ea90ac9/chrome/chrome_gpu.gypi
[modify] https://crrev.com/e785d25b620665c05bfebedbc354b4989ea90ac9/chrome/gpu/BUILD.gn
[modify] https://crrev.com/e785d25b620665c05bfebedbc354b4989ea90ac9/chrome/gpu/DEPS
[modify] https://crrev.com/e785d25b620665c05bfebedbc354b4989ea90ac9/chrome/gpu/arc_gpu_video_decode_accelerator.cc
[modify] https://crrev.com/e785d25b620665c05bfebedbc354b4989ea90ac9/chrome/gpu/arc_gpu_video_decode_accelerator.h
[modify] https://crrev.com/e785d25b620665c05bfebedbc354b4989ea90ac9/chrome/gpu/chrome_content_gpu_client.cc
[modify] https://crrev.com/e785d25b620665c05bfebedbc354b4989ea90ac9/chrome/gpu/chrome_content_gpu_client.h
[modify] https://crrev.com/e785d25b620665c05bfebedbc354b4989ea90ac9/chrome/gpu/gpu_arc_video_service.cc
[modify] https://crrev.com/e785d25b620665c05bfebedbc354b4989ea90ac9/chrome/gpu/gpu_arc_video_service.h
[modify] https://crrev.com/e785d25b620665c05bfebedbc354b4989ea90ac9/content/content_gpu.gypi
[modify] https://crrev.com/e785d25b620665c05bfebedbc354b4989ea90ac9/content/gpu/gpu_child_thread.cc
[modify] https://crrev.com/e785d25b620665c05bfebedbc354b4989ea90ac9/content/gpu/gpu_child_thread.h
[modify] https://crrev.com/e785d25b620665c05bfebedbc354b4989ea90ac9/content/public/gpu/BUILD.gn
[modify] https://crrev.com/e785d25b620665c05bfebedbc354b4989ea90ac9/content/public/gpu/DEPS
[modify] https://crrev.com/e785d25b620665c05bfebedbc354b4989ea90ac9/content/public/gpu/content_gpu_client.h
[delete] https://crrev.com/2d4220aeef84c8b33fcc7640efa943c8bc9abf16/content/public/gpu/gpu_video_decode_accelerator_factory.cc
[delete] https://crrev.com/2d4220aeef84c8b33fcc7640efa943c8bc9abf16/content/public/gpu/gpu_video_decode_accelerator_factory.h
[modify] https://crrev.com/e785d25b620665c05bfebedbc354b4989ea90ac9/media/gpu/BUILD.gn
[modify] https://crrev.com/e785d25b620665c05bfebedbc354b4989ea90ac9/media/gpu/gpu_video_decode_accelerator_factory_impl.h
►

Sign in to add a comment