From d664836efa2f51edc6d7e277f007d47ae97ba7ff Mon Sep 17 00:00:00 2001 From: hbos Date: Thu, 21 Jan 2016 05:43:11 -0800 Subject: [PATCH] Added EncodedImage::GetBufferPaddingBytes. The FFmpeg video decoder requires up to 8 additional bytes to be allocated for its encoded image buffer input, due to optimized byte readers over-reading on some platforms. We plan to use FFmpeg for a soon-to-land H.264 enc/dec. This CL adds support for padding encoded image buffers based on codec type, and makes sure calls to VCMEncodedFrame::VerifyAndAllocate use the padding. All padding constants are 0 but making H.264 pad with 8 bytes will be a one-line change. Also, added -framework CoreFoundation to webrtc_h264_video_toolbox which was missing. BUG=chromium:468365 BUG=https://bugs.chromium.org/p/webrtc/issues/detail?id=5424 NOTRY=True Review URL: https://codereview.webrtc.org/1602523004 Cr-Commit-Position: refs/heads/master@{#11337} --- webrtc/common_video/video_frame.cc | 22 +++++++++++++++++++ .../video_coding/codecs/h264/h264.gypi | 1 + webrtc/modules/video_coding/encoded_frame.cc | 6 +++-- webrtc/modules/video_coding/frame_buffer.cc | 3 ++- webrtc/video_frame.h | 6 +++++ 5 files changed, 35 insertions(+), 3 deletions(-) diff --git a/webrtc/common_video/video_frame.cc b/webrtc/common_video/video_frame.cc index 8ccd821d09..3bf59a720c 100644 --- a/webrtc/common_video/video_frame.cc +++ b/webrtc/common_video/video_frame.cc @@ -19,6 +19,10 @@ namespace webrtc { +// TODO(hbos): The FFmpeg video decoder will require up to 8 bytes, update this +// when the FFmpeg decoder is added. +const size_t EncodedImage::kBufferPaddingBytesH264 = 0; + bool EqualPlane(const uint8_t* data1, const uint8_t* data2, int stride, @@ -242,4 +246,22 @@ bool VideoFrame::EqualsFrame(const VideoFrame& frame) const { stride(kVPlane), half_width, half_height); } +size_t EncodedImage::GetBufferPaddingBytes(VideoCodecType codec_type) { + switch (codec_type) { + case kVideoCodecVP8: + case kVideoCodecVP9: + return 0; + case kVideoCodecH264: + return kBufferPaddingBytesH264; + case kVideoCodecI420: + case kVideoCodecRED: + case kVideoCodecULPFEC: + case kVideoCodecGeneric: + case kVideoCodecUnknown: + return 0; + } + RTC_NOTREACHED(); + return 0; +} + } // namespace webrtc diff --git a/webrtc/modules/video_coding/codecs/h264/h264.gypi b/webrtc/modules/video_coding/codecs/h264/h264.gypi index e0c32de0f8..78f0be37a7 100644 --- a/webrtc/modules/video_coding/codecs/h264/h264.gypi +++ b/webrtc/modules/video_coding/codecs/h264/h264.gypi @@ -50,6 +50,7 @@ 'link_settings': { 'xcode_settings': { 'OTHER_LDFLAGS': [ + '-framework CoreFoundation', '-framework CoreMedia', '-framework CoreVideo', '-framework VideoToolbox', diff --git a/webrtc/modules/video_coding/encoded_frame.cc b/webrtc/modules/video_coding/encoded_frame.cc index 261074ae73..cc3a91eeac 100644 --- a/webrtc/modules/video_coding/encoded_frame.cc +++ b/webrtc/modules/video_coding/encoded_frame.cc @@ -41,7 +41,8 @@ VCMEncodedFrame::VCMEncodedFrame(const webrtc::EncodedImage& rhs) _size = 0; _length = 0; if (rhs._buffer != NULL) { - VerifyAndAllocate(rhs._length); + VerifyAndAllocate(rhs._length + + EncodedImage::GetBufferPaddingBytes(_codec)); memcpy(_buffer, rhs._buffer, rhs._length); } } @@ -60,7 +61,8 @@ VCMEncodedFrame::VCMEncodedFrame(const VCMEncodedFrame& rhs) _size = 0; _length = 0; if (rhs._buffer != NULL) { - VerifyAndAllocate(rhs._length); + VerifyAndAllocate(rhs._length + + EncodedImage::GetBufferPaddingBytes(_codec)); memcpy(_buffer, rhs._buffer, rhs._length); _length = rhs._length; } diff --git a/webrtc/modules/video_coding/frame_buffer.cc b/webrtc/modules/video_coding/frame_buffer.cc index b6ddeda4e7..11db7853bf 100644 --- a/webrtc/modules/video_coding/frame_buffer.cc +++ b/webrtc/modules/video_coding/frame_buffer.cc @@ -105,7 +105,8 @@ VCMFrameBufferEnum VCMFrameBuffer::InsertPacket( uint32_t requiredSizeBytes = Length() + packet.sizeBytes + - (packet.insertStartCode ? kH264StartCodeLengthBytes : 0); + (packet.insertStartCode ? kH264StartCodeLengthBytes : 0) + + EncodedImage::GetBufferPaddingBytes(packet.codec); if (requiredSizeBytes >= _size) { const uint8_t* prevBuffer = _buffer; const uint32_t increments = diff --git a/webrtc/video_frame.h b/webrtc/video_frame.h index 9d2ed9fd4d..39ba8ee0e8 100644 --- a/webrtc/video_frame.h +++ b/webrtc/video_frame.h @@ -173,6 +173,12 @@ class VideoFrame { // TODO(pbos): Rename EncodedFrame and reformat this class' members. class EncodedImage { public: + static const size_t kBufferPaddingBytesH264; + + // Some decoders require encoded image buffers to be padded with a small + // number of additional bytes (due to over-reading byte readers). + static size_t GetBufferPaddingBytes(VideoCodecType codec_type); + EncodedImage() : EncodedImage(nullptr, 0, 0) {} EncodedImage(uint8_t* buffer, size_t length, size_t size) : _buffer(buffer), _length(length), _size(size) {}