Passed is_encrypted parameter to the VDA initialization.

media/video/video_decode_accelerator.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_VIDEO_VIDEO_DECODE_ACCELERATOR_H_
 #define MEDIA_VIDEO_VIDEO_DECODE_ACCELERATOR_H_
 
 #include <memory>
 #include <vector>
 
@@ skipping 35 matching lines @@
     // Encoded input is unreadable.
     UNREADABLE_INPUT,
     // A failure occurred at the browser layer or one of its dependencies.
     // Examples of such failures include GPU hardware failures, GPU driver
     // failures, GPU library failures, browser programming errors, and so on.
     PLATFORM_FAILURE,
     // Largest used enum. This should be adjusted when new errors are added.
     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.
   // This interface is extended by the various layers that relay messages back
   // to the plugin, through the PPP_VideoDecoder_Dev interface the plugin
   // implements.
   class MEDIA_EXPORT Client {
    public:
     // SetCdm completion callback to indicate whether the CDM is successfully
     // attached to the decoder. The default implementation is a no-op since most
@@ skipping 38 matching lines @@
   // decoder construction.  This call is synchronous and returns true iff
   // initialization is successful.
   //
   // For encrpyted video, the decoder needs a CDM to be able to decode encrypted
   // buffers. SetCdm() should be called after Initialize() to set such a CDM.
   // Client::NotifyCdmAttached() will then be called to indicate whether the CDM
   // is successfully attached to the decoder. Only when a CDM is successfully
   // 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
   // is successfully attached to the decoder. The default implementation is a
   // no-op since most VDAs don't support encrypted video.
   virtual void SetCdm(int cdm_id);
 
   // Decodes given bitstream buffer that contains at most one frame.  Once
   // decoder is done with processing |bitstream_buffer| it will call
   // NotifyEndOfBitstreamBuffer() with the bitstream buffer id.
@@ skipping 68 matching lines @@
 // Specialize std::default_delete so that scoped_ptr<VideoDecodeAccelerator>
 // uses "Destroy()" instead of trying to use the destructor.
 template <>
 struct MEDIA_EXPORT default_delete<media::VideoDecodeAccelerator> {
   void operator()(media::VideoDecodeAccelerator* vda) const;
 };
 
 }  // namespace std
 
 #endif  // MEDIA_VIDEO_VIDEO_DECODE_ACCELERATOR_H_