media: split GpuMemoryBufferVideoFramePool into GpuMemoryBufferVideoFrameCopier/Pool

media: split GpuMemoryBufferVideoFramePool into GpuMemoryBufferVideoFrameCopier/Pool

Current GpuMemoryBufferVideoFramePool has only one API, which copies software VideoFrame
to GpuMemoryBuffer VideoFrame. However, GpuMemoryBufferVideoFramePool can have new API to
provide new VideoFrame managed by internal pool like VideoFramePool.
If GpuMemoryBufferVideoFramePool provides new GMB VideoFrame, each decoder (e.g. ffmpeg, vpx)
can decode video frame on the GMB VideoFrame.
So split GpuMemoryBufferVideoFramePool into GpuMemoryBufferVideoFrameCopier/Pool.
Now GpuMemoryBufferVideoFramePool is very similar to VideoFramePool except for producing
a GMB VideoFrame. GpuMemoryBufferVideoFrameCopier has same API to old GpuMemoryBufferVideoFramePool.

TEST=media_unittests --gtest_filter=GpuMemoryBufferVideoFrame(Copier|Pool)*
BUG=601788, 590358

[dshwang, 2016-04-08 15:06:51]

Description was changed from

==========
media: split GpuMemoryBufferVideoFramePool into
GpuMemoryBufferVideoFrameCopier/Pool

Each decoder (e.g. ffmpeg, vpx) will reuse GpuMemoryBufferVideoFramePool.

BUG=601788
==========

to

==========
media: split GpuMemoryBufferVideoFramePool into
GpuMemoryBufferVideoFrameCopier/Pool

Each decoder (e.g. ffmpeg, vpx) will reuse GpuMemoryBufferVideoFramePool.

BUG=601788
==========

[dshwang, 2016-04-08 15:06:51]

dongseong.hwang@intel.com changed reviewers:
+ dcastagna@chromium.org

[dshwang, 2016-04-08 15:07:11]

dongseong.hwang@intel.com changed reviewers:
+ dalecurtis@chromium.org

[dshwang, 2016-04-08 15:08:23]

Daniele, could you review?

I extract this CL from https://codereview.chromium.org/1869303004/ to make it
easy to review the CL.
This CL doesn't change any logic.

[dshwang, 2016-04-08 15:15:19]

The CQ bit was checked by dongseong.hwang@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2016-04-08 15:15:34]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1874733002/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1874733002/1

[dshwang, 2016-04-08 15:25:50]

Patchset #2 (id:20001) has been deleted

[dshwang, 2016-04-08 15:25:57]

The CQ bit was checked by dongseong.hwang@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2016-04-08 15:26:12]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1874733002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1874733002/40001

[commit-bot: I haz the power, 2016-04-08 16:56:38]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-04-08 16:56:39]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[dshwang, 2016-04-11 08:58:30]

Description was changed from

==========
media: split GpuMemoryBufferVideoFramePool into
GpuMemoryBufferVideoFrameCopier/Pool

Each decoder (e.g. ffmpeg, vpx) will reuse GpuMemoryBufferVideoFramePool.

BUG=601788
==========

to

==========
media: split GpuMemoryBufferVideoFramePool into
GpuMemoryBufferVideoFrameCopier/Pool

Each decoder (e.g. ffmpeg, vpx) will reuse GpuMemoryBufferVideoFramePool.

BUG=601788, 590358
==========

[dshwang, 2016-04-11 15:42:38]

The CQ bit was checked by dongseong.hwang@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2016-04-11 15:42:46]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1874733002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1874733002/60001

[dshwang, 2016-04-11 15:44:04]

Description was changed from

==========
media: split GpuMemoryBufferVideoFramePool into
GpuMemoryBufferVideoFrameCopier/Pool

Each decoder (e.g. ffmpeg, vpx) will reuse GpuMemoryBufferVideoFramePool.

BUG=601788, 590358
==========

to

==========
media: split GpuMemoryBufferVideoFramePool into
GpuMemoryBufferVideoFrameCopier/Pool

Each decoder (e.g. ffmpeg, vpx) will reuse GpuMemoryBufferVideoFramePool.

TEST=media_unittests --gtest_filter=GpuMemoryBufferVideoFrame(Copier|Pool)*
BUG=601788, 590358
==========

[commit-bot: I haz the power, 2016-04-11 15:44:47]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-04-11 15:44:47]

Dry run: Try jobs failed on following builders:
  ios_rel_device_gn on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_gn...)
  ios_rel_device_ninja on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_ni...)

[dshwang, 2016-04-11 15:47:47]

Patchset #3 (id:60001) has been deleted

[dshwang, 2016-04-11 15:48:37]

The CQ bit was checked by dongseong.hwang@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2016-04-11 15:48:53]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1874733002/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1874733002/80001

[commit-bot: I haz the power, 2016-04-11 15:58:01]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-04-11 15:58:01]

Dry run: Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[dshwang, 2016-04-11 16:07:13]

Patchset #3 (id:80001) has been deleted

[dshwang, 2016-04-11 16:07:49]

The CQ bit was checked by dongseong.hwang@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2016-04-11 16:08:06]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1874733002/100001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1874733002/100001

[commit-bot: I haz the power, 2016-04-11 18:07:04]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-04-11 18:07:04]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[dshwang, 2016-05-04 10:02:36]

Daniele, Dale, PTAL?

[DaleCurtis, 2016-05-09 18:54:37]

I'm not familiar with Daniele's more recent work, so I defer to him until this
is ready for a media/ review.

[Daniele Castagna, 2016-05-11 03:03:17]

Sorry for the delay, I'm currently traveling.
I'm going to be back in the office next Tuesday and I'll look into this.

[dshwang, 2016-05-11 07:30:45]

