Add limitations of number of frames that can be created in I420BufferPool::CreateBuffer.

Add limitations of number of frames that can be created in I420BufferPool::CreateBuffer.
If more than 60 frames are created and not returned, the implementation will crash.

I420BufferPool are currently used by the VP8 decoder, Quality scaler and VideoFrameFactory.

BUG=b/31390397
NOTRY=true  // Because of failing gclient runhooks on some bots

Committed: https://crrev.com/5f8ebaeffd2f4f7a3e5f857c407b2ff9aca7454e
Cr-Commit-Position: refs/heads/master@{#14395}

[perkj_webrtc, 2016-09-26 12:50:25]

Patchset #2 (id:20001) has been deleted

[perkj_webrtc, 2016-09-26 12:51:19]

perkj@webrtc.org changed reviewers:
+ magjed@webrtc.org, mflodman@webrtc.org

[perkj_webrtc, 2016-09-26 12:51:19]

Is this a good idea to ad to Canary?

[mflodman, 2016-09-26 14:36:47]

One comment about the limit, LGTM with that comment addressed.

https://codereview.webrtc.org/2370653003/diff/1/webrtc/common_video/include/i...
File webrtc/common_video/include/i420_buffer_pool.h (right):

https://codereview.webrtc.org/2370653003/diff/1/webrtc/common_video/include/i...
webrtc/common_video/include/i420_buffer_pool.h:41: static const int
kMaxNumberOfFramesBeforeCrash = 60;
I'd like to raise this limit a bit higher. Normally it shouldn't be this high,
but in the case of intentionally increasing the receive side delay this (= 2
second @ 30fps and 1s @ 60 fps) might be on the lower end.

WDYT of setting this to 300 (=10s @ 30fps, and 330MB mem usage in HD)?

[magjed_webrtc, 2016-09-26 14:50:55]

lgtm if this is temporary for debugging purposes and not intended to reach
stable.

https://codereview.webrtc.org/2370653003/diff/1/webrtc/common_video/i420_buff...
File webrtc/common_video/i420_buffer_pool.cc (right):

https://codereview.webrtc.org/2370653003/diff/1/webrtc/common_video/i420_buff...
webrtc/common_video/i420_buffer_pool.cc:27: RTC_CHECK(buffers_.size() <
kMaxNumberOfFramesBeforeCrash);
nit: Use RTC_CHECK_LT (even if it must crash exactly at
kMaxNumberOfFramesBeforeCrash with the current code).

[perkj_webrtc, 2016-09-27 07:41:41]

https://codereview.webrtc.org/2370653003/diff/1/webrtc/common_video/i420_buff...
File webrtc/common_video/i420_buffer_pool.cc (right):

https://codereview.webrtc.org/2370653003/diff/1/webrtc/common_video/i420_buff...
webrtc/common_video/i420_buffer_pool.cc:27: RTC_CHECK(buffers_.size() <
kMaxNumberOfFramesBeforeCrash);
On 2016/09/26 14:50:55, magjed_webrtc wrote:
> nit: Use RTC_CHECK_LT (even if it must crash exactly at
> kMaxNumberOfFramesBeforeCrash with the current code).

Done.

https://codereview.webrtc.org/2370653003/diff/1/webrtc/common_video/include/i...
File webrtc/common_video/include/i420_buffer_pool.h (right):

https://codereview.webrtc.org/2370653003/diff/1/webrtc/common_video/include/i...
webrtc/common_video/include/i420_buffer_pool.h:41: static const int
kMaxNumberOfFramesBeforeCrash = 60;
On 2016/09/26 14:36:47, mflodman wrote:
> I'd like to raise this limit a bit higher. Normally it shouldn't be this high,
> but in the case of intentionally increasing the receive side delay this (= 2
> second @ 30fps and 1s @ 60 fps) might be on the lower end.
> 
> WDYT of setting this to 300 (=10s @ 30fps, and 330MB mem usage in HD)?
> 

ok- set to 300.

[perkj_webrtc, 2016-09-27 07:42:07]

The CQ bit was checked by perkj@webrtc.org

[perkj_webrtc, 2016-09-27 07:42:08]

The patchset sent to the CQ was uploaded after l-g-t-m from magjed@webrtc.org,
mflodman@webrtc.org
Link to the patchset: https://codereview.webrtc.org/2370653003/#ps40001 (title:
"Increased size and addressed nit.")

