summaryrefslogtreecommitdiffstats
path: root/content
diff options
context:
space:
mode:
authorposciak@chromium.org <posciak@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2014-02-05 01:20:36 +0000
committerposciak@chromium.org <posciak@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2014-02-05 01:20:36 +0000
commitafa74f1bb45de9f6180b8314afa34691273d0b88 (patch)
tree75f6dea746de86f4bc9f4d71b61887e292b79ebd /content
parentea11002ab7c971f3024cd4d95066bddf7c5bf40b (diff)
downloadchromium_src-afa74f1bb45de9f6180b8314afa34691273d0b88.zip
chromium_src-afa74f1bb45de9f6180b8314afa34691273d0b88.tar.gz
chromium_src-afa74f1bb45de9f6180b8314afa34691273d0b88.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/trunk/src@248819 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'content')
-rw-r--r--content/common/gpu/media/v4l2_video_decode_accelerator.cc128
-rw-r--r--content/common/gpu/media/v4l2_video_decode_accelerator.h32
-rw-r--r--content/common/gpu/media/video_decode_accelerator_unittest.cc98
3 files changed, 172 insertions, 86 deletions
diff --git a/content/common/gpu/media/v4l2_video_decode_accelerator.cc b/content/common/gpu/media/v4l2_video_decode_accelerator.cc
index 285c96b..92f687e 100644
--- a/content/common/gpu/media/v4l2_video_decode_accelerator.cc
+++ b/content/common/gpu/media/v4l2_video_decode_accelerator.cc
@@ -207,6 +207,7 @@ V4L2VideoDecodeAccelerator::V4L2VideoDecodeAccelerator(
output_buffer_pixelformat_(0),
output_dpb_size_(0),
picture_clearing_count_(0),
+ pictures_assigned_(false, false),
device_poll_thread_("V4L2DevicePollThread"),
make_context_current_(make_context_current),
egl_display_(egl_display),
@@ -349,6 +350,8 @@ void V4L2VideoDecodeAccelerator::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);
@@ -383,11 +386,29 @@ void V4L2VideoDecodeAccelerator::AssignPictureBuffers(
picture_buffers_ref->picture_buffers.push_back(
PictureBufferArrayRef::PictureBufferRef(egl_image, buffers[i].id()));
}
- decoder_thread_.message_loop()->PostTask(
- FROM_HERE,
- base::Bind(&V4L2VideoDecodeAccelerator::AssignPictureBuffersTask,
- base::Unretained(this),
- base::Passed(&picture_buffers_ref)));
+
+ DCHECK(free_output_buffers_.empty());
+ DCHECK_EQ(picture_buffers_ref->picture_buffers.size(),
+ output_buffer_map_.size());
+ for (size_t i = 0; i < output_buffer_map_.size(); ++i) {
+ OutputRecord& output_record = 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 V4L2VideoDecodeAccelerator::ReusePictureBuffer(int32 picture_buffer_id) {
@@ -441,6 +462,7 @@ void V4L2VideoDecodeAccelerator::Destroy() {
if (decoder_thread_.IsRunning()) {
decoder_thread_.message_loop()->PostTask(FROM_HERE, base::Bind(
&V4L2VideoDecodeAccelerator::DestroyTask, base::Unretained(this)));
+ pictures_assigned_.Signal();
// DestroyTask() will cause the decoder_thread_ to flush all tasks.
decoder_thread_.Stop();
} else {
@@ -473,7 +495,7 @@ void V4L2VideoDecodeAccelerator::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
@@ -888,46 +910,6 @@ bool V4L2VideoDecodeAccelerator::FlushInputFrame() {
return (decoder_state_ != kError);
}
-void V4L2VideoDecodeAccelerator::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", "V4L2VDA::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(), output_buffer_map_.size());
- for (size_t i = 0; i < output_buffer_map_.size(); ++i) {
- OutputRecord& output_record = 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;
- free_output_buffers_.push(i);
- DVLOG(3) << "AssignPictureBuffersTask(): buffer[" << i
- << "]: picture_id=" << buffer_ref.picture_id;
- }
- pic_buffers->picture_buffers.clear();
-
- // We got buffers! Enqueue.
- Enqueue();
-
- if (decoder_state_ == kChangingResolution)
- ResumeAfterResolutionChange();
-}
-
void V4L2VideoDecodeAccelerator::ServiceDeviceTask(bool event_pending) {
DVLOG(3) << "ServiceDeviceTask()";
DCHECK_EQ(decoder_thread_.message_loop(), base::MessageLoop::current());
@@ -1171,7 +1153,7 @@ bool V4L2VideoDecodeAccelerator::EnqueueInputRecord() {
input_record.at_device = true;
input_buffer_queued_count_++;
DVLOG(3) << "EnqueueInputRecord(): enqueued input_id="
- << input_record.input_id;
+ << input_record.input_id << " size=" << input_record.bytes_used;
return true;
}
@@ -1237,8 +1219,13 @@ void V4L2VideoDecodeAccelerator::ReusePictureBufferTask(
break;
if (index >= 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;
}
@@ -1250,6 +1237,7 @@ void V4L2VideoDecodeAccelerator::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;
free_output_buffers_.push(index);
@@ -1401,7 +1389,14 @@ void V4L2VideoDecodeAccelerator::ResetDoneTask() {
// Jobs drained, we're finished resetting.
DCHECK_EQ(decoder_state_, kResetting);
- decoder_state_ = kAfterReset;
+ if (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(
@@ -1496,15 +1491,22 @@ bool V4L2VideoDecodeAccelerator::StopDevicePoll(bool keep_input_state) {
}
input_buffer_queued_count_ = 0;
}
+
while (!free_output_buffers_.empty())
free_output_buffers_.pop();
+
for (size_t i = 0; i < output_buffer_map_.size(); ++i) {
OutputRecord& output_record = 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.
+ 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);
free_output_buffers_.push(i);
- output_buffer_map_[i].at_device = false;
}
}
output_buffer_queued_count_ = 0;
@@ -1539,6 +1541,7 @@ void V4L2VideoDecodeAccelerator::StartResolutionChangeIfNeeded() {
void V4L2VideoDecodeAccelerator::FinishResolutionChange() {
DCHECK_EQ(decoder_thread_.message_loop(), base::MessageLoop::current());
+ DCHECK_EQ(decoder_state_, kChangingResolution);
DVLOG(3) << "FinishResolutionChange()";
if (decoder_state_ == kError) {
@@ -1561,8 +1564,7 @@ void V4L2VideoDecodeAccelerator::FinishResolutionChange() {
return;
}
- // From here we stay in kChangingResolution and wait for
- // AssignPictureBuffers() before we can resume.
+ ResumeAfterResolutionChange();
}
void V4L2VideoDecodeAccelerator::ResumeAfterResolutionChange() {
@@ -1646,7 +1648,7 @@ bool V4L2VideoDecodeAccelerator::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;
}
@@ -1787,6 +1789,22 @@ bool V4L2VideoDecodeAccelerator::CreateOutputBuffers() {
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/v4l2_video_decode_accelerator.h b/content/common/gpu/media/v4l2_video_decode_accelerator.h
index 15f8ee1..83a8a18 100644
--- a/content/common/gpu/media/v4l2_video_decode_accelerator.h
+++ b/content/common/gpu/media/v4l2_video_decode_accelerator.h
@@ -15,6 +15,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/v4l2_video_device.h"
@@ -50,16 +51,27 @@ namespace content {
// 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 V4L2VideoDecodeAccelerator
: public VideoDecodeAcceleratorImpl {
public:
@@ -121,7 +133,7 @@ class CONTENT_EXPORT V4L2VideoDecodeAccelerator
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).
@@ -184,11 +196,6 @@ class CONTENT_EXPORT V4L2VideoDecodeAccelerator
// 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 |event_pending| is true, one or more events
// on file descriptor are pending.
@@ -355,6 +362,9 @@ class CONTENT_EXPORT V4L2VideoDecodeAccelerator
//
// Hardware state and associated queues. Since decoder_thread_ services
// the hardware, decoder_thread_ owns these too.
+ // 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.
@@ -389,6 +399,10 @@ class CONTENT_EXPORT V4L2VideoDecodeAccelerator
// 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 16e3e74..a97d05f 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)
@@ -108,6 +109,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
@@ -128,7 +131,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) {
}
@@ -139,7 +142,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;
};
@@ -380,7 +383,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,
@@ -459,7 +462,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_;
@@ -486,7 +489,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,
@@ -578,10 +581,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(
@@ -674,6 +676,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;
}
@@ -845,6 +848,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;
@@ -865,6 +891,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;
@@ -879,13 +918,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,
@@ -1017,8 +1063,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);
@@ -1035,14 +1083,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)
@@ -1364,16 +1416,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.