Add color metadata info to VideoDecoderConfig.

media/base/video_decoder_config.h

 // 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.
 
 #ifndef MEDIA_BASE_VIDEO_DECODER_CONFIG_H_
 #define MEDIA_BASE_VIDEO_DECODER_CONFIG_H_
 
 #include <stdint.h>
 
 #include <string>
 #include <vector>
 
 #include "base/macros.h"
+#include "base/optional.h"
 #include "media/base/encryption_scheme.h"
+#include "media/base/hdr_metadata.h"
 #include "media/base/media_export.h"
 #include "media/base/video_codecs.h"
 #include "media/base/video_types.h"
+#include "ui/gfx/color_space.h"
 #include "ui/gfx/geometry/rect.h"
 #include "ui/gfx/geometry/size.h"
 
 namespace media {
 
 MEDIA_EXPORT VideoCodec
 VideoCodecProfileToVideoCodec(VideoCodecProfile profile);
 
 class MEDIA_EXPORT VideoDecoderConfig {
  public:
@@ skipping 60 matching lines @@
   gfx::Size coded_size() const { return coded_size_; }
 
   // Region of |coded_size_| that is visible.
   gfx::Rect visible_rect() const { return visible_rect_; }
 
   // Final visible width and height of a video frame with aspect ratio taken
   // into account.
   gfx::Size natural_size() const { return natural_size_; }
 
   // Optional byte data required to initialize video decoders, such as H.264
-  // AAVC data.
+  // AVCC data.
   const std::vector<uint8_t>& extra_data() const { return extra_data_; }
 
   // Whether the video stream is potentially encrypted.
   // Note that in a potentially encrypted video stream, individual buffers
   // can be encrypted or not encrypted.
   bool is_encrypted() const { return encryption_scheme_.is_encrypted(); }
 
   // Encryption scheme used for encrypted buffers.
   const EncryptionScheme& encryption_scheme() const {
     return encryption_scheme_;
   }
 
+  void set_color_space_info(const gfx::ColorSpace& color_space_info);
+  base::Optional<gfx::ColorSpace> color_space_info() const;
+
+  void set_hdr_metadata(const HDRMetadata& hdr_metadata);
+  base::Optional<HDRMetadata> hdr_metadata() const;
+
  private:
   VideoCodec codec_;
   VideoCodecProfile profile_;
 
   VideoPixelFormat format_;
+
+  // TODO(servolk): Deprecated, use color_space_info_ instead.
   ColorSpace color_space_;
 
   gfx::Size coded_size_;
   gfx::Rect visible_rect_;
   gfx::Size natural_size_;
 
   std::vector<uint8_t> extra_data_;
 
   EncryptionScheme encryption_scheme_;
 
+  base::Optional<gfx::ColorSpace> color_space_info_;

[hubbe, 2016/09/15 17:30:10]

I don't think this needs to be optional.
gfx::ColorSpace is meant to be small and lightweight and defaults to UNDEFINED
values. In most places in the code we just use the class directly. (including
media::VideoFrame)

[servolk, 2016/09/15 20:29:03]

I've made it optional mostly because I wasn't sure that we'll be able to
initialize this properly in all cases and I assumed that not having color space
info in those cases would be clearer than having UNDEFINED. But I don't have a
strong preference here, so converted to non-optional for now. Perhaps we should
even make it the constructor/Initialize parameter? I didn't do that yet to avoid
confusion, since the old color space enum is already there, we need to get rid
of it first.
Also I guess we could use Rec. 601 for SD video by default and Rec. 709 for HD
video, like we do e.g. at
https://cs.chromium.org/chromium/src/media/ffmpeg/ffmpeg_common.cc?rcl=0&l=516

[hubbe, 2016/09/15 20:42:53]

I'm not actually sure this is what we want to do going forward.
Changing defaults based on resolution can lead to some really ugly things when
the resolution changes dynamically.  My current thinking is that defaulting to
709 in all cases is the right thing to do, and if you want something else, make
sure to tag your content correctly.

[servolk, 2016/09/15 23:12:41]

Ok, I've looked at all code paths that invoke VideoDecoderConfig::Initialize and
found only 4 places (excluding test code) that use actual input values for the
color_space, that are not copied from another VideoDecoderConfig struct:
1. VideoDecoderShim::Initialize
(https://cs.chromium.org/chromium/src/content/renderer/pepper/video_decoder_sh...)
uses COLOR_SPACE_UNSPECIFIED. IIUC this is mostly for Flash video, so Rec. 709
should be ok here.
2. AVStreamToVideoDecoderConfig for src=/FFmpegDemuxer playback uses 601 for SD
and 709 for HD
(https://cs.chromium.org/chromium/src/media/ffmpeg/ffmpeg_common.cc?rcl=147393...).
3,4. WebM parser and MP4 parser for MSE, both use 709 unconditionally, which is
probably the right choice for MSE, even for lower resolutions:
https://cs.chromium.org/chromium/src/media/formats/webm/webm_video_client.cc?...
https://cs.chromium.org/chromium/src/media/formats/mp4/mp4_stream_parser.cc?r...

So yes, I agree, we probably don't need to select based on resolution here,
since the only place where it makes sense (FFmpegDemuxer) already does that.
I'll remove that code.

+  base::Optional<HDRMetadata> hdr_metadata_;
+
   // Not using DISALLOW_COPY_AND_ASSIGN here intentionally to allow the compiler
   // generated copy constructor and assignment operator. Since the extra data is
   // typically small, the performance impact is minimal.
 };
 
 }  // namespace media
 
 #endif  // MEDIA_BASE_VIDEO_DECODER_CONFIG_H_