Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(612)

Unified Diff: content/common/gpu/media/vt_video_decode_accelerator.cc

Issue 1481633002: Never reuse a VideoToolbox decompression session. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@2564
Patch Set: Created 5 years, 1 month ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « content/common/gpu/media/vt_video_decode_accelerator.h ('k') | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: content/common/gpu/media/vt_video_decode_accelerator.cc
diff --git a/content/common/gpu/media/vt_video_decode_accelerator.cc b/content/common/gpu/media/vt_video_decode_accelerator.cc
index 2947fe1dc303b2c2fce5a53e814cf27d27f14ade..2ff1a5318d983e052b78e3da34d4f7d2a2ec8e20 100644
--- a/content/common/gpu/media/vt_video_decode_accelerator.cc
+++ b/content/common/gpu/media/vt_video_decode_accelerator.cc
@@ -65,12 +65,6 @@ static const int kNumPictureBuffers = media::limits::kMaxVideoFrames + 1;
// reorder queue.)
static const int kMaxReorderQueueSize = 16;
-// When set to false, always create a new decoder instead of reusing the
-// existing configuration when the configuration changes. This works around a
-// bug in VideoToolbox that results in corruption before Mac OS X 10.10.3. The
-// value is set in InitializeVideoToolbox().
-static bool g_enable_compatible_configuration_reuse = true;
-
// Build an |image_config| dictionary for VideoToolbox initialization.
static base::ScopedCFTypeRef<CFMutableDictionaryRef>
BuildImageConfig(CMVideoDimensions coded_dimensions) {
@@ -188,7 +182,6 @@ static bool InitializeVideoToolboxInternal() {
// CoreVideo is also required, but the loader stops after the first path is
// loaded. Instead we rely on the transitive dependency from VideoToolbox to
// CoreVideo.
- // TODO(sandersd): Fallback to PrivateFrameworks to support OS X < 10.8.
StubPathMap paths;
paths[kModuleVt].push_back(FILE_PATH_LITERAL(
"/System/Library/Frameworks/VideoToolbox.framework/VideoToolbox"));
@@ -225,12 +218,6 @@ static bool InitializeVideoToolboxInternal() {
return false;
}
- // Set |g_enable_compatible_configuration_reuse| to false on
- // Mac OS X < 10.10.3.
- base::Version os_x_version(base::SysInfo::OperatingSystemVersion());
- if (os_x_version.IsOlderThan("10.10.3"))
- g_enable_compatible_configuration_reuse = false;
-
return true;
}
@@ -270,7 +257,10 @@ VTVideoDecodeAccelerator::Task::~Task() {
}
VTVideoDecodeAccelerator::Frame::Frame(int32_t bitstream_id)
- : bitstream_id(bitstream_id), pic_order_cnt(0), reorder_window(0) {
+ : bitstream_id(bitstream_id),
+ pic_order_cnt(0),
+ is_idr(false),
+ reorder_window(0) {
}
VTVideoDecodeAccelerator::Frame::~Frame() {
@@ -386,7 +376,6 @@ bool VTVideoDecodeAccelerator::ConfigureDecoder() {
nalu_data_sizes.push_back(last_pps_.size());
// Construct a new format description from the parameter sets.
- // TODO(sandersd): Replace this with custom code to support OS X < 10.9.
format_.reset();
OSStatus status = CMVideoFormatDescriptionCreateFromH264ParameterSets(
kCFAllocatorDefault,
@@ -402,16 +391,13 @@ bool VTVideoDecodeAccelerator::ConfigureDecoder() {
}
// Store the new configuration data.
+ // TODO(sandersd): Despite the documentation, this seems to return the visible
+ // size. However, the output always appears to be top-left aligned, so it
+ // makes no difference. Re-verify this and update the variable name.
CMVideoDimensions coded_dimensions =
CMVideoFormatDescriptionGetDimensions(format_);
coded_size_.SetSize(coded_dimensions.width, coded_dimensions.height);
- // If the session is compatible, there's nothing else to do.
- if (g_enable_compatible_configuration_reuse && session_ &&
- VTDecompressionSessionCanAcceptFormatDescription(session_, format_)) {
- return true;
- }
-
// Prepare VideoToolbox configuration dictionaries.
base::ScopedCFTypeRef<CFMutableDictionaryRef> decoder_config(
CFDictionaryCreateMutable(
@@ -558,12 +544,8 @@ void VTVideoDecodeAccelerator::DecodeTask(
case media::H264NALU::kSliceDataB:
case media::H264NALU::kSliceDataC:
case media::H264NALU::kNonIDRSlice:
- // TODO(sandersd): Check that there has been an IDR slice since the
- // last reset.
case media::H264NALU::kIDRSlice:
// Compute the |pic_order_cnt| for the picture from the first slice.
- // TODO(sandersd): Make sure that any further slices are part of the
- // same picture or a redundant coded picture.
if (!has_slice) {
media::H264SliceHeader slice_hdr;
result = parser_.ParseSliceHeader(nalu, &slice_hdr);
@@ -603,6 +585,9 @@ void VTVideoDecodeAccelerator::DecodeTask(
return;
}
+ if (nalu.nal_unit_type == media::H264NALU::kIDRSlice)
+ frame->is_idr = true;
+
if (sps->vui_parameters_present_flag &&
sps->bitstream_restriction_flag) {
frame->reorder_window = std::min(sps->max_num_reorder_frames,
@@ -639,7 +624,10 @@ void VTVideoDecodeAccelerator::DecodeTask(
NotifyError(INVALID_ARGUMENT, SFT_INVALID_STREAM);
return;
}
- if (!ConfigureDecoder())
+
+ // If it's not an IDR frame, we can't reconfigure the decoder anyway. We
+ // assume that any config change not on an IDR must be compatible.
+ if (frame->is_idr && !ConfigureDecoder())
return;
}
@@ -896,9 +884,8 @@ bool VTVideoDecodeAccelerator::ProcessTaskQueue() {
const Task& task = task_queue_.front();
switch (task.type) {
case TASK_FRAME:
- // TODO(sandersd): Signal IDR explicitly (not using pic_order_cnt == 0).
if (reorder_queue_.size() < kMaxReorderQueueSize &&
- (task.frame->pic_order_cnt != 0 || reorder_queue_.empty())) {
+ (!task.frame->is_idr || reorder_queue_.empty())) {
assigned_bitstream_ids_.erase(task.frame->bitstream_id);
client_->NotifyEndOfBitstreamBuffer(task.frame->bitstream_id);
reorder_queue_.push(task.frame);
@@ -952,7 +939,7 @@ bool VTVideoDecodeAccelerator::ProcessReorderQueue() {
// the next frame.
bool flushing = !task_queue_.empty() &&
(task_queue_.front().type != TASK_FRAME ||
- task_queue_.front().frame->pic_order_cnt == 0);
+ task_queue_.front().frame->is_idr);
size_t reorder_window = std::max(0, reorder_queue_.top()->reorder_window);
if (flushing || reorder_queue_.size() > reorder_window) {
@@ -1121,8 +1108,8 @@ void VTVideoDecodeAccelerator::Destroy() {
// For a graceful shutdown, return assigned buffers and flush before
// destructing |this|.
- // TODO(sandersd): Make sure the decoder won't try to read the buffers again
- // before discarding them.
+ // TODO(sandersd): Prevent the decoder from reading buffers before discarding
+ // them.
for (int32_t bitstream_id : assigned_bitstream_ids_)
client_->NotifyEndOfBitstreamBuffer(bitstream_id);
assigned_bitstream_ids_.clear();
« no previous file with comments | « content/common/gpu/media/vt_video_decode_accelerator.h ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698