Always round up coded size to avoid VideoFrame restrictions.

Prior to http://crrev.com/268831 we never considered the actual
coded size when constructing buffers for FFmpeg.  We do now, but
were not aligning the extents properly.

Sadly we had no test coverage for odd sized videos :(  I've fixed
the extents and added a test using one of the bug clips (which
will be landed separately).

BUG=379127
TEST=new pipeline test
NOTRY=true

Review URL: https://codereview.chromium.org/318133002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@275719 0039d316-1c4b-4281-b951-d872f2087c98
parent 0f647935
...@@ -66,6 +66,12 @@ static void ReleaseVideoBufferImpl(void* opaque, uint8* data) { ...@@ -66,6 +66,12 @@ static void ReleaseVideoBufferImpl(void* opaque, uint8* data) {
video_frame.swap(reinterpret_cast<VideoFrame**>(&opaque)); video_frame.swap(reinterpret_cast<VideoFrame**>(&opaque));
} }
static size_t RoundUp(size_t value, size_t alignment) {
// Check that |alignment| is a power of 2.
DCHECK((alignment + (alignment - 1)) == (alignment | (alignment - 1)));
return ((value + (alignment - 1)) & ~(alignment - 1));
}
FFmpegVideoDecoder::FFmpegVideoDecoder( FFmpegVideoDecoder::FFmpegVideoDecoder(
const scoped_refptr<base::SingleThreadTaskRunner>& task_runner) const scoped_refptr<base::SingleThreadTaskRunner>& task_runner)
: task_runner_(task_runner), state_(kUninitialized), : task_runner_(task_runner), state_(kUninitialized),
...@@ -105,9 +111,13 @@ int FFmpegVideoDecoder::GetVideoBuffer(struct AVCodecContext* codec_context, ...@@ -105,9 +111,13 @@ int FFmpegVideoDecoder::GetVideoBuffer(struct AVCodecContext* codec_context,
// //
// When lowres is non-zero, dimensions should be divided by 2^(lowres), but // When lowres is non-zero, dimensions should be divided by 2^(lowres), but
// since we don't use this, just DCHECK that it's zero. // since we don't use this, just DCHECK that it's zero.
//
// Always round up to a multiple of two to match VideoFrame restrictions on
// frame alignment.
DCHECK_EQ(codec_context->lowres, 0); DCHECK_EQ(codec_context->lowres, 0);
gfx::Size coded_size(std::max(size.width(), codec_context->coded_width), gfx::Size coded_size(
std::max(size.height(), codec_context->coded_height)); RoundUp(std::max(size.width(), codec_context->coded_width), 2),
RoundUp(std::max(size.height(), codec_context->coded_height), 2));
if (!VideoFrame::IsValidConfig( if (!VideoFrame::IsValidConfig(
format, coded_size, gfx::Rect(size), natural_size)) format, coded_size, gfx::Rect(size), natural_size))
......
...@@ -1485,6 +1485,14 @@ TEST_F(PipelineIntegrationTest, P444_VP9_WebM) { ...@@ -1485,6 +1485,14 @@ TEST_F(PipelineIntegrationTest, P444_VP9_WebM) {
EXPECT_EQ(last_video_frame_format_, VideoFrame::YV24); EXPECT_EQ(last_video_frame_format_, VideoFrame::YV24);
} }
// Verify that videos with an odd frame size playback successfully.
TEST_F(PipelineIntegrationTest, BasicPlayback_OddVideoSize) {
ASSERT_TRUE(Start(GetTestDataFilePath("butterfly-853x480.webm"),
PIPELINE_OK));
Play();
ASSERT_TRUE(WaitUntilOnEnded());
}
// For MediaSource tests, generate two sets of tests: one using FrameProcessor, // For MediaSource tests, generate two sets of tests: one using FrameProcessor,
// and one using LegacyFrameProcessor. // and one using LegacyFrameProcessor.
INSTANTIATE_TEST_CASE_P(NewFrameProcessor, PipelineIntegrationTest, INSTANTIATE_TEST_CASE_P(NewFrameProcessor, PipelineIntegrationTest,
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment