Delete EncodedImage::GetBufferPaddingBytes

For the ffmpeg H.264 decoder, rely on ffmpeg being configured with
CONFIG_SAFE_BITSTREAM_READER.

Bug: webrtc:9378
Change-Id: Ia7a46580d520808e36581252a95feeb5f9c57bf9
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/119665
Reviewed-by: Stefan Holmer <stefan@webrtc.org>
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Reviewed-by: Sergey Silkin <ssilkin@webrtc.org>
Commit-Queue: Niels Moller <nisse@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#27124}
This commit is contained in:
Niels Möller
2019-03-08 11:26:58 +01:00
committed by Commit Bot
parent 1f4173e420
commit 009ab3c438
9 changed files with 22 additions and 50 deletions

View File

@ -251,17 +251,6 @@ int32_t H264DecoderImpl::Decode(const EncodedImage& input_image,
return WEBRTC_VIDEO_CODEC_ERR_PARAMETER;
}
// FFmpeg requires padding due to some optimized bitstream readers reading 32
// or 64 bits at once and could read over the end. See avcodec_decode_video2.
RTC_CHECK_GE(input_image.capacity(),
input_image.size() +
EncodedImage::GetBufferPaddingBytes(kVideoCodecH264));
// "If the first 23 bits of the additional bytes are not 0, then damaged MPEG
// bitstreams could cause overread and segfault." See
// AV_INPUT_BUFFER_PADDING_SIZE. We'll zero the entire padding just in case.
memset(input_image.mutable_data() + input_image.size(), 0,
EncodedImage::GetBufferPaddingBytes(kVideoCodecH264));
AVPacket packet;
av_init_packet(&packet);
packet.data = input_image.mutable_data();

View File

@ -16,6 +16,20 @@
#include "modules/video_coding/codecs/h264/include/h264.h"
// CAVEAT: According to ffmpeg docs for avcodec_send_packet, ffmpeg requires a
// few extra padding bytes after the end of input. And in addition, docs for
// AV_INPUT_BUFFER_PADDING_SIZE says "If the first 23 bits of the additional
// bytes are not 0, then damaged MPEG bitstreams could cause overread and
// segfault."
//
// WebRTC doesn't ensure any such padding, and REQUIRES ffmpeg to be compiled
// with CONFIG_SAFE_BITSTREAM_READER, which is intended to eliminate
// out-of-bounds reads. ffmpeg docs doesn't say explicitly what effects this
// flag has on the h.264 decoder or avcodec_send_packet, though, so this is in
// some way depending on undocumented behavior. If any problems turn up, we may
// have to add an extra copy operation, to enforce padding before buffers are
// passed to ffmpeg.
extern "C" {
#include "third_party/ffmpeg/libavcodec/avcodec.h"
} // extern "C"

View File

@ -169,10 +169,8 @@ EncodedImage MultiplexEncodedImagePacker::PackAndRelease(
frame_header.component_index = images[i].component_index;
frame_header.bitstream_offset = bitstream_offset;
const size_t padding =
EncodedImage::GetBufferPaddingBytes(images[i].codec_type);
frame_header.bitstream_length =
static_cast<uint32_t>(images[i].encoded_image.size() + padding);
static_cast<uint32_t>(images[i].encoded_image.size());
bitstream_offset += frame_header.bitstream_length;
frame_header.codec_type = images[i].codec_type;
@ -267,9 +265,8 @@ MultiplexImage MultiplexEncodedImagePacker::Unpack(
encoded_image.set_buffer(
combined_image.mutable_data() + frame_headers[i].bitstream_offset,
static_cast<size_t>(frame_headers[i].bitstream_length));
const size_t padding =
EncodedImage::GetBufferPaddingBytes(image_component.codec_type);
encoded_image.set_size(encoded_image.capacity() - padding);
encoded_image.set_size(encoded_image.capacity());
image_component.encoded_image = encoded_image;

View File

@ -558,11 +558,9 @@ const webrtc::EncodedImage* VideoProcessor::BuildAndStoreSuperframe(
}
}
const size_t payload_size_bytes = base_image.size() + encoded_image.size();
const size_t buffer_size_bytes =
payload_size_bytes + EncodedImage::GetBufferPaddingBytes(codec);
EncodedImage copied_image = encoded_image;
copied_image.Allocate(buffer_size_bytes);
copied_image.Allocate(payload_size_bytes);
if (base_image.size()) {
RTC_CHECK(base_image.data());
memcpy(copied_image.data(), base_image.data(), base_image.size());