[commit-bot: I haz the power, 2016-09-27 07:42:17]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[commit-bot: I haz the power, 2016-09-27 07:43:09]

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

[commit-bot: I haz the power, 2016-09-27 07:43:10]

Try jobs failed on following builders:
  ios64_sim_dbg on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/ios64_sim_dbg/builds/10959)
  ios_arm64_dbg on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/ios_arm64_dbg/builds/13259)
  mac_compile_dbg on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/mac_compile_dbg/builds/...)

[perkj_webrtc, 2016-09-27 09:11:38]

The CQ bit was checked by perkj@webrtc.org

[perkj_webrtc, 2016-09-27 09:11:38]

The patchset sent to the CQ was uploaded after l-g-t-m from magjed@webrtc.org,
mflodman@webrtc.org
Link to the patchset: https://codereview.webrtc.org/2370653003/#ps60001 (title:
"Fix definition/declaration.")

[commit-bot: I haz the power, 2016-09-27 09:11:43]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[commit-bot: I haz the power, 2016-09-27 10:36:46]

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

[commit-bot: I haz the power, 2016-09-27 10:36:47]

Try jobs failed on following builders:
  win_x64_clang_dbg on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/win_x64_clang_dbg/build...)

[perkj_webrtc, 2016-09-27 10:46:02]

Description was changed from

==========
Add limitations of number of frames that can be created in
I420BufferPool::CreateBuffer.
If more than 60 frames are created and not returned, the implementation will
crash.

I420BufferPool are currently used by the VP8 decoder, Quality scaler and
VideoFrameFactory.

BUG=b/31390397
==========

to

==========
Add limitations of number of frames that can be created in
I420BufferPool::CreateBuffer.
If more than 60 frames are created and not returned, the implementation will
crash.

I420BufferPool are currently used by the VP8 decoder, Quality scaler and
VideoFrameFactory.

BUG=b/31390397
NOTRY=true  // Because of failing gclient runhooks on some bots
==========

[perkj_webrtc, 2016-09-27 10:46:15]

The CQ bit was checked by perkj@webrtc.org

[commit-bot: I haz the power, 2016-09-27 10:46:28]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[commit-bot: I haz the power, 2016-09-27 10:47:43]

Description was changed from

==========
Add limitations of number of frames that can be created in
I420BufferPool::CreateBuffer.
If more than 60 frames are created and not returned, the implementation will
crash.

I420BufferPool are currently used by the VP8 decoder, Quality scaler and
VideoFrameFactory.

BUG=b/31390397
NOTRY=true  // Because of failing gclient runhooks on some bots
==========

to

==========
Add limitations of number of frames that can be created in
I420BufferPool::CreateBuffer.
If more than 60 frames are created and not returned, the implementation will
crash.

I420BufferPool are currently used by the VP8 decoder, Quality scaler and
VideoFrameFactory.

BUG=b/31390397
NOTRY=true  // Because of failing gclient runhooks on some bots
==========

[commit-bot: I haz the power, 2016-09-27 10:47:45]

Committed patchset #3 (id:60001)

[commit-bot: I haz the power, 2016-09-27 10:47:53]

Description was changed from

==========
Add limitations of number of frames that can be created in
I420BufferPool::CreateBuffer.
If more than 60 frames are created and not returned, the implementation will
crash.

I420BufferPool are currently used by the VP8 decoder, Quality scaler and
VideoFrameFactory.

BUG=b/31390397
NOTRY=true  // Because of failing gclient runhooks on some bots
==========

to

==========
Add limitations of number of frames that can be created in
I420BufferPool::CreateBuffer.
If more than 60 frames are created and not returned, the implementation will
crash.

I420BufferPool are currently used by the VP8 decoder, Quality scaler and
VideoFrameFactory.

BUG=b/31390397
NOTRY=true  // Because of failing gclient runhooks on some bots

Committed: https://crrev.com/5f8ebaeffd2f4f7a3e5f857c407b2ff9aca7454e
Cr-Commit-Position: refs/heads/master@{#14395}
==========

[commit-bot: I haz the power, 2016-09-27 10:47:53]

Patchset 3 (id:??) landed as
https://crrev.com/5f8ebaeffd2f4f7a3e5f857c407b2ff9aca7454e
Cr-Commit-Position: refs/heads/master@{#14395}