Passed is_encrypted parameter to the VDA initialization.

media/video/video_decode_accelerator.h

Index: media/video/video_decode_accelerator.h
diff --git a/media/video/video_decode_accelerator.h b/media/video/video_decode_accelerator.h
index e2bcdf64855fdf3455fc0a3b73ae2b59c2e928e6..5cd16dd99fadd6d6ce534bf4fc20791faf822fc1 100644
--- a/media/video/video_decode_accelerator.h
+++ b/media/video/video_decode_accelerator.h
@@ -53,6 +53,19 @@ class MEDIA_EXPORT VideoDecodeAccelerator {
     LARGEST_ERROR_ENUM,
   };
 
+  // Parameters required for the Initialize().
+  struct MEDIA_EXPORT StreamParameters {

[xhwang, 2015/12/02 06:48:18]

IMHO "Parameters" doesn't add any value to the name.  Also, I don't see why we
want to distinguish this from VideoDecoderConfig. If not for the sake of
additional IPC work, I'd like to send VideoDecoderConfig directly to
VDA::Initialize() to avoid reinventing a new type for the initialization config.
VDA::Config seems a perfect tradeoff to me...

[liberato (no reviews please), 2015/12/02 16:18:50]

i don't think that VDA::Config will always be a subset of VDC.  we've had
several discussions lately (e.g., full screen surface ID) that suggest it won't
be.  since updating all the VDA implementations is no fun, i believe that we
definitely need some VDA-specific config type, rather than sending the VDC
verbatim. +1 xhwang's VDA::Config name.

i also really don't like 'shadow types', but the IPC cost prevented me from
objecting yesterday.  this morning, the IPC cost doesn't seem so bad as it did
yesterday; VDC is lightweight and initialization is infrequent.

how about this, which is what i would favor if IPC weren't involved:

class VideoDecodeAccelerator::Config {
  VideoDecoderConfig decoder_config_;
  // other stuff as it shows up
}

[Tima Vaisburd, 2015/12/02 18:02:15]

(1) Not every method calling Initialize() has VDC easily available, what to do
in rtc_video_decoder.cc ?
(2) Do you prefer passing the whole VDC over IPC even if we use only 2 fields,
namely, profile and is_encrypted? Or shall there be another structure dedicated
just for IPC? 

[liberato (no reviews please), 2015/12/02 18:29:47]

(1) good question -- i don't know.  "whole VDC" doesn't bother me, though, in
this case.  but if not every use of VDA has one, well, that's almost the same
problem that makes me not like "VDCLite" - manually thunking one config
structure into another can go wrong easily.

(2) ignoring (1) for a moment, i'd favor all of VDC.  otherwise, we'll end up
adding "just one more field" to the interim structure, and/or forgetting to
initialize it elsewhere.

on balance, creating VDA::Config with just |profile_| and |is_encrypted_| seems
like the best of a bad set of options.  might as well provide only the fields
that have meaningful data, and centralize the thunk from VDC in the VDA::C
constructor.

[xhwang, 2015/12/02 18:58:34]

I am okay with just renaming the current StreamParameters to Config, which
contains only |profile_| and |is_encrypted_|.

[Tima Vaisburd, 2015/12/02 22:56:58]

Done.

[Tima Vaisburd, 2015/12/02 22:56:58]

This is what I ended up doing in this PS, although maybe we can make fake
VideoDecoderConfig in this case, similar to this:
https://code.google.com/p/chromium/codesearch#chromium/src/content/renderer/p...

[sandersd (OOO until July 31), 2015/12/02 23:33:27]

I wanted to add my late $0.02 here because this interface is slowly mutating
into VDAv2. In VDAv2, I plan to use VideoDecoderConfig. Note that the data
passed over IPC can still be only a subset.

It will be overall easier for me if that's what happened in this CL ;-)

+    StreamParameters();
+    StreamParameters(VideoCodecProfile profile);
+    StreamParameters(const VideoDecoderConfig& config);
+
+    // |profile| combines the information about the codec and its profile.
+    VideoCodecProfile profile;
+
+    // The flag indicating whether the stream is encrypted.
+    bool is_encrypted;

[xhwang, 2015/12/02 06:48:18]

Having multiple constructors is actually the perfect case to use non-static
class member initializers. That way, you  have consistent default values and
only overwrite members with non-default values in the initialization list.

Just in case it's not clear, I meant having default values here for both
|profile| and |is_encrypted|.

Imagine what if you have a typo in the following code. It's hard to find it out.

VideoDecodeAccelerator::StreamParameters::StreamParameters()
  : profile(VIDEO_CODEC_PROFILE_UNKNOWN), is_encrypted(false) {}

VideoDecodeAccelerator::StreamParameters::StreamParameters(VideoCodecProfile p)
   : profile(p), is_encrypted(true) {}

[liberato (no reviews please), 2015/12/02 16:18:50]

+1

[Tima Vaisburd, 2015/12/02 18:02:15]

Acknowledged.

[Tima Vaisburd, 2015/12/02 22:56:58]

Done.

+  };
+
   // Interface for collaborating with picture interface to provide memory for
   // output picture and blitting them. These callbacks will not be made unless
   // Initialize() has returned successfully.
@@ -111,10 +124,10 @@ class MEDIA_EXPORT VideoDecodeAccelerator {
   // attached can we start to decode.
   //
   // Parameters:
-  //  |profile| is the video stream's format profile.
+  //  |params| are the initialization parameters.
   //  |client| is the client of this video decoder. Does not take ownership of
   //  |client| which must be valid until Destroy() is called.
-  virtual bool Initialize(VideoCodecProfile profile, Client* client) = 0;
+  virtual bool Initialize(const StreamParameters& params, Client* client) = 0;
 
   // Sets a CDM to be used by the decoder to decode encrypted buffers.
   // Client::NotifyCdmAttached() will then be called to indicate whether the CDM