diff options
author | posciak@chromium.org <posciak@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2014-02-13 04:37:51 +0000 |
---|---|---|
committer | posciak@chromium.org <posciak@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2014-02-13 04:37:51 +0000 |
commit | 986c2f3214cc7158b71bba10fb8c37130ea20140 (patch) | |
tree | 58e543ef1cd27f998f3e149742b6a0ab2dd332fa | |
parent | 223008eb0d750fd83ad845c2f4cc4f64e12b0a77 (diff) | |
download | chromium_src-986c2f3214cc7158b71bba10fb8c37130ea20140.zip chromium_src-986c2f3214cc7158b71bba10fb8c37130ea20140.tar.gz chromium_src-986c2f3214cc7158b71bba10fb8c37130ea20140.tar.bz2 |
V4L2VDA: Fix a few bugs related to Reset() and buffer management.
- If a Reset() arrived before the first Decode(), or, in general, if it
arrived before V4L2VDA requested buffers, it would assume it had already
have allocated buffers and go back to kDecoding before resuming.
Fix this by checking if the buffers are allocated before resuming and go
back to kInitialized state, instead of kAfterReset. This only works because
we also now make buffer allocation an "atomic" operation for the decoder
thread (see below).
- If a Reset() arrived after V4L2VDA had a chance to request buffers, but
before AssignPictureBuffers() and AssignPictureBuffersTask() resulted in
finishing the buffer allocation sequence, ResetTask would call
StopDevicePoll(), assume the buffers were good for reuse, and queue them to
the free output buffer queue. They would still be missing textures though
and not be ready for reuse, but the decoding would continue. Moreover, the
AssignPictureBuffersTask() would re-queue them to the free queue again.
Fix this by simply making the decoder thread wait for AssignPictureBuffers
before continuing decoding. This doesn't have too much perf impact and
should solve some other potential corner cases, similar to the one above.
With this, we don't need the AssignPictureBuffersTask anymore, and can
do everything in AssignPictureBuffers, while the decoder thread waits.
Separately, don't bail out with an error in a situation when we've already
posted a DismissPictureBuffer for a PictureBuffer to the client and cleared
the corresponding metadata, but it has not yet executed in client before
it had a chance to post a ReusePictureBuffer for it back to us.
Verifying that this exact situation occurred however and not a random
picture id was returned is difficult/impossible and not worth it in
general, so just ignore such buffers (they will have been already freed).
Finally, mark all buffers as not in device after streamoff, and not only
those that are known to be in client; V4L2 spec mandates that the device
is to drop ownership of all queued buffers regardless of whether we dequeue
them in this case.
To test these situations:
- enable the ResetBeforeDecode test for V4L2VDA; this tests for Reset()s
before the Decode() with first config info is called;
- add a new ResetAfterFirstConfigChange case to test situations where
Reset() is called immediately after Decode() with first config info, which
tests how VDA handles Reset() after it had a chance to request buffers,
but before receiving them back.
Also, fix ResetBeforeDecode test to work again, as it seems to have been
accidentally broken by the recent refactoring.
Finally, if a stream is shorter than kMaxResetAfterFrameNum, reset in the
middle of it, instead of resetting after first frame arrives.
BUG=319495,chrome-os-partner:21063
TEST=vdatest, MSE tests, seek tests, res switch tests, youtube with &t=1m
R=fischman@chromium.org, sheu@chromium.org
Review URL: https://codereview.chromium.org/151293003
git-svn-id: svn://svn.chromium.org/chrome/branches/1750/src@250948 0039d316-1c4b-4281-b951-d872f2087c98
3 files changed, 172 insertions, 86 deletions
diff --git a/content/common/gpu/media/exynos_video_decode_accelerator.cc b/content/common/gpu/media/exynos_video_decode_accelerator.cc index b74d4d2..a046104 100644 --- a/content/common/gpu/media/exynos_video_decode_accelerator.cc +++ b/content/common/gpu/media/exynos_video_decode_accelerator.cc @@ -209,6 +209,7 @@ ExynosVideoDecodeAccelerator::ExynosVideoDecodeAccelerator( mfc_output_buffer_pixelformat_(0), mfc_output_dpb_size_(0), picture_clearing_count_(0), + pictures_assigned_(false, false), device_poll_thread_("ExynosDevicePollThread"), device_poll_interrupt_fd_(-1), make_context_current_(make_context_current), @@ -387,6 +388,8 @@ void ExynosVideoDecodeAccelerator::AssignPictureBuffers( return; } + // It's safe to manipulate all the buffer state here, because the decoder + // thread is waiting on pictures_assigned_. scoped_ptr<PictureBufferArrayRef> picture_buffers_ref( new PictureBufferArrayRef(egl_display_)); gfx::ScopedTextureBinder bind_restore(GL_TEXTURE_EXTERNAL_OES, 0); @@ -421,11 +424,29 @@ void ExynosVideoDecodeAccelerator::AssignPictureBuffers( picture_buffers_ref->picture_buffers.push_back( PictureBufferArrayRef::PictureBufferRef(egl_image, buffers[i].id())); } - decoder_thread_.message_loop()->PostTask( - FROM_HERE, - base::Bind(&ExynosVideoDecodeAccelerator::AssignPictureBuffersTask, - base::Unretained(this), - base::Passed(&picture_buffers_ref))); + + DCHECK(free_output_buffers_.empty()); + DCHECK_EQ(picture_buffers_ref->picture_buffers.size(), + mfc_output_buffer_map_.size()); + for (size_t i = 0; i < mfc_output_buffer_map_.size(); ++i) { + OutputRecord& output_record = mfc_output_buffer_map_[i]; + PictureBufferArrayRef::PictureBufferRef& buffer_ref = + picture_buffers_ref->picture_buffers[i]; + // We should be blank right now. + DCHECK(!output_record.at_device); + DCHECK(!output_record.at_client); + DCHECK_EQ(output_record.egl_image, EGL_NO_IMAGE_KHR); + DCHECK_EQ(output_record.egl_sync, EGL_NO_SYNC_KHR); + DCHECK_EQ(output_record.picture_id, -1); + DCHECK_EQ(output_record.cleared, false); + output_record.egl_image = buffer_ref.egl_image; + output_record.picture_id = buffer_ref.picture_id; + free_output_buffers_.push(i); + DVLOG(3) << "AssignPictureBuffers(): buffer[" << i + << "]: picture_id=" << buffer_ref.picture_id; + } + picture_buffers_ref->picture_buffers.clear(); + pictures_assigned_.Signal(); } void ExynosVideoDecodeAccelerator::ReusePictureBuffer(int32 picture_buffer_id) { @@ -479,6 +500,7 @@ void ExynosVideoDecodeAccelerator::Destroy() { if (decoder_thread_.IsRunning()) { decoder_thread_.message_loop()->PostTask(FROM_HERE, base::Bind( &ExynosVideoDecodeAccelerator::DestroyTask, base::Unretained(this))); + pictures_assigned_.Signal(); // DestroyTask() will cause the decoder_thread_ to flush all tasks. decoder_thread_.Stop(); } else { @@ -511,7 +533,7 @@ void ExynosVideoDecodeAccelerator::DecodeTask( NOTIFY_ERROR(UNREADABLE_INPUT); return; } - DVLOG(3) << "Decode(): mapped to addr=" << bitstream_record->shm->memory(); + DVLOG(3) << "DecodeTask(): mapped at=" << bitstream_record->shm->memory(); if (decoder_state_ == kResetting || decoder_flushing_) { // In the case that we're resetting or flushing, we need to delay decoding @@ -926,46 +948,6 @@ bool ExynosVideoDecodeAccelerator::FlushInputFrame() { return (decoder_state_ != kError); } -void ExynosVideoDecodeAccelerator::AssignPictureBuffersTask( - scoped_ptr<PictureBufferArrayRef> pic_buffers) { - DVLOG(3) << "AssignPictureBuffersTask()"; - DCHECK_EQ(decoder_thread_.message_loop(), base::MessageLoop::current()); - DCHECK_NE(decoder_state_, kUninitialized); - TRACE_EVENT0("Video Decoder", "EVDA::AssignPictureBuffersTask"); - - // We run AssignPictureBuffersTask even if we're in kResetting. - if (decoder_state_ == kError) { - DVLOG(2) << "AssignPictureBuffersTask(): early out: kError state"; - return; - } - - DCHECK_EQ(pic_buffers->picture_buffers.size(), mfc_output_buffer_map_.size()); - for (size_t i = 0; i < mfc_output_buffer_map_.size(); ++i) { - MfcOutputRecord& output_record = mfc_output_buffer_map_[i]; - PictureBufferArrayRef::PictureBufferRef& buffer_ref = - pic_buffers->picture_buffers[i]; - // We should be blank right now. - DCHECK(!output_record.at_device); - DCHECK(!output_record.at_client); - DCHECK_EQ(output_record.egl_image, EGL_NO_IMAGE_KHR); - DCHECK_EQ(output_record.egl_sync, EGL_NO_SYNC_KHR); - DCHECK_EQ(output_record.picture_id, -1); - DCHECK_EQ(output_record.cleared, false); - output_record.egl_image = buffer_ref.egl_image; - output_record.picture_id = buffer_ref.picture_id; - mfc_free_output_buffers_.push(i); - DVLOG(3) << "AssignPictureBuffersTask(): buffer[" << i - << "]: picture_id=" << buffer_ref.picture_id; - } - pic_buffers->picture_buffers.clear(); - - // We got buffers! Kick the MFC. - EnqueueMfc(); - - if (decoder_state_ == kChangingResolution) - ResumeAfterResolutionChange(); -} - void ExynosVideoDecodeAccelerator::ServiceDeviceTask(bool mfc_event_pending) { DVLOG(3) << "ServiceDeviceTask()"; DCHECK_EQ(decoder_thread_.message_loop(), base::MessageLoop::current()); @@ -1200,7 +1182,7 @@ bool ExynosVideoDecodeAccelerator::EnqueueMfcInputRecord() { input_record.at_device = true; mfc_input_buffer_queued_count_++; DVLOG(3) << "EnqueueMfcInputRecord(): enqueued input_id=" - << input_record.input_id; + << input_record.input_id << " size=" << input_record.bytes_used; return true; } @@ -1266,8 +1248,13 @@ void ExynosVideoDecodeAccelerator::ReusePictureBufferTask( break; if (index >= mfc_output_buffer_map_.size()) { - DLOG(ERROR) << "ReusePictureBufferTask(): picture_buffer_id not found"; - NOTIFY_ERROR(INVALID_ARGUMENT); + // It's possible that we've already posted a DismissPictureBuffer for this + // picture, but it has not yet executed when this ReusePictureBuffer was + // posted to us by the client. In that case just ignore this (we've already + // dismissed it and accounted for that) and let the sync object get + // destroyed. + DVLOG(4) << "ReusePictureBufferTask(): got picture id= " + << picture_buffer_id << " not in use (anymore?)."; return; } @@ -1279,6 +1266,7 @@ void ExynosVideoDecodeAccelerator::ReusePictureBufferTask( } DCHECK_EQ(output_record.egl_sync, EGL_NO_SYNC_KHR); + DCHECK(!output_record.at_device); output_record.at_client = false; output_record.egl_sync = egl_sync_ref->egl_sync; mfc_free_output_buffers_.push(index); @@ -1431,7 +1419,14 @@ void ExynosVideoDecodeAccelerator::ResetDoneTask() { // Jobs drained, we're finished resetting. DCHECK_EQ(decoder_state_, kResetting); - decoder_state_ = kAfterReset; + if (mfc_output_buffer_map_.empty()) { + // We must have gotten Reset() before we had a chance to request buffers + // from the client. + decoder_state_ = kInitialized; + } else { + decoder_state_ = kAfterReset; + } + decoder_partial_frame_pending_ = false; decoder_delay_bitstream_buffer_id_ = -1; child_message_loop_proxy_->PostTask(FROM_HERE, base::Bind( @@ -1521,15 +1516,22 @@ bool ExynosVideoDecodeAccelerator::StopDevicePoll(bool keep_mfc_input_state) { } mfc_input_buffer_queued_count_ = 0; } + while (!mfc_free_output_buffers_.empty()) mfc_free_output_buffers_.pop(); + for (size_t i = 0; i < mfc_output_buffer_map_.size(); ++i) { MfcOutputRecord& output_record = mfc_output_buffer_map_[i]; - // Only mark those free that aren't being held by the VDA client. + DCHECK(!(output_record.at_client && output_record.at_device)); + + // After streamoff, the device drops ownership of all buffers, even if + // we don't dequeue them explicitly. + mfc_output_buffer_map_[i].at_device = false; + // Some of them may still be owned by the client however. + // Reuse only those that aren't. if (!output_record.at_client) { DCHECK_EQ(output_record.egl_sync, EGL_NO_SYNC_KHR); mfc_free_output_buffers_.push(i); - mfc_output_buffer_map_[i].at_device = false; } } mfc_output_buffer_queued_count_ = 0; @@ -1595,6 +1597,7 @@ void ExynosVideoDecodeAccelerator::StartResolutionChangeIfNeeded() { void ExynosVideoDecodeAccelerator::FinishResolutionChange() { DCHECK_EQ(decoder_thread_.message_loop(), base::MessageLoop::current()); + DCHECK_EQ(decoder_state_, kChangingResolution); DVLOG(3) << "FinishResolutionChange()"; if (decoder_state_ == kError) { @@ -1617,8 +1620,7 @@ void ExynosVideoDecodeAccelerator::FinishResolutionChange() { return; } - // From here we stay in kChangingResolution and wait for - // AssignPictureBuffers() before we can resume. + ResumeAfterResolutionChange(); } void ExynosVideoDecodeAccelerator::ResumeAfterResolutionChange() { @@ -1726,7 +1728,7 @@ bool ExynosVideoDecodeAccelerator::GetFormatInfo(struct v4l2_format* format, *again = true; return true; } else { - DPLOG(ERROR) << "DecodeBufferInitial(): ioctl() failed: VIDIOC_G_FMT"; + DPLOG(ERROR) << __func__ << "(): ioctl() failed: VIDIOC_G_FMT"; NOTIFY_ERROR(PLATFORM_FAILURE); return false; } @@ -1865,6 +1867,22 @@ bool ExynosVideoDecodeAccelerator::CreateMfcOutputBuffers() { frame_buffer_size_, GL_TEXTURE_EXTERNAL_OES)); + // Wait for the client to call AssignPictureBuffers() on the Child thread. + // We do this, because if we continue decoding without finishing buffer + // allocation, we may end up Resetting before AssignPictureBuffers arrives, + // resulting in unnecessary complications and subtle bugs. + // For example, if the client calls Decode(Input1), Reset(), Decode(Input2) + // in a sequence, and Decode(Input1) results in us getting here and exiting + // without waiting, we might end up running Reset{,Done}Task() before + // AssignPictureBuffers is scheduled, thus cleaning up and pushing buffers + // to the free_output_buffers_ map twice. If we somehow marked buffers as + // not ready, we'd need special handling for restarting the second Decode + // task and delaying it anyway. + // Waiting here is not very costly and makes reasoning about different + // situations much simpler. + pictures_assigned_.Wait(); + + Enqueue(); return true; } diff --git a/content/common/gpu/media/exynos_video_decode_accelerator.h b/content/common/gpu/media/exynos_video_decode_accelerator.h index 7ccd594..71b5d66 100644 --- a/content/common/gpu/media/exynos_video_decode_accelerator.h +++ b/content/common/gpu/media/exynos_video_decode_accelerator.h @@ -14,6 +14,7 @@ #include "base/callback_forward.h" #include "base/memory/linked_ptr.h" #include "base/memory/scoped_ptr.h" +#include "base/synchronization/waitable_event.h" #include "base/threading/thread.h" #include "content/common/content_export.h" #include "content/common/gpu/media/video_decode_accelerator_impl.h" @@ -46,16 +47,27 @@ class H264Parser; // media::VideoDecodeAccelerator::Client interface. // * The decoder_thread_, owned by this class. It services API tasks, through // the *Task() routines, as well as V4L2 device events, through -// ServiceDeviceTask(). Almost all state modification is done on this thread. +// ServiceDeviceTask(). Almost all state modification is done on this thread +// (this doesn't include buffer (re)allocation sequence, see below). // * The device_poll_thread_, owned by this class. All it does is epoll() on // the V4L2 in DevicePollTask() and schedule a ServiceDeviceTask() on the // decoder_thread_ when something interesting happens. // TODO(sheu): replace this thread with an TYPE_IO decoder_thread_. // -// Note that this class has no locks! Everything's serviced on the -// decoder_thread_, so there are no synchronization issues. +// Note that this class has (almost) no locks, apart from the pictures_assigned_ +// WaitableEvent. Everything (apart from buffer (re)allocation) is serviced on +// the decoder_thread_, so there are no synchronization issues. // ... well, there are, but it's a matter of getting messages posted in the // right order, not fiddling with locks. +// Buffer creation is a two-step process that is serviced partially on the +// Child thread, because we need to wait for the client to provide textures +// for the buffers we allocate. We cannot keep the decoder thread running while +// the client allocates Pictures for us, because we need to REQBUFS first to get +// the required number of output buffers from the device and that cannot be done +// unless we free the previous set of buffers, leaving the decoding in a +// inoperable state for the duration of the wait for Pictures. So to prevent +// subtle races (esp. if we get Reset() in the meantime), we block the decoder +// thread while we wait for AssignPictureBuffers from the client. class CONTENT_EXPORT ExynosVideoDecodeAccelerator : public VideoDecodeAcceleratorImpl { public: @@ -122,7 +134,7 @@ class CONTENT_EXPORT ExynosVideoDecodeAccelerator struct BitstreamBufferRef; // Auto-destruction reference for an array of PictureBuffer, for - // message-passing from AssignPictureBuffers() to AssignPictureBuffersTask(). + // simpler EGLImage cleanup if any calls fail in AssignPictureBuffers(). struct PictureBufferArrayRef; // Auto-destruction reference for EGLSync (for message-passing). @@ -185,11 +197,6 @@ class CONTENT_EXPORT ExynosVideoDecodeAccelerator // Flush data for one decoded frame. bool FlushInputFrame(); - // Process an AssignPictureBuffers() API call. After this, the - // device_poll_thread_ can be started safely, since we have all our - // buffers. - void AssignPictureBuffersTask(scoped_ptr<PictureBufferArrayRef> pic_buffers); - // Service I/O on the V4L2 devices. This task should only be scheduled from // DevicePollTask(). If |mfc_event_pending| is true, one or more events // on MFC file descriptor are pending. @@ -357,6 +364,9 @@ class CONTENT_EXPORT ExynosVideoDecodeAccelerator // // Hardware state and associated queues. Since decoder_thread_ services // the hardware, decoder_thread_ owns these too. + // mfc_output_buffer_map_ and free_output_buffers_ are an exception during the + // buffer (re)allocation sequence, when the decoder_thread_ is blocked briefly + // while the Child thread manipulates them. // // Completed decode buffers, waiting for MFC. @@ -394,6 +404,10 @@ class CONTENT_EXPORT ExynosVideoDecodeAccelerator // The number of pictures that are sent to PictureReady and will be cleared. int picture_clearing_count_; + // Used by the decoder thread to wait for AssignPictureBuffers to arrive + // to avoid races with potential Reset requests. + base::WaitableEvent pictures_assigned_; + // Output picture size. gfx::Size frame_buffer_size_; diff --git a/content/common/gpu/media/video_decode_accelerator_unittest.cc b/content/common/gpu/media/video_decode_accelerator_unittest.cc index 7c1f077..bc1c282 100644 --- a/content/common/gpu/media/video_decode_accelerator_unittest.cc +++ b/content/common/gpu/media/video_decode_accelerator_unittest.cc @@ -49,6 +49,7 @@ #include "content/common/gpu/media/rendering_helper.h" #include "content/common/gpu/media/video_accelerator_unittest_helpers.h" #include "content/public/common/content_switches.h" +#include "media/filters/h264_parser.h" #include "ui/gfx/codec/png_codec.h" #if defined(OS_WIN) @@ -104,6 +105,8 @@ bool g_disable_rendering = false; // Magic constants for differentiating the reasons for NotifyResetDone being // called. enum ResetPoint { + // Reset() just after calling Decode() with a fragment containing config info. + RESET_AFTER_FIRST_CONFIG_INFO = -4, START_OF_STREAM_RESET = -3, MID_STREAM_RESET = -2, END_OF_STREAM_RESET = -1 @@ -124,7 +127,7 @@ struct TestVideoFile { num_fragments(-1), min_fps_render(-1), min_fps_no_render(-1), - profile(-1), + profile(media::VIDEO_CODEC_PROFILE_UNKNOWN), reset_after_frame_num(END_OF_STREAM_RESET) { } @@ -135,7 +138,7 @@ struct TestVideoFile { int num_fragments; int min_fps_render; int min_fps_no_render; - int profile; + media::VideoCodecProfile profile; int reset_after_frame_num; std::string data_str; }; @@ -376,7 +379,7 @@ class GLRenderingVDAClient int delete_decoder_state, int frame_width, int frame_height, - int profile, + media::VideoCodecProfile profile, double rendering_fps, bool suppress_rendering, int delay_reuse_after_frame_num, @@ -455,7 +458,7 @@ class GLRenderingVDAClient int num_done_bitstream_buffers_; PictureBufferById picture_buffers_by_id_; base::TimeTicks initialize_done_ticks_; - int profile_; + media::VideoCodecProfile profile_; GLenum texture_target_; bool suppress_rendering_; std::vector<base::TimeTicks> frame_delivery_times_; @@ -482,7 +485,7 @@ GLRenderingVDAClient::GLRenderingVDAClient( int delete_decoder_state, int frame_width, int frame_height, - int profile, + media::VideoCodecProfile profile, double rendering_fps, bool suppress_rendering, int delay_reuse_after_frame_num, @@ -567,10 +570,9 @@ void GLRenderingVDAClient::CreateAndStartDecoder() { return; // Configure the decoder. - media::VideoCodecProfile profile = media::H264PROFILE_BASELINE; - if (profile_ != -1) - profile = static_cast<media::VideoCodecProfile>(profile_); - CHECK(decoder_->Initialize(profile)); + profile_ = (profile_ != media::VIDEO_CODEC_PROFILE_UNKNOWN ? + profile_ : media::H264PROFILE_BASELINE); + CHECK(decoder_->Initialize(profile_)); } void GLRenderingVDAClient::ProvidePictureBuffers( @@ -663,6 +665,7 @@ void GLRenderingVDAClient::NotifyInitializeDone() { initialize_done_ticks_ = base::TimeTicks::Now(); if (reset_after_frame_num_ == START_OF_STREAM_RESET) { + reset_after_frame_num_ = MID_STREAM_RESET; decoder_->Reset(); return; } @@ -834,6 +837,29 @@ std::string GLRenderingVDAClient::GetBytesForNextFrame( return bytes; } +static bool FragmentHasConfigInfo(const uint8* data, size_t size, + media::VideoCodecProfile profile) { + if (profile >= media::H264PROFILE_MIN && + profile <= media::H264PROFILE_MAX) { + media::H264Parser parser; + parser.SetStream(data, size); + media::H264NALU nalu; + media::H264Parser::Result result = parser.AdvanceToNextNALU(&nalu); + if (result != media::H264Parser::kOk) { + // Let the VDA figure out there's something wrong with the stream. + return false; + } + + return nalu.nal_unit_type == media::H264NALU::kSPS; + } else if (profile >= media::VP8PROFILE_MIN && + profile <= media::VP8PROFILE_MAX) { + return (size > 0 && !(data[0] & 0x01)); + } + + CHECK(false) << "Invalid profile"; // Shouldn't happen at this point. + return false; +} + void GLRenderingVDAClient::DecodeNextFragment() { if (decoder_deleted()) return; @@ -854,6 +880,19 @@ void GLRenderingVDAClient::DecodeNextFragment() { } size_t next_fragment_size = next_fragment_bytes.size(); + // Call Reset() just after Decode() if the fragment contains config info. + // This tests how the VDA behaves when it gets a reset request before it has + // a chance to ProvidePictureBuffers(). + bool reset_here = false; + if (reset_after_frame_num_ == RESET_AFTER_FIRST_CONFIG_INFO) { + reset_here = FragmentHasConfigInfo( + reinterpret_cast<const uint8*>(next_fragment_bytes.data()), + next_fragment_size, + profile_); + if (reset_here) + reset_after_frame_num_ = END_OF_STREAM_RESET; + } + // Populate the shared memory buffer w/ the fragment, duplicate its handle, // and hand it off to the decoder. base::SharedMemory shm; @@ -868,13 +907,20 @@ void GLRenderingVDAClient::DecodeNextFragment() { next_bitstream_buffer_id_ = (next_bitstream_buffer_id_ + 1) & 0x3FFFFFFF; decoder_->Decode(bitstream_buffer); ++outstanding_decodes_; - encoded_data_next_pos_to_decode_ = end_pos; - if (!remaining_play_throughs_ && -delete_decoder_state_ == next_bitstream_buffer_id_) { DeleteDecoder(); } + if (reset_here) { + reset_after_frame_num_ = MID_STREAM_RESET; + decoder_->Reset(); + // Restart from the beginning to re-Decode() the SPS we just sent. + encoded_data_next_pos_to_decode_ = 0; + } else { + encoded_data_next_pos_to_decode_ = end_pos; + } + if (decode_calls_per_second_ > 0) { base::MessageLoop::current()->PostDelayedTask( FROM_HERE, @@ -1006,8 +1052,10 @@ void VideoDecodeAcceleratorTest::ParseAndReadTestVideoData( CHECK(base::StringToInt(fields[5], &video_file->min_fps_render)); if (!fields[6].empty()) CHECK(base::StringToInt(fields[6], &video_file->min_fps_no_render)); + int profile = -1; if (!fields[7].empty()) - CHECK(base::StringToInt(fields[7], &video_file->profile)); + CHECK(base::StringToInt(fields[7], &profile)); + video_file->profile = static_cast<media::VideoCodecProfile>(profile); // Read in the video data. base::FilePath filepath(video_file->file_name); @@ -1024,14 +1072,18 @@ void VideoDecodeAcceleratorTest::UpdateTestVideoFileParams( std::vector<TestVideoFile*>* test_video_files) { for (size_t i = 0; i < test_video_files->size(); i++) { TestVideoFile* video_file = (*test_video_files)[i]; - if (video_file->num_frames > 0 && reset_point == MID_STREAM_RESET) { - // Reset should not go beyond the last frame; reset after the first - // frame for short videos. + if (reset_point == MID_STREAM_RESET) { + // Reset should not go beyond the last frame; + // reset in the middle of the stream for short videos. video_file->reset_after_frame_num = kMaxResetAfterFrameNum; - if (video_file->num_frames <= kMaxResetAfterFrameNum) - video_file->reset_after_frame_num = 1; + if (video_file->num_frames <= video_file->reset_after_frame_num) + video_file->reset_after_frame_num = video_file->num_frames / 2; + video_file->num_frames += video_file->reset_after_frame_num; + } else { + video_file->reset_after_frame_num = reset_point; } + if (video_file->min_fps_render != -1) video_file->min_fps_render /= num_concurrent_decoders; if (video_file->min_fps_no_render != -1) @@ -1353,16 +1405,18 @@ INSTANTIATE_TEST_CASE_P( ::testing::Values( MakeTuple(1, 1, 4, END_OF_STREAM_RESET, CS_RESET, false, false))); -// This hangs on Exynos, preventing further testing and wasting test machine -// time. -// TODO(ihf): Enable again once http://crbug.com/269754 is fixed. -#if defined(ARCH_CPU_X86_FAMILY) // Test that Reset() before the first Decode() works fine. INSTANTIATE_TEST_CASE_P( ResetBeforeDecode, VideoDecodeAcceleratorParamTest, ::testing::Values( MakeTuple(1, 1, 1, START_OF_STREAM_RESET, CS_RESET, false, false))); -#endif // ARCH_CPU_X86_FAMILY + +// Test Reset() immediately after Decode() containing config info. +INSTANTIATE_TEST_CASE_P( + ResetAfterFirstConfigInfo, VideoDecodeAcceleratorParamTest, + ::testing::Values( + MakeTuple( + 1, 1, 1, RESET_AFTER_FIRST_CONFIG_INFO, CS_RESET, false, false))); // Test that Reset() mid-stream works fine and doesn't affect decoding even when // Decode() calls are made during the reset. |