New issue
Advanced search Search tips

Issue 889739 link

Starred by 2 users

Issue metadata

Status: Untriaged
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Feature



Sign in to add a comment

VideoEncodeAccelerator: allocate bitstream buffer properly.

Project Member Reported by akahuang@chromium.org, Sep 27

Issue description

Currently, we determine the output bitstream buffer size only by the resolution. For more details, we assess the size by the uncompressed buffer size (i.e. width * height * 1.5). However, this formula has some drawback:

1. The size might not big enough in small resolution because of the encoding overhead. 
2. The size is overestimated in large resolution.
3. The bitstream buffer size also depends on the bitrate and framerate.
To be more specific, the size should be proportional to (bitrate / framerate).

Considering these points, I think we can change the way to determine the buffer size:

1. Measure the biggest buffer size for common set of resolution, bitrate, framerate, and hard-coded the value to a table.
2. At runtime, select the buffer size of the closest resolution, and then multiply the ratio of (bitrate / framerate).

Here we have two assumptions:
Let the actual bitstream buffer size is a function of resolution size (i.e. width * height) and the ratio from bitrate to framerate (i.e. bitrate / framerate).
  buffer_size = f(resolution, ratio)
1. At the same bitrate and framerate, if the resolution is larger, then the bitstream buffer size is larger.
  if a < b then f(a, ratio) < f(b, ratio)
2. At the same resolution, linear extrapolation is a feasible upper bound.
  if k > 1 then f(size, k * ratio) <= k * f(size, ratio)


Besides this issue, now the VEA will crash directly when the buffer size is not big enough. I think we should be able to allocate a larger buffer instead of crash.
 
Investigation update:

I used the tulip series and crowd as test video, and measured the biggest buffer size at Eve device.

Here is the result:
Resolution  Bitrate framerate  BufferSize  MarginBufferSize
  320x180   100kbps   30fps       7615         15230
  640x360   500kbps   30fps      26121         52242
 1280x720   1.2Mbps   30fps      55051        110102
1092x1080   4.0Mbps   30fps     193829        387658


However, currently we don't change the buffer size when updating the bitrate/framerate. Therefore I need to calculate the buffer size by the maximum bitrate, which is 40Mbps in Intel chip. And the calculated value is larger than 2MB in every resolution. 

I'm wondering that maybe it's the best choice to use 2MB fixed size before we support changing the buffer size when updating bitrate and framerate?
Project Member

Comment 2 by bugdroid1@chromium.org, Oct 2

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

commit 260cd1798f0b0f5af17b69aae7eb3b0609653d92
Author: Chih-Yu Huang <akahuang@google.com>
Date: Tue Oct 02 06:15:30 2018

vaapi vea, v4l2 vea: Use fixed output buffer size.

In CL:1195175 we change the formula for Vaapi VEA. However, the
formula still failed at small resolution. According to the
investigation at crbug.com/889739 comment 1 and 2, using the fixed
buffer size might be the choice before we can update the buffer size
when the bitrate and framerate are changed.

Also, Because this function is general for Vaapi VEA and V4L2 VEA, we
move it to gpu_video_accelerator_util.

BUG=b:113309237
BUG=chromium:889739
TEST=pass VideoEncoderTest CTS on eve and scarlet device
TEST=pass RTC lookback on eve at H264 HD
TEST=pass VEA unittest on eve

Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel
Change-Id: I0aa6fd8b67c19ad93bae6e62430105dc9ccf6a80
Reviewed-on: https://chromium-review.googlesource.com/1253315
Commit-Queue: Chih-Yu Huang <akahuang@chromium.org>
Reviewed-by: Pawel Osciak <posciak@chromium.org>
Reviewed-by: Hirokazu Honda <hiroh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#595738}
[modify] https://crrev.com/260cd1798f0b0f5af17b69aae7eb3b0609653d92/media/gpu/BUILD.gn
[add] https://crrev.com/260cd1798f0b0f5af17b69aae7eb3b0609653d92/media/gpu/gpu_video_encode_accelerator_helpers.cc
[add] https://crrev.com/260cd1798f0b0f5af17b69aae7eb3b0609653d92/media/gpu/gpu_video_encode_accelerator_helpers.h
[modify] https://crrev.com/260cd1798f0b0f5af17b69aae7eb3b0609653d92/media/gpu/v4l2/v4l2_video_encode_accelerator.cc
[modify] https://crrev.com/260cd1798f0b0f5af17b69aae7eb3b0609653d92/media/gpu/v4l2/v4l2_video_encode_accelerator.h
[modify] https://crrev.com/260cd1798f0b0f5af17b69aae7eb3b0609653d92/media/gpu/vaapi/accelerated_video_encoder.cc

Project Member

Comment 3 by bugdroid1@chromium.org, Oct 9

Labels: merge-merged-3538
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3eecffee528a366e9d25f83cba8df86411dea2ce

commit 3eecffee528a366e9d25f83cba8df86411dea2ce
Author: Chih-Yu Huang <akahuang@google.com>
Date: Tue Oct 09 02:16:40 2018

vaapi vea, v4l2 vea: Use fixed output buffer size.

In CL:1195175 we change the formula for Vaapi VEA. However, the
formula still failed at small resolution. According to the
investigation at crbug.com/889739 comment 1 and 2, using the fixed
buffer size might be the choice before we can update the buffer size
when the bitrate and framerate are changed.

Also, Because this function is general for Vaapi VEA and V4L2 VEA, we
move it to gpu_video_accelerator_util.

BUG=b:113309237
BUG=chromium:889739
TEST=pass VideoEncoderTest CTS on eve and scarlet device
TEST=pass RTC lookback on eve at H264 HD
TEST=pass VEA unittest on eve

(cherry picked from commit 260cd1798f0b0f5af17b69aae7eb3b0609653d92)

Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel
Change-Id: I0aa6fd8b67c19ad93bae6e62430105dc9ccf6a80
Reviewed-on: https://chromium-review.googlesource.com/1253315
Commit-Queue: Chih-Yu Huang <akahuang@chromium.org>
Reviewed-by: Pawel Osciak <posciak@chromium.org>
Reviewed-by: Hirokazu Honda <hiroh@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#595738}
Reviewed-on: https://chromium-review.googlesource.com/c/1270201
Cr-Commit-Position: refs/branch-heads/3538@{#912}
Cr-Branched-From: 79f7c91a2b2a2932cd447fa6f865cb6662fa8fa6-refs/heads/master@{#587811}
[modify] https://crrev.com/3eecffee528a366e9d25f83cba8df86411dea2ce/media/gpu/BUILD.gn
[add] https://crrev.com/3eecffee528a366e9d25f83cba8df86411dea2ce/media/gpu/gpu_video_encode_accelerator_helpers.cc
[add] https://crrev.com/3eecffee528a366e9d25f83cba8df86411dea2ce/media/gpu/gpu_video_encode_accelerator_helpers.h
[modify] https://crrev.com/3eecffee528a366e9d25f83cba8df86411dea2ce/media/gpu/v4l2/v4l2_video_encode_accelerator.cc
[modify] https://crrev.com/3eecffee528a366e9d25f83cba8df86411dea2ce/media/gpu/v4l2/v4l2_video_encode_accelerator.h
[modify] https://crrev.com/3eecffee528a366e9d25f83cba8df86411dea2ce/media/gpu/vaapi/accelerated_video_encoder.cc

Sign in to add a comment