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..31ad7a71365fb2c7219431ba8abb35ea0334117b 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,9 +112,19 @@ void VideoPipelineImpl::Initialize(
   if (frame_provider)
     SetCodedFrameProvider(frame_provider.Pass());
 
-  if (!video_device_->SetConfig(
-          DecoderConfigAdapter::ToCastVideoConfig(kPrimary, video_config)) ||
-      !av_pipeline_impl_->Initialize()) {
+  VideoConfig *video_configs = new VideoConfig[configs.size()];

[gunsch, 2015/06/04 17:03:43]

scoped_ptr has an array templatization that handles array delete. Prefer that
over raw pointers for this.

[erickung1, 2015/06/04 20:29:20]

I removed the code here using loop and assume at most there will be two for now

+  for (size_t i = 0; i < configs.size(); i++) {
+    LOG(INFO) << configs[i].AsHumanReadableString();
+    video_configs[i] = DecoderConfigAdapter::ToCastVideoConfig(
+        static_cast<StreamId>(i), configs[i]);
+    if (i > 0) {
+      video_configs[i-1].additional_config = &video_configs[i];

[gunsch, 2015/06/04 17:03:43]

I don't understand this. So you allow for N video configs, but this reads to me
like you're assuming it's actually exactly 2, and that you're going to pass one
VideoConfig object to the backend. Can you clarify?

If we're expecting no more than two everywhere, we might pass around
primary/secondary everywhere, or regularly DCHECK(configs.size() <= 2).

[erickung1, 2015/06/04 20:29:20]

The original thought is to use programming solution to chain the data, and don't
limit the code to support only 2. But I think I have plan to refactor the
interface better without chaining the data later on. put a TODO here

+    }
+  }
+  bool result = video_device_->SetConfig(video_configs[0]);

[gunsch, 2015/06/04 17:03:43]

Yeah, see, you do a complicated loop above, but then only pass down the first
VideoConfig object. Just handle two, e.g.

VideoConfig video_config = ToCastVideoConfig(0, configs[0])
VideoConfig secondary_config;
if (configs.size() == 2)
  secondary_config = ToCastVideoConfig(1, configs[1]);
  video_config.additional_config = &secondary_config;
}
video_device_->SetConfig(video_config);

[erickung1, 2015/06/04 20:29:20]

Done.

+  delete[] video_configs;
+
+  if (!result || !av_pipeline_impl_->Initialize()) {
     status_cb.Run(::media::PIPELINE_ERROR_INITIALIZATION_FAILED);
     return;
   }
@@ -127,6 +136,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();