Attach color space information to hardware decoded NV12 video frames.

media/gpu/dxva_video_decode_accelerator_win.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "media/gpu/dxva_video_decode_accelerator_win.h"
 
 #include <memory>
 
 #if !defined(OS_WIN)
 #error This file should only be built on Windows.
@@ skipping 440 matching lines @@
         config_changed_ = true;
       } else {
         pending_config_changed_ = true;
       }
     }
     last_pps_.swap(pps);
   }
   return true;
 }
 
+gfx::ColorSpace H264ConfigChangeDetector::current_color_space() const {
+  // TODO(hubbe): Is using last_sps_id_ correct here?

[sandersd (OOO until July 31), 2016/09/16 21:54:13]

There could be a gap between the ID changing and the config change occurring,
although in practice I've never seen that (which isn't too surprising since it's
invalid for the MP4 container).

However, there is a more serious possible problem, which I don't know DXVA well
enough off-hand to be sure of: a config change could be detected before the last
frame is output, and you would tag trailing frames with the wrong colorspace.

To do this correctly the tag should be assigned to frames before decode and
tracked all the way through.

[hubbe, 2016/09/21 22:04:26]

Updated the code to handle this (more) correctly.

+  const H264SPS* sps = parser_->GetSPS(last_sps_id_);
+  if (sps)
+    return sps->GetColorSpace();
+  return gfx::ColorSpace();
+}
 
 DXVAVideoDecodeAccelerator::PendingSampleInfo::PendingSampleInfo(
     int32_t buffer_id,
     IMFSample* sample)
     : input_buffer_id(buffer_id), picture_buffer_id(-1) {
   output_sample.Attach(sample);
 }
 
 DXVAVideoDecodeAccelerator::PendingSampleInfo::PendingSampleInfo(
     const PendingSampleInfo& other) = default;
@@ skipping 1415 matching lines @@
         provide_nv12_textures ? 2 : 1, gfx::Size(width, height),
         provide_nv12_textures ? GL_TEXTURE_EXTERNAL_OES : GL_TEXTURE_2D);
   }
 }
 
 void DXVAVideoDecodeAccelerator::NotifyPictureReady(int picture_buffer_id,
                                                     int input_buffer_id) {
   DCHECK(main_thread_task_runner_->BelongsToCurrentThread());
   // This task could execute after the decoder has been torn down.
   if (GetState() != kUninitialized && client_) {
+    gfx::ColorSpace color_space;
+    if (share_nv12_textures_ || copy_nv12_textures_) {
+      // Color space information is only valid for NV12 textures.
+      // If the texture has been converted to RGB, we don't actually know what
+      // Color space the output is in.
+      color_space = config_change_detector_->current_color_space();
+    }
     // TODO(henryhsu): Use correct visible size instead of (0, 0). We can't use
     // coded size here so use (0, 0) intentionally to have the client choose.
-    Picture picture(picture_buffer_id, input_buffer_id, gfx::Rect(0, 0), false);
+    Picture picture(picture_buffer_id, input_buffer_id, gfx::Rect(0, 0),
+                    color_space, false);
     client_->PictureReady(picture);
   }
 }
 
 void DXVAVideoDecodeAccelerator::NotifyInputBuffersDropped() {
   DCHECK(main_thread_task_runner_->BelongsToCurrentThread());
   if (!client_)
     return;
 
   for (PendingInputs::iterator it = pending_input_buffers_.begin();
@@ skipping 776 matching lines @@
   SetState(kConfigChange);
   Invalidate();
   Initialize(config_, client_);
   decoder_thread_task_runner_->PostTask(
       FROM_HERE,
       base::Bind(&DXVAVideoDecodeAccelerator::DecodePendingInputBuffers,
                  base::Unretained(this)));
 }
 
 }  // namespace media