RTCVideoDecoder: Fixes for high resolutions and high pressure cases.

RTCVideoDecoder: Fixes for high resolutions and high pressure cases.

RTCVideoDecoder allocates SHM buffers up to kMaxNumSharedMemorySegments,
using the currently requested size. However, if, after allocating
kMaxNumSharedMemorySegments, an input buffer arrives with a size larger
than the previously allocated, we will never allocate any more buffers
to fit the new size, and will not decode anymore, waiting forever for
new SHM buffers

Separately, we allow 300 input buffers to be pended and expect to be able
to catch up later on. However, if we are already 300 buffers behind, it is
very unlikely that we ever would. Moreover, it's better for user
experience to just catch up as fast as possible, dropping pending buffers.

This modifies SHM buffer allocation to always allocate a fixed number of
buffers, based on the currently requested size. If larger buffers are
needed later on, we wait for all SHM buffers to be returned and reallocate
with the new size.

Also, allow only 8 pending buffers. This, together with 16 decode buffers,
should be a deep enough pipeline.

BUG=496349
TEST=apprtc, large Hangouts

Committed: https://crrev.com/4a4514009cf31d4e7ae304d4c0dadda52d3d1516
Cr-Commit-Position: refs/heads/master@{#339787}

[Pawel Osciak, 2015-07-17 10:28:14]

posciak@chromium.org changed reviewers:
+ vmr@chromium.org, wuchengli@chromium.org

[Pawel Osciak, 2015-07-17 10:28:22]

ptal

[Ville-Mikko Rautio, 2015-07-17 11:26:15]

https://codereview.chromium.org/1236663003/diff/1/content/renderer/media/rtc_...
File content/renderer/media/rtc_video_decoder.cc (right):

https://codereview.chromium.org/1236663003/diff/1/content/renderer/media/rtc_...
content/renderer/media/rtc_video_decoder.cc:764: scoped_ptr<base::SharedMemory>
shm = factories_->CreateSharedMemory(size);
Earlier it was quaranteed that each buffer would be at least
kSharedMemorySegmentBytes (=100 << 10) bytes. Now we are pushing this floor
potentially down. Was it deliberate change? I don't know what was the original
reasoning for guaranteeing kSharedMemorySegmentBytes buffer.

You seem to have compensated this by requiring min_size * 2 buffer above in
GetSHM_Locked. Overall the determining the size of the buffer seems to contain
quite a bit of heuristics (which is true also on ToT).

[Pawel Osciak, 2015-07-21 07:10:07]

https://codereview.chromium.org/1236663003/diff/1/content/renderer/media/rtc_...
File content/renderer/media/rtc_video_decoder.cc (right):

https://codereview.chromium.org/1236663003/diff/1/content/renderer/media/rtc_...
content/renderer/media/rtc_video_decoder.cc:764: scoped_ptr<base::SharedMemory>
shm = factories_->CreateSharedMemory(size);
On 2015/07/17 11:26:15, Ville-Mikko Rautio wrote:
> Earlier it was quaranteed that each buffer would be at least
> kSharedMemorySegmentBytes (=100 << 10) bytes. Now we are pushing this floor
> potentially down. Was it deliberate change? I don't know what was the original
> reasoning for guaranteeing kSharedMemorySegmentBytes buffer.

Yes, that was intentional. We may have to reallocate after switching to a higher
resolution, but that will only happen once, after that we'll keep using these
larger buffers.

> You seem to have compensated this by requiring min_size * 2 buffer above in
> GetSHM_Locked. Overall the determining the size of the buffer seems to contain
> quite a bit of heuristics (which is true also on ToT).

If it becomes too small, this will reallocate to 2 * new_size again.
The existing ToT has a bug that may prevent it from reallocating when the
buffers are not big enough (please see CL description).

[wuchengli, 2015-07-21 08:36:25]

lgtm

[Pawel Osciak, 2015-07-21 11:15:12]

The CQ bit was checked by posciak@chromium.org

[commit-bot: I haz the power, 2015-07-21 11:15:22]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1236663003/1

[Pawel Osciak, 2015-07-21 11:15:42]

posciak@chromium.org changed reviewers:
+ dalecurtis@chromium.org

[Pawel Osciak, 2015-07-21 11:15:43]

dalecurtis@chromium.org: May I have owners please? Thanks.

[Pawel Osciak, 2015-07-21 11:15:54]

The CQ bit was unchecked by posciak@chromium.org

[DaleCurtis, 2015-07-21 18:19:32]

Since both WebRTC owners (tommi, perkj) are out I'll approve, but cc: tommi

lgtm

[Pawel Osciak, 2015-07-21 23:52:37]

The CQ bit was checked by posciak@chromium.org

[Pawel Osciak, 2015-07-21 23:52:52]

On 2015/07/21 18:19:32, DaleCurtis wrote:
> Since both WebRTC owners (tommi, perkj) are out I'll approve, but cc: tommi
> 
> lgtm

Thanks.

[commit-bot: I haz the power, 2015-07-21 23:52:56]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1236663003/1

[commit-bot: I haz the power, 2015-07-21 23:58:41]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2015-07-21 23:59:32]

Patchset 1 (id:??) landed as
https://crrev.com/4a4514009cf31d4e7ae304d4c0dadda52d3d1516
Cr-Commit-Position: refs/heads/master@{#339787}