Created VideoFramePool to avoid unnecessary VideoFrame alloc/free.

Created VideoFramePool to avoid unnecessary VideoFrame alloc/free.

BUG=319000
TEST=All existing tests still pass.

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=239666

[acolwell GONE FROM CHROMIUM, 2013-12-09 20:19:53]

changing reviewer to dalecurtis@

[DaleCurtis, 2013-12-09 20:48:24]

Add a simple unittest?

https://codereview.chromium.org/101623006/diff/100001/media/base/video_frame.cc
File media/base/video_frame.cc (right):

https://codereview.chromium.org/101623006/diff/100001/media/base/video_frame....
media/base/video_frame.cc:187: 
Extra line.

https://codereview.chromium.org/101623006/diff/100001/media/base/video_frame.h
File media/base/video_frame.h (right):

https://codereview.chromium.org/101623006/diff/100001/media/base/video_frame....
media/base/video_frame.h:179: const WrapperDestroyedCB& wrapper_destroyed_cb);
Rename to no_longer_needed_cb for consistency with the above and change to
base::Closure()?  You can bind the frame in with the call to WrapVideoFrame and
avoid the extra bind within the function.

https://codereview.chromium.org/101623006/diff/100001/media/base/video_frame_...
File media/base/video_frame_pool.cc (right):

https://codereview.chromium.org/101623006/diff/100001/media/base/video_frame_...
media/base/video_frame_pool.cc:20: // Returns a frame from the pool that matches
the specified
Comment should indicate that the pool is drained if no matching frame is found.

https://codereview.chromium.org/101623006/diff/100001/media/base/video_frame_...
media/base/video_frame_pool.cc:87: frame,
base::Bind(&VideoFramePool::PoolImpl::FrameReleased, this));
As mentioned on the other file, just bind frame in here.

https://codereview.chromium.org/101623006/diff/100001/media/base/video_frame_...
media/base/video_frame_pool.cc:95: 
Extra line.

https://codereview.chromium.org/101623006/diff/100001/media/base/video_frame_...
File media/base/video_frame_pool.h (right):

https://codereview.chromium.org/101623006/diff/100001/media/base/video_frame_...
media/base/video_frame_pool.h:14: // VideoFrame objects.
Needs some comments about the lifetime of frames in the pool.

[acolwell GONE FROM CHROMIUM, 2013-12-09 21:42:11]

Addressed comments and added simple unittests.

https://codereview.chromium.org/101623006/diff/100001/media/base/video_frame.cc
File media/base/video_frame.cc (right):

https://codereview.chromium.org/101623006/diff/100001/media/base/video_frame....
media/base/video_frame.cc:187: 
On 2013/12/09 20:48:25, DaleCurtis wrote:
> Extra line.

Done.

https://codereview.chromium.org/101623006/diff/100001/media/base/video_frame.h
File media/base/video_frame.h (right):

https://codereview.chromium.org/101623006/diff/100001/media/base/video_frame....
media/base/video_frame.h:179: const WrapperDestroyedCB& wrapper_destroyed_cb);
On 2013/12/09 20:48:25, DaleCurtis wrote:
> Rename to no_longer_needed_cb for consistency with the above and change to
> base::Closure()?  You can bind the frame in with the call to WrapVideoFrame
and
> avoid the extra bind within the function.

Done.

https://codereview.chromium.org/101623006/diff/100001/media/base/video_frame_...
File media/base/video_frame_pool.cc (right):

https://codereview.chromium.org/101623006/diff/100001/media/base/video_frame_...
media/base/video_frame_pool.cc:20: // Returns a frame from the pool that matches
the specified
On 2013/12/09 20:48:25, DaleCurtis wrote:
> Comment should indicate that the pool is drained if no matching frame is
found.

Done.

https://codereview.chromium.org/101623006/diff/100001/media/base/video_frame_...
media/base/video_frame_pool.cc:87: frame,
base::Bind(&VideoFramePool::PoolImpl::FrameReleased, this));
On 2013/12/09 20:48:25, DaleCurtis wrote:
> As mentioned on the other file, just bind frame in here.

Done.

https://codereview.chromium.org/101623006/diff/100001/media/base/video_frame_...
media/base/video_frame_pool.cc:95: 
On 2013/12/09 20:48:25, DaleCurtis wrote:
> Extra line.

Done.

https://codereview.chromium.org/101623006/diff/100001/media/base/video_frame_...
File media/base/video_frame_pool.h (right):

https://codereview.chromium.org/101623006/diff/100001/media/base/video_frame_...
media/base/video_frame_pool.h:14: // VideoFrame objects.
On 2013/12/09 20:48:25, DaleCurtis wrote:
> Needs some comments about the lifetime of frames in the pool.

Done.

[DaleCurtis, 2013-12-09 21:48:36]

lgtm

[commit-bot: I haz the power, 2013-12-09 22:10:56]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/acolwell@chromium.org/101623006/2

[commit-bot: I haz the power, 2013-12-10 01:21:17]

Failed to trigger a try job on linux_rel
HTTP Error 400: Bad Request

[commit-bot: I haz the power, 2013-12-10 01:24:34]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/acolwell@chromium.org/101623006/120001

[commit-bot: I haz the power, 2013-12-10 05:17:20]

Change committed as 239666