On 2016/05/11 03:03:17, Daniele Castagna wrote:
> Sorry for the delay, I'm currently traveling.
> I'm going to be back in the office next Tuesday and I'll look into this.

Ok, thanks in advance.

[Daniele Castagna, 2016-05-18 04:30:07]

A few questions/observations from a high level point of view.

You don't explain in the CL description why we'd want to do this.
It'd be nice to explain this CL would be necessary to decode directly into GMBs.
And this brings us back to what we already discussed in another CL. How are we
going to determine when we can read back from GMBs? Without a plan for that, I'm
not sure we should move forward with this.

You said you tried to "postpone the VideoFrame creation until FFMPEG release
reference" in crrev.com/1869303004/#msg37. That is not what I meant we should do
(I promised a doc I haven't written yet, sorry), I think the approach we should
try is described in crbug.com/529103.

WDYT?

[dshwang, 2016-05-18 18:34:25]

Description was changed from

==========
media: split GpuMemoryBufferVideoFramePool into
GpuMemoryBufferVideoFrameCopier/Pool

Each decoder (e.g. ffmpeg, vpx) will reuse GpuMemoryBufferVideoFramePool.

TEST=media_unittests --gtest_filter=GpuMemoryBufferVideoFrame(Copier|Pool)*
BUG=601788, 590358
==========

to

==========
media: split GpuMemoryBufferVideoFramePool into
GpuMemoryBufferVideoFrameCopier/Pool

Current GpuMemoryBufferVideoFramePool has only one API, which copies software
VideoFrame
to GpuMemoryBuffer VideoFrame. However, GpuMemoryBufferVideoFramePool can have
new API to
provide new VideoFrame managed by internal pool like VideoFramePool.
If GpuMemoryBufferVideoFramePool provides new GMB VideoFrame, each decoder (e.g.
ffmpeg, vpx)
can decode video frame on the GMB VideoFrame.
So split GpuMemoryBufferVideoFramePool into
GpuMemoryBufferVideoFrameCopier/Pool.
Now GpuMemoryBufferVideoFramePool is very similar to VideoFramePool except for
producing
a GMB VideoFrame. GpuMemoryBufferVideoFrameCopier has same API to old
GpuMemoryBufferVideoFramePool.

TEST=media_unittests --gtest_filter=GpuMemoryBufferVideoFrame(Copier|Pool)*
BUG=601788, 590358
==========

[dshwang, 2016-05-18 18:36:10]

On 2016/05/18 04:30:07, Daniele Castagna wrote:
> A few questions/observations from a high level point of view.
> 
> You don't explain in the CL description why we'd want to do this.
> It'd be nice to explain this CL would be necessary to decode directly into
GMBs.
> And this brings us back to what we already discussed in another CL. How are we
> going to determine when we can read back from GMBs? Without a plan for that,
I'm
> not sure we should move forward with this.

Thank you for feedback. Explained why we'd want to do this in the CL
description.

> 
> You said you tried to "postpone the VideoFrame creation until FFMPEG release
> reference" in crrev.com/1869303004/#msg37. That is not what I meant we should
do
> (I promised a doc I haven't written yet, sorry), I think the approach we
should
> try is described in crbug.com/529103.

I explained Cros plan in
https://bugs.chromium.org/p/chromium/issues/detail?id=529103#c5

[Daniele Castagna, 2016-05-19 18:08:04]

On 2016/05/18 at 18:36:10, dongseong.hwang wrote:
> On 2016/05/18 04:30:07, Daniele Castagna wrote:
> > A few questions/observations from a high level point of view.
> > 
> > You don't explain in the CL description why we'd want to do this.
> > It'd be nice to explain this CL would be necessary to decode directly into
GMBs.
> > And this brings us back to what we already discussed in another CL. How are
we
> > going to determine when we can read back from GMBs? Without a plan for that,
I'm
> > not sure we should move forward with this.
> 
> Thank you for feedback. Explained why we'd want to do this in the CL
description.
> 
> > 
> > You said you tried to "postpone the VideoFrame creation until FFMPEG release
> > reference" in crrev.com/1869303004/#msg37. That is not what I meant we
should do
> > (I promised a doc I haven't written yet, sorry), I think the approach we
should
> > try is described in crbug.com/529103.
> 
> I explained Cros plan in
https://bugs.chromium.org/p/chromium/issues/detail?id=529103#c5

You said:
"I tried to do with ffmpeg, but I could not implement promising prototype
because
- I could not know whether the frame is key frame and dependent frame
- ffmpeg release bunch of frames together (which means I have to copy all the
bunch of frames)"

Do you mind uploading a CL with the prototype?

If ffmpeg releases a frame, it means we don't need to copy it. I don't
understand why you're saying we need to copy them when they get released.

[dshwang, 2016-05-19 18:28:39]

On 2016/05/19 18:08:04, Daniele Castagna wrote:
> You said:
> "I tried to do with ffmpeg, but I could not implement promising prototype
> because
> - I could not know whether the frame is key frame and dependent frame
> - ffmpeg release bunch of frames together (which means I have to copy all the
> bunch of frames)"
> 
> Do you mind uploading a CL with the prototype?

The prototype is lots of LOG(ERROR) based on
https://codereview.chromium.org/1871383003/
When ffmpeg releases a fraem, following callback is called 'static void
ReleaseVideoBufferImpl(void* opaque, uint8_t* data)'
I just compare |data| pointer value between decoding and releasing.

> If ffmpeg releases a frame, it means we don't need to copy it. I don't
> understand why you're saying we need to copy them when they get released.

My explanation seems to be not clear.
As ffmpeg releases a frame too late, we cannot wait it in order to produce new
VideoFrame. So we need to copy them during ffmpeg holding the frame.