Chromecast: Use std::vector to pass video config in CMA

chromecast/media/cma/pipeline/video_pipeline_impl.cc

Index: chromecast/media/cma/pipeline/video_pipeline_impl.cc
diff --git a/chromecast/media/cma/pipeline/video_pipeline_impl.cc b/chromecast/media/cma/pipeline/video_pipeline_impl.cc
index c1328af39f1b7155cdaf7b9341ed5e7711477db0..4dbac5bc416ea17251a8bcb656063c983d097c63 100644
--- a/chromecast/media/cma/pipeline/video_pipeline_impl.cc
+++ b/chromecast/media/cma/pipeline/video_pipeline_impl.cc
@@ -101,11 +101,10 @@ void VideoPipelineImpl::SetClient(const VideoPipelineClient& client) {
 }
 
 void VideoPipelineImpl::Initialize(
-    const ::media::VideoDecoderConfig& video_config,
+    const std::vector<::media::VideoDecoderConfig>& configs,
     scoped_ptr<CodedFrameProvider> frame_provider,
     const ::media::PipelineStatusCB& status_cb) {
-  CMALOG(kLogControl) << "VideoPipelineImpl::Initialize "
-                      << video_config.AsHumanReadableString();
+  CMALOG(kLogControl) << __FUNCTION__ << " config (" << configs.size() << ")";
   VideoPipelineDevice::VideoClient client;
   client.natural_size_changed_cb =
       base::Bind(&VideoPipelineImpl::OnNaturalSizeChanged, weak_this_);
@@ -113,8 +112,19 @@ void VideoPipelineImpl::Initialize(
   if (frame_provider)
     SetCodedFrameProvider(frame_provider.Pass());
 
-  if (!video_device_->SetConfig(
-          DecoderConfigAdapter::ToCastVideoConfig(kPrimary, video_config)) ||
+  // TODO(erickung): Consider better interface to pass multiple video configs

[gunsch, 2015/06/05 00:17:13]

Is there _ever_ a case when we will have more than 2? It seems to me like the
two-stream case is a pretty big exception as it is with a potentially fixed end
date and that this isn't intended to become N configs.

Or is this TODO about it being awkward to pass a config along as a pointer in
VideoConfig? Because if so, I agree, but maybe put the TODO in the VideoConfig
definition?

[erickung1, 2015/06/05 01:04:18]

This TODO is for later. I'll remove it from this CL and will add it to the
header file later on.

+  // to CMA without chaining all of them.
+  DCHECK(configs.size() <= 2);
+  VideoConfig video_config =
+      DecoderConfigAdapter::ToCastVideoConfig(kPrimary, configs[0]);
+  VideoConfig secondary_config;
+  if (configs.size() == 2) {
+    secondary_config = DecoderConfigAdapter::ToCastVideoConfig(kSecondary,

[gunsch, 2015/06/05 00:17:13]

(Thanks for removing the "for" loop, this is a lot more straightforward to
read).

Are we expecting valid configs as inputs? Should this DCHECK that
secondary_config is valid, or possibly fail initialization if they're not?

Or is that the job of VideoPipelineDevice::SetConfig to do? (return false if the
configs are invalid?)

It seems like we should be validating those in our common code before passing to
vendor implementation.

[erickung1, 2015/06/05 01:04:18]

From the original code, VideoPipelineImpl::Initialize() should provide valid
VideoDecoderConfig information. Therefore I don't think we need to add
additional check here.

After converting it to VideoConfig, we don't need to check because
- no IsValidConfig() util function because we don't want to provide such util
function in public header file
- event though we think it's valid config, the platform still can reject the
request and return false.

It's a sync call and straightforward, I think we can leave the decision to OEM
vendor.

[gunsch, 2015/06/05 18:08:16]

If that's the assumption, then how about DCHECK(config.IsValidConfig()) for both
configs then? Then it's clear when reading the code that the configs are always
valid at this point.

[erickung1, 2015/06/05 20:44:20]

Done. I add the DCHECK on audio_pipeline_impl as well

+                                                               configs[1]);
+    video_config.additional_config = &secondary_config;
+  }
+
+  if (!video_device_->SetConfig(video_config) ||
       !av_pipeline_impl_->Initialize()) {
     status_cb.Run(::media::PIPELINE_ERROR_INITIALIZATION_FAILED);
     return;
@@ -127,6 +137,7 @@ void VideoPipelineImpl::OnUpdateConfig(
     StreamId id,
     const ::media::AudioDecoderConfig& audio_config,
     const ::media::VideoDecoderConfig& video_config) {
+  DCHECK(!audio_config.IsValidConfig());
   if (video_config.IsValidConfig()) {
     CMALOG(kLogControl) << "VideoPipelineImpl::OnUpdateConfig id:" << id << " "
                         << video_config.AsHumanReadableString();