Reintroduce kFramePadBytes in frame allocation.

Reintroduce kFramePadBytes in frame allocation.

Some FFmpeg SIMD functions overread/write slightly to have
more efficient buffer access code. Not adding padding thus
can lead to overreads/bytes (and crashes) when the buffer
pointer points near the end of the frame boundaries.

BUG=136231
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=145900

[DaleCurtis, 2012-07-09 23:25:51]

https://chromiumcodereview.appspot.com/10765015/diff/1/media/base/video_frame.cc
File media/base/video_frame.cc (right):

https://chromiumcodereview.appspot.com/10765015/diff/1/media/base/video_frame...
media/base/video_frame.cc:115: av_malloc(bytes_per_row * aligned_height +
kFramePadBytes));
FF_INPUT_BUFFER_PADDING_SIZE instead ?

[rbultje1, 2012-07-09 23:31:16]

They're syntactically different, INPUT_BUFFER_PADDING_SIZE is the padding in
input buffers (encoded bitstream). This is the padding in the actual decoded
bitmap buffer.

[DaleCurtis, 2012-07-10 00:01:45]

On 2012/07/09 23:31:16, rbultje1 wrote:
> They're syntactically different, INPUT_BUFFER_PADDING_SIZE is the padding in
> input buffers (encoded bitstream). This is the padding in the actual decoded
> bitmap buffer.

Perhaps, but should you still set kFramePadBytes = FF_INPUT_BUFFER_PADDING_SIZE
? In case it were to ever change.

[rbultje1, 2012-07-10 00:07:27]

On 2012/07/10 00:01:45, DaleCurtis wrote:
> On 2012/07/09 23:31:16, rbultje1 wrote:
> > They're syntactically different, INPUT_BUFFER_PADDING_SIZE is the padding in
> > input buffers (encoded bitstream). This is the padding in the actual decoded
> > bitmap buffer.
> 
> Perhaps, but should you still set kFramePadBytes =
FF_INPUT_BUFFER_PADDING_SIZE
> ? In case it were to ever change.

Well, the values are only the same by pure coincidence.
FF_INPUT_BUFFER_PADDING_SIZE should actually go down to 8 or 4 in one of the
next cycles, since we now have an overread-safe bitstream reading API so we
never read more than 4 bytes per read (in fact, it should be 8+1 or 4+1, since
we sometimes increment the pointer by 2), all this depending on the bitstream
reader in use (I can elaborate on this further offline if you want, but I'd just
recommend to follow upstream). On the other hand, Bitmap padding should be by
instruction boundary minus 1, so for SSE2, that's 16-1 (15), but as soon as
integer AVX instructions exist, it'll increase to 32-1 (31).

[DaleCurtis, 2012-07-10 00:16:55]

On 2012/07/10 00:07:27, rbultje1 wrote:
> On 2012/07/10 00:01:45, DaleCurtis wrote:
> > On 2012/07/09 23:31:16, rbultje1 wrote:
> > > They're syntactically different, INPUT_BUFFER_PADDING_SIZE is the padding
in
> > > input buffers (encoded bitstream). This is the padding in the actual
decoded
> > > bitmap buffer.
> > 
> > Perhaps, but should you still set kFramePadBytes =
> FF_INPUT_BUFFER_PADDING_SIZE
> > ? In case it were to ever change.
> 
> Well, the values are only the same by pure coincidence.
> FF_INPUT_BUFFER_PADDING_SIZE should actually go down to 8 or 4 in one of the
> next cycles, since we now have an overread-safe bitstream reading API so we
> never read more than 4 bytes per read (in fact, it should be 8+1 or 4+1, since
> we sometimes increment the pointer by 2), all this depending on the bitstream
> reader in use (I can elaborate on this further offline if you want, but I'd
just
> recommend to follow upstream). On the other hand, Bitmap padding should be by
> instruction boundary minus 1, so for SSE2, that's 16-1 (15), but as soon as
> integer AVX instructions exist, it'll increase to 32-1 (31).

Okay. LGTM.

[commit-bot: I haz the power, 2012-07-10 16:21:32]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rbultje@chromium.org/10765015/1

[commit-bot: I haz the power, 2012-07-10 17:31:30]

Change committed as 145900