In mediaconduit_unittest.cpp, if we call mVideoSession->SendVideoFrame() and pass 4x2 or 2x4 as input resolution, it may crash after test cases finished.

tim - is this fixed in a newer release?

(In reply to Randell Jesup [:jesup] from comment #1) > tim - is this fixed in a newer release? I have no idea. I don't remember seeing any issues on the subject.

With debug version, it asserted at below location. Breakpoint 3, webrtc::VP8EncoderImpl::GetEncodedPartitions (this=0x7fffe67021a0, input_image=...) at /home/sywu/work/mozilla-central/media/webrtc/trunk/webrtc/modules/video_coding/codecs/vp8/vp8_impl.cc:448 448 assert(encoded_image_._length <= encoded_image_._size); The values are: encoded_image_._length = 30 encoded_image_._size = 6 The resolution been tested is 2x4, and we allocate the video frame length for 6 bytes (width*height*3/2) bytes.

(In reply to Shian-Yow Wu [:shianyow] from comment #3) > The resolution been tested is 2x4, and we allocate the video frame length > for 6 bytes (width*height*3/2) bytes. To correct, it is 12 bytes for resolution 2x4.

Sorry, to clarify again. The resolution been tested was 2x2 with frame legnth 6 allocated in the above example.

It seems a VP8 encoder needs at least 31 bytes(first time 30 and second time 1) to process the frame. However, we cannot just allocate and pass a larger frame length (ex: width*height*3/2 + 100), because there is a buffer size check at: dxr.mozilla.org/mozilla-central/source/media/webrtc/trunk/webrtc/modules/video_capture/video_capture_impl.cc?from=video_capture_impl.cc#l254 Is it a required check? Or can we modify the CalcBufferSize() calculation in order to reserve more buffer?

(In reply to Shian-Yow Wu [:shianyow] from comment #6) > However, we cannot just allocate and pass a larger frame length (ex: > width*height*3/2 + 100), because there is a buffer size check at: > dxr.mozilla.org/mozilla-central/source/media/webrtc/trunk/webrtc/modules/ > video_capture/video_capture_impl.cc?from=video_capture_impl.cc#l254 > > Is it a required check? Or can we modify the CalcBufferSize() calculation > in order to reserve more buffer? Sorry for the delay in replying. I don't think we should modify CalcBufferSize(), since it is part of libyuv. However, we can modify the call in VP8EncoderImpl::InitEncode() to add a constant to the return value of CalcBufferSize().

Comment on attachment 8341524 [details] [diff] [review] bug919979.patch Tim, Thanks for your comment. The VP8 overhead for small resolution is 31 bytes, and the patch reserves 100 bytes for it. Could you review it?

Previous try result showed some failures, this is the new one. https://tbpl.mozilla.org/?tree=Try&rev=d2d99463b34d

Comment on attachment 8341524 [details] [diff] [review] bug919979.patch Review of attachment 8341524 [details] [diff] [review]: ----------------------------------------------------------------- ::: media/webrtc/trunk/webrtc/modules/video_coding/codecs/vp8/vp8_impl.cc @@ +170,5 @@ > // allocate memory for encoded image > if (encoded_image_._buffer != NULL) { > delete [] encoded_image_._buffer; > } > + // Reserve more buffer for small resolution overhead. It's not clear this comment applies to the magic number "100". Perhaps: // Reserve 100 extra bytes for overhead at small resolutions.

Addressed review comments.