Implement support for multiple tracks in FFmpegDemuxer

media/filters/ffmpeg_demuxer.cc

 // 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.
 
 #include "media/filters/ffmpeg_demuxer.h"
 
 #include <algorithm>
 #include <memory>
 #include <utility>
 
@@ skipping 877 matching lines @@
   // in the process of being fulfilled by the DataSource.
   data_source_->Stop();
   url_protocol_->Abort();
 
   // This will block until all tasks complete. Note that after this returns it's
   // possible for reply tasks (e.g., OnReadFrameDone()) to be queued on this
   // thread. Each of the reply task methods must check whether we've stopped the
   // thread and drop their results on the floor.
   blocking_thread_.Stop();
 
-  StreamVector::iterator iter;
-  for (iter = streams_.begin(); iter != streams_.end(); ++iter) {
-    if (*iter)
-      (*iter)->Stop();
+  for (const auto& stream : streams_) {
+    if (stream)
+      stream->Stop();
   }
 
   data_source_ = NULL;
 
   // Invalidate WeakPtrs on |task_runner_|, destruction may happen on another
   // thread.
   weak_factory_.InvalidateWeakPtrs();
 }
 
 void FFmpegDemuxer::StartWaitingForSeek(base::TimeDelta seek_time) {}
@@ skipping 61 matching lines @@
   return timeline_offset_;
 }
 
 DemuxerStream* FFmpegDemuxer::GetStream(DemuxerStream::Type type) {
   DCHECK(task_runner_->BelongsToCurrentThread());
   return GetFFmpegStream(type);
 }
 
 FFmpegDemuxerStream* FFmpegDemuxer::GetFFmpegStream(
     DemuxerStream::Type type) const {
-  StreamVector::const_iterator iter;
-  for (iter = streams_.begin(); iter != streams_.end(); ++iter) {
-    if (*iter && (*iter)->type() == type) {
-      return *iter;
+  for (const auto& it : streams_) {

[DaleCurtis, 2016/08/29 18:15:20]

Not really an iterator any more.

[servolk, 2016/08/29 18:38:20]

What would be a better name? 'stream'? 's'?

[DaleCurtis, 2016/08/29 18:49:29]

Yeah, you renamed all the rest to streams :)

[servolk, 2016/08/29 20:25:28]

Done.

+    if (it && it->type() == type && it->enabled()) {
+      return it.get();
     }
   }
   return NULL;
 }
 
 base::TimeDelta FFmpegDemuxer::GetStartTime() const {
   return std::max(start_time_, base::TimeDelta());
 }
 
 void FFmpegDemuxer::AddTextStreams() {
   DCHECK(task_runner_->BelongsToCurrentThread());
 
-  for (StreamVector::size_type idx = 0; idx < streams_.size(); ++idx) {
-    FFmpegDemuxerStream* stream = streams_[idx];
-    if (stream == NULL || stream->type() != DemuxerStream::TEXT)
+  for (const auto& stream : streams_) {
+    if (!stream || stream->type() != DemuxerStream::TEXT)
       continue;
 
     TextKind kind = stream->GetTextKind();
     std::string title = stream->GetMetadata("title");
     std::string language = stream->GetMetadata("language");
 
     // TODO: Implement "id" metadata in FFMPEG.
     // See: http://crbug.com/323183
-    host_->AddTextStream(stream, TextTrackConfig(kind, title, language,
-        std::string()));
+    host_->AddTextStream(stream.get(),
+                         TextTrackConfig(kind, title, language, std::string()));
   }
 }
 
 int64_t FFmpegDemuxer::GetMemoryUsage() const {
   int64_t allocation_size = 0;
-  for (auto* stream : streams_) {
+  for (const auto& stream : streams_) {
     if (stream)
       allocation_size += stream->MemoryUsage();
   }
   return allocation_size;
 }
 
 void FFmpegDemuxer::OnEncryptedMediaInitData(
     EmeInitDataType init_data_type,
     const std::string& encryption_key_id) {
   std::vector<uint8_t> key_id_local(encryption_key_id.begin(),
@@ skipping 129 matching lines @@
         const base::TimeDelta packet_pts =
             ConvertFromTimeBase(stream->time_base, packet_buffer->pkt.pts);
         if (packet_pts < start_time_estimates[stream->index])
           start_time_estimates[stream->index] = packet_pts;
       }
       packet_buffer = packet_buffer->next;
     }
   }
 
   std::unique_ptr<MediaTracks> media_tracks(new MediaTracks());
-  AVStream* audio_stream = NULL;
-  AudioDecoderConfig audio_config;
-  AVStream* video_stream = NULL;
-  VideoDecoderConfig video_config;
 
   DCHECK(track_id_to_demux_stream_map_.empty());
 
   // If available, |start_time_| will be set to the lowest stream start time.
   start_time_ = kInfiniteDuration;
 
   base::TimeDelta max_duration;
   int detected_audio_track_count = 0;
   int detected_video_track_count = 0;
   int detected_text_track_count = 0;
   for (size_t i = 0; i < format_context->nb_streams; ++i) {
     AVStream* stream = format_context->streams[i];
     const AVCodecContext* codec_context = stream->codec;
     const AVMediaType codec_type = codec_context->codec_type;
 
     if (codec_type == AVMEDIA_TYPE_AUDIO) {
       // Log the codec detected, whether it is supported or not, and whether or
       // not we have already detected a supported codec in another stream.
       UMA_HISTOGRAM_SPARSE_SLOWLY("Media.DetectedAudioCodecHash",
                                   HashCodecName(GetCodecName(codec_context)));
       detected_audio_track_count++;
-
-      if (audio_stream) {
-        MEDIA_LOG(INFO, media_log_) << GetDisplayName()
-                                    << ": skipping extra audio track";
-        continue;
-      }
     } else if (codec_type == AVMEDIA_TYPE_VIDEO) {
       // Log the codec detected, whether it is supported or not, and whether or
       // not we have already detected a supported codec in another stream.
       UMA_HISTOGRAM_SPARSE_SLOWLY("Media.DetectedVideoCodecHash",
                                   HashCodecName(GetCodecName(codec_context)));
       detected_video_track_count++;
 
-      if (video_stream) {
-        MEDIA_LOG(INFO, media_log_) << GetDisplayName()
-                                    << ": skipping extra video track";
-        continue;
-      }
-
 #if BUILDFLAG(ENABLE_HEVC_DEMUXING)
       if (stream->codec->codec_id == AV_CODEC_ID_HEVC) {
         // If ffmpeg is built without HEVC parser/decoder support, it will be
         // able to demux HEVC based solely on container-provided information,
         // but unable to get some of the parameters without parsing the stream
         // (e.g. coded size needs to be read from SPS, pixel format is typically
         // deduced from decoder config in hvcC box). These are not really needed
         // when using external decoder (e.g. hardware decoder), so override them
         // here, to make sure this translates into a valid VideoDecoderConfig.
         if (stream->codec->coded_width == 0 &&
@@ skipping 16 matching lines @@
     } else {
       continue;
     }
 
     // Attempt to create a FFmpegDemuxerStream from the AVStream. This will
     // return nullptr if the AVStream is invalid. Validity checks will verify
     // things like: codec, channel layout, sample/pixel format, etc...
     std::unique_ptr<FFmpegDemuxerStream> demuxer_stream =
         FFmpegDemuxerStream::Create(this, stream, media_log_);
     if (demuxer_stream.get()) {
-      streams_[i] = demuxer_stream.release();
+      streams_[i] = std::move(demuxer_stream);
     } else {
       if (codec_type == AVMEDIA_TYPE_AUDIO) {
         MEDIA_LOG(INFO, media_log_)
             << GetDisplayName()
             << ": skipping invalid or unsupported audio track";
       } else if (codec_type == AVMEDIA_TYPE_VIDEO) {
         MEDIA_LOG(INFO, media_log_)
             << GetDisplayName()
             << ": skipping invalid or unsupported video track";
       }
@@ skipping 13 matching lines @@
       // Need to fix that and use it as track id. crbug.com/323183
       track_id =
           static_cast<StreamParser::TrackId>(media_tracks->tracks().size() + 1);
       track_label = streams_[i]->GetMetadata("title");
     }
 
     // Note when we find our audio/video stream (we only want one of each) and
     // record src= playback UMA stats for the stream's decoder config.
     MediaTrack* media_track = nullptr;
     if (codec_type == AVMEDIA_TYPE_AUDIO) {
-      CHECK(!audio_stream);
-      audio_stream = stream;
-      audio_config = streams_[i]->audio_decoder_config();
+      AudioDecoderConfig audio_config = streams_[i]->audio_decoder_config();
       RecordAudioCodecStats(audio_config);
 
       media_track = media_tracks->AddAudioTrack(audio_config, track_id, "main",
                                                 track_label, track_language);
       media_track->set_id(base::UintToString(track_id));
       DCHECK(track_id_to_demux_stream_map_.find(media_track->id()) ==
              track_id_to_demux_stream_map_.end());
-      track_id_to_demux_stream_map_[media_track->id()] = streams_[i];
+      track_id_to_demux_stream_map_[media_track->id()] = streams_[i].get();
     } else if (codec_type == AVMEDIA_TYPE_VIDEO) {
-      CHECK(!video_stream);
-      video_stream = stream;
-      video_config = streams_[i]->video_decoder_config();
+      VideoDecoderConfig video_config = streams_[i]->video_decoder_config();
 
       RecordVideoCodecStats(video_config, stream->codec->color_range,
                             media_log_.get());
 
       media_track = media_tracks->AddVideoTrack(video_config, track_id, "main",
                                                 track_label, track_language);
       media_track->set_id(base::UintToString(track_id));
       DCHECK(track_id_to_demux_stream_map_.find(media_track->id()) ==
              track_id_to_demux_stream_map_.end());
-      track_id_to_demux_stream_map_[media_track->id()] = streams_[i];
+      track_id_to_demux_stream_map_[media_track->id()] = streams_[i].get();
     }
 
     max_duration = std::max(max_duration, streams_[i]->duration());
 
     const base::TimeDelta start_time =
         ExtractStartTime(stream, start_time_estimates[i]);
     const bool has_start_time = start_time != kNoTimestamp;
 
     if (!has_start_time)
       continue;
 
     streams_[i]->set_start_time(start_time);
     if (start_time < start_time_) {
       start_time_ = start_time;
     }
   }
 
   RecordDetectedTrackTypeStats(detected_audio_track_count,
                                detected_video_track_count,
                                detected_text_track_count);
 
-  if (!audio_stream && !video_stream) {
+  if (media_tracks->tracks().size() == 0) {

[DaleCurtis, 2016/08/29 18:15:20]

.empty?

[servolk, 2016/08/29 18:38:20]

Done.

     MEDIA_LOG(ERROR, media_log_) << GetDisplayName()
                                  << ": no supported streams";
     status_cb.Run(DEMUXER_ERROR_NO_SUPPORTED_STREAMS);
     return;
   }
 
   if (text_enabled_)
     AddTextStreams();
 
   if (format_context->duration != static_cast<int64_t>(AV_NOPTS_VALUE)) {
@@ skipping 17 matching lines @@
   // not be exposed to negative timestamps.  Which means we need to rebase these
   // negative timestamps and mark them for discard post decoding.
   //
   // Post-decode frame dropping for packets with negative timestamps is outlined
   // in section A.2 in the Ogg Vorbis spec:
   // http://xiph.org/vorbis/doc/Vorbis_I_spec.html
   //
   // FFmpeg's use of negative timestamps for opus pre-skip is nonstandard, but
   // for more information on pre-skip see section 4.2 of the Ogg Opus spec:
   // https://tools.ietf.org/html/draft-ietf-codec-oggopus-08#section-4.2
-  if (audio_stream && (audio_stream->codec->codec_id == AV_CODEC_ID_OPUS ||
-                       (strcmp(format_context->iformat->name, "ogg") == 0 &&
-                        audio_stream->codec->codec_id == AV_CODEC_ID_VORBIS))) {
-    for (size_t i = 0; i < streams_.size(); ++i) {
-      if (!streams_[i])
-        continue;
-      streams_[i]->enable_negative_timestamp_fixups();
+  for (const auto& stream : streams_) {

[DaleCurtis, 2016/08/29 18:15:20]

The behavior here is different. Previously it was only enabled if the selected
stream has negative timestamps, this will enable the fixups if any audio stream
has negative timestamps. Practically it may not matter.

[servolk, 2016/08/29 18:38:21]

It think it might matter. If we do have a stream that requires those fix ups,
that stream may not be enabled at first. But what if it gets enabled later? We
may not be able to adjust timestamps on buffers already in the pipeline, so it's
better to enable fix ups before we start the pipeline.

+    if (!stream || stream->type() != DemuxerStream::AUDIO)
+      continue;
+    const AVStream* audio_stream = stream->av_stream();
+    DCHECK(audio_stream);
+    if (audio_stream->codec->codec_id == AV_CODEC_ID_OPUS ||
+        (strcmp(format_context->iformat->name, "ogg") == 0 &&

[DaleCurtis, 2016/08/29 18:15:21]

This can be moved outside the loop.

[servolk, 2016/08/29 18:38:21]

Are you talking about strcmp? But it's bound to the codec_id == VORBIS check
with an &&, and then codec_id check has to happen on each stream inside the
loop. If we moved that strcmp outside the loop that would change the behavior,
no?

[DaleCurtis, 2016/08/29 18:49:29]

Ah, sorry I forgot this isn't explicitly for ogg anymore since we found negative
ts in webm too.

[servolk, 2016/08/29 20:25:28]

Acknowledged.

+         audio_stream->codec->codec_id == AV_CODEC_ID_VORBIS)) {
+      for (size_t i = 0; i < streams_.size(); ++i) {
+        if (!streams_[i])
+          continue;
+        streams_[i]->enable_negative_timestamp_fixups();
 
-      // Fixup the seeking information to avoid selecting the audio stream
-      // simply because it has a lower starting time.
-      if (streams_[i]->av_stream() == audio_stream &&
-          streams_[i]->start_time() < base::TimeDelta()) {
-        streams_[i]->set_start_time(base::TimeDelta());
+        // Fixup the seeking information to avoid selecting the audio stream
+        // simply because it has a lower starting time.
+        if (streams_[i]->av_stream() == audio_stream &&
+            streams_[i]->start_time() < base::TimeDelta()) {
+          streams_[i]->set_start_time(base::TimeDelta());
+        }
       }
     }
   }
 
   // If no start time could be determined, default to zero.
   if (start_time_ == kInfiniteDuration) {
     start_time_ = base::TimeDelta();
   }
 
   // MPEG-4 B-frames cause grief for a simple container like AVI. Enable PTS
@@ skipping 22 matching lines @@
   // initializing.
   host_->SetDuration(max_duration);
   duration_known_ = (max_duration != kInfiniteDuration);
 
   int64_t filesize_in_bytes = 0;
   url_protocol_->GetSize(&filesize_in_bytes);
   bitrate_ = CalculateBitrate(format_context, max_duration, filesize_in_bytes);
   if (bitrate_ > 0)
     data_source_->SetBitrate(bitrate_);
 
+  LogMetadata(format_context, max_duration);
+  media_tracks_updated_cb_.Run(std::move(media_tracks));
+
+  status_cb.Run(PIPELINE_OK);
+}
+
+void FFmpegDemuxer::LogMetadata(AVFormatContext* avctx,
+                                base::TimeDelta max_duration) {
   // Use a single MediaLogEvent to batch all parameter updates at once; this
   // prevents throttling of events due to the large number of updates here.
   std::unique_ptr<MediaLogEvent> metadata_event =
       media_log_->CreateEvent(MediaLogEvent::PROPERTY_CHANGE);
 
-  // Audio logging.
-  metadata_event->params.SetBoolean("found_audio_stream", !!audio_stream);
-  if (audio_stream) {
-    const AVCodecContext* audio_codec = audio_stream->codec;
-    metadata_event->params.SetString("audio_codec_name",
-                                     GetCodecName(audio_codec));
-    metadata_event->params.SetInteger("audio_channels_count",
-                                      audio_codec->channels);
-    metadata_event->params.SetString(
-        "audio_sample_format",
-        SampleFormatToString(audio_config.sample_format()));
-    metadata_event->params.SetInteger("audio_samples_per_second",
-                                      audio_config.samples_per_second());
+  DCHECK_EQ(avctx->nb_streams, streams_.size());
+  auto& params = metadata_event->params;
+  int audio_track_count = 0;
+  int video_track_count = 0;
+  for (size_t i = 0; i < streams_.size(); ++i) {

[DaleCurtis, 2016/08/29 18:15:20]

I think some of these keys might be used by MediaInternals to log UMA
histograms. Can you check that this change does not break that?

[servolk, 2016/08/29 18:38:21]

I have found only one place where these params seem to be used/checked:
https://cs.chromium.org/chromium/src/content/browser/media/media_internals.cc...
But even there some of the parameters set below are unused (e.g. video
width/height), so perhaps I'm missing something. For now I've tried to avoid
breaking anything by keeping parameter names the same as before for the first
audio/video track.
Also it looks like we are actually logging most of the relevant UMA metrics in
RecordAudioCodecStats/RecordVideoCodecStats, so perhaps this is unnecessary
anymore?

[DaleCurtis, 2016/08/29 18:49:29]

It's still necessary, it allows us to break out pipeline status by codec type. I
think the correct solution is to emit "*_codec_name" whenever the track switches
so that the uma handler has the right status.

That said, do you expect codecs to change within tracks? I think that is broken
in some cases, I.e. h264 -> vp9 would not work when GpuVideDecoder is not
present since we release VpxVideoDecoder to select FFmpegVideoDecoder. Ditto for
audio tracks switching between Opus -> Anything.

[servolk, 2016/08/29 20:25:28]

IIUC codecs will never change within tracks, but it's possible to have one video
track using h264 and another one using vp9 and switching between them
dynamically  should work, although of course that is going to need some more
changes in renderers (and even then we probably won't be able to guarantee the
switching to be completely smooth). Can we move the generation of "*_codec_name"
to renderers?

[DaleCurtis, 2016/08/29 20:29:07]

Yeah, that's definitely not going to work today nor even with changes to the
renderers. You need to change the DecoderSelector to not destruct unused
(Video|Audio)Decoders and retry the whole suite upon track change instead of
just those that haven't been tried yet.

Moving generation of _codec_name to the renderer seems fine to me.

[servolk, 2016/08/29 21:46:24]

There is probably no need to change DecoderSelector at all. As I've mentioned in
the previous comment, codec changes cannot happen within a stream/track. Codec
changes may only happen when switching tracks. But in that case we probably
don't need to reuse the DecoderStream anyway, we could probably
reset/reinitialize the audio/video renderer impl with the newly selected demuxer
stream and allow the RendererImpls to recreate the DecoderStream with the same
initial set of potential decoders. I think it would be easier than changing the
DecoderSelector/DecoderStream logic and almost the same performance-wise. WDYT?

[servolk, 2016/08/29 22:02:38]

Also, I have just looked a bit more into how this metadata info is used.  AFAICT
this data is not used for UMA at all. UMA is logged in
RecordAudioCodecStats/RecordVideoCodecStats for FFmpegDemuxer and in
StreamParserFactory::Create for MSE playback. None of them rely on this metadata
event. Looks like this is only used for the 'Player Properties' table in the
chrome://media-internals/ page. So we can probably keep it as it is and it
shouldn't break anything in UMA.

+    FFmpegDemuxerStream* stream = streams_[i].get();
+    if (!stream)
+      continue;
+    if (stream->type() == DemuxerStream::AUDIO) {
+      ++audio_track_count;
+      std::string suffix = "";
+      if (audio_track_count > 1)
+        suffix = "_track" + base::IntToString(audio_track_count);
+      const AVCodecContext* audio_codec = avctx->streams[i]->codec;
+      const AudioDecoderConfig& audio_config = stream->audio_decoder_config();
+      params.SetString("audio_codec_name" + suffix, GetCodecName(audio_codec));
+      params.SetInteger("audio_channels_count" + suffix, audio_codec->channels);
+      params.SetString("audio_sample_format" + suffix,
+                       SampleFormatToString(audio_config.sample_format()));
+      params.SetInteger("audio_samples_per_second" + suffix,
+                        audio_config.samples_per_second());
+    } else if (stream->type() == DemuxerStream::VIDEO) {
+      ++video_track_count;
+      std::string suffix = "";
+      if (video_track_count > 1)
+        suffix = "_track" + base::IntToString(video_track_count);
+      const AVCodecContext* video_codec = avctx->streams[i]->codec;
+      const VideoDecoderConfig& video_config = stream->video_decoder_config();
+      params.SetString("video_codec_name" + suffix, GetCodecName(video_codec));
+      params.SetInteger("width" + suffix, video_codec->width);
+      params.SetInteger("height" + suffix, video_codec->height);
+      params.SetInteger("coded_width" + suffix, video_codec->coded_width);
+      params.SetInteger("coded_height" + suffix, video_codec->coded_height);
+      params.SetString("time_base" + suffix,
+                       base::StringPrintf("%d/%d", video_codec->time_base.num,
+                                          video_codec->time_base.den));
+      params.SetString("video_format" + suffix,
+                       VideoPixelFormatToString(video_config.format()));
+      params.SetBoolean("video_is_encrypted" + suffix,
+                        video_config.is_encrypted());
+    }
   }
-
-  // Video logging
-  metadata_event->params.SetBoolean("found_video_stream", !!video_stream);
-  if (video_stream) {
-    const AVCodecContext* video_codec = video_stream->codec;
-    metadata_event->params.SetString("video_codec_name",
-                                     GetCodecName(video_codec));
-    metadata_event->params.SetInteger("width", video_codec->width);
-    metadata_event->params.SetInteger("height", video_codec->height);
-    metadata_event->params.SetInteger("coded_width", video_codec->coded_width);
-    metadata_event->params.SetInteger("coded_height",
-                                      video_codec->coded_height);
-    metadata_event->params.SetString(
-        "time_base", base::StringPrintf("%d/%d", video_codec->time_base.num,
-                                        video_codec->time_base.den));
-    metadata_event->params.SetString(
-        "video_format", VideoPixelFormatToString(video_config.format()));
-    metadata_event->params.SetBoolean("video_is_encrypted",
-                                      video_config.is_encrypted());
-  }
-
+  params.SetBoolean("found_audio_stream", (audio_track_count > 0));
+  params.SetBoolean("found_video_stream", (video_track_count > 0));
   SetTimeProperty(metadata_event.get(), "max_duration", max_duration);
   SetTimeProperty(metadata_event.get(), "start_time", start_time_);
   metadata_event->params.SetInteger("bitrate", bitrate_);
   media_log_->AddEvent(std::move(metadata_event));
-
-  media_tracks_updated_cb_.Run(std::move(media_tracks));
-
-  status_cb.Run(PIPELINE_OK);
 }
 
 FFmpegDemuxerStream* FFmpegDemuxer::FindPreferredStreamForSeeking(
     base::TimeDelta seek_time) {
   // If we have a selected/enabled video stream and its start time is lower
   // than the |seek_time| or unknown, then always prefer it for seeking.
   FFmpegDemuxerStream* video_stream = nullptr;
   for (const auto& stream : streams_) {
     if (stream && stream->type() == DemuxerStream::VIDEO && stream->enabled()) {
-      video_stream = stream;
+      video_stream = stream.get();
       if (video_stream->start_time() == kNoTimestamp ||
           video_stream->start_time() <= seek_time) {
-        return stream;
+        return video_stream;
       }
       break;
     }
   }
 
   // If video stream is not present or |seek_time| is lower than the video start
   // time, then try to find an enabled stream with the lowest start time.
   FFmpegDemuxerStream* lowest_start_time_stream = nullptr;
   for (const auto& stream : streams_) {
     if (!stream || !stream->enabled() || stream->start_time() == kNoTimestamp)
       continue;
     if (!lowest_start_time_stream ||
         stream->start_time() < lowest_start_time_stream->start_time()) {
-      lowest_start_time_stream = stream;
+      lowest_start_time_stream = stream.get();
     }
   }
   // If we found a stream with start time lower than |seek_time|, then use it.
   if (lowest_start_time_stream &&
       lowest_start_time_stream->start_time() <= seek_time) {
     return lowest_start_time_stream;
   }
 
   // If we couldn't find any streams with the start time lower than |seek_time|
   // then use either video (if one exists) or any audio stream.
@@ skipping 13 matching lines @@
   }
 
   if (result < 0) {
     // Use VLOG(1) instead of NOTIMPLEMENTED() to prevent the message being
     // captured from stdout and contaminates testing.
     // TODO(scherkus): Implement this properly and signal error (BUG=23447).
     VLOG(1) << "Not implemented";
   }
 
   // Tell streams to flush buffers due to seeking.
-  StreamVector::iterator iter;
-  for (iter = streams_.begin(); iter != streams_.end(); ++iter) {
-    if (*iter)
-      (*iter)->FlushBuffers();
+  for (const auto& stream : streams_) {
+    if (stream)
+      stream->FlushBuffers();
   }
 
   // Resume reading until capacity.
   ReadFrameIfNeeded();
 
   // Notify we're finished seeking.
   cb.Run(PIPELINE_OK);
 }
 
 void FFmpegDemuxer::OnEnabledAudioTracksChanged(
     const std::vector<MediaTrack::Id>& track_ids,
     base::TimeDelta currTime) {
   DCHECK(task_runner_->BelongsToCurrentThread());
-  bool enabled = false;
-  DemuxerStream* audio_stream = GetStream(DemuxerStream::AUDIO);
-  CHECK(audio_stream);
-  if (track_ids.size() > 0) {
-    DCHECK(track_id_to_demux_stream_map_[track_ids[0]] == audio_stream);
-    enabled = true;
+
+  std::set<DemuxerStream*> enabled_streams;
+  for (const auto& id : track_ids) {
+    DemuxerStream* stream = track_id_to_demux_stream_map_[id];
+    DCHECK(stream);
+    DCHECK_EQ(DemuxerStream::AUDIO, stream->type());
+    enabled_streams.insert(stream);

[DaleCurtis, 2016/08/29 18:15:20]

Is there a more efficient insert here? This can cause poor complexity by
incrementally resizing.

[DaleCurtis, 2016/08/29 18:15:20]

Is there a more efficient insert here? This can cause poor complexity by
incrementally resizing.

[servolk, 2016/08/29 18:38:20]

I think we don't need to be concerned about performance here for two reasons:
1. I suspect that the number of enabled tracks is going to be <= 2 in 99% of
cases even after full multi-track support is implemented.
2. Note that we are inserting into an std::set here, which is should have a
decent insert performance even if the number of elements is high.

   }
-  DVLOG(1) << __func__ << ": " << (enabled ? "enabling" : "disabling")
-           << " audio stream";
-  audio_stream->set_enabled(enabled, currTime);
+
+  // First disable all streams that need to be disabled and then enable streams
+  // that are enabled.
+  for (const auto& stream : streams_) {
+    if (stream->type() == DemuxerStream::AUDIO &&
+        enabled_streams.find(stream.get()) == enabled_streams.end()) {
+      DVLOG(1) << __func__ << ": disabling stream " << stream.get();
+      stream->set_enabled(false, currTime);
+    }
+  }
+  for (const auto& stream : enabled_streams) {
+    DVLOG(1) << __func__ << ": enabling stream " << stream;
+    stream->set_enabled(true, currTime);
+  }
 }
 
 void FFmpegDemuxer::OnSelectedVideoTrackChanged(
     const std::vector<MediaTrack::Id>& track_ids,
     base::TimeDelta currTime) {
   DCHECK(task_runner_->BelongsToCurrentThread());
-  bool enabled = false;
-  DemuxerStream* video_stream = GetStream(DemuxerStream::VIDEO);
-  CHECK(video_stream);
-  if (track_ids.size() > 0) {
-    DCHECK(track_id_to_demux_stream_map_[track_ids[0]] == video_stream);
-    enabled = true;
+  DCHECK_LE(track_ids.size(), 1u);
+
+  DemuxerStream* selected_stream = nullptr;
+  if (!track_ids.empty()) {
+    selected_stream = track_id_to_demux_stream_map_[track_ids[0]];
+    DCHECK(selected_stream);
+    DCHECK_EQ(DemuxerStream::VIDEO, selected_stream->type());
   }
-  DVLOG(1) << __func__ << ": " << (enabled ? "enabling" : "disabling")
-           << " video stream";
-  video_stream->set_enabled(enabled, currTime);
+
+  // First disable all streams that need to be disabled and then enable the
+  // stream that needs to be enabled (if any).
+  for (const auto& stream : streams_) {
+    if (stream->type() == DemuxerStream::VIDEO &&
+        stream.get() != selected_stream) {
+      DVLOG(1) << __func__ << ": disabling stream " << stream.get();
+      stream->set_enabled(false, currTime);
+    }
+  }
+  if (selected_stream) {
+    DVLOG(1) << __func__ << ": enabling stream " << selected_stream;
+    selected_stream->set_enabled(true, currTime);
+  }
 }
 
 void FFmpegDemuxer::ReadFrameIfNeeded() {
   DCHECK(task_runner_->BelongsToCurrentThread());
 
   // Make sure we have work to do before reading.
   if (!blocking_thread_.IsRunning() || !StreamsHaveAvailableCapacity() ||
       pending_read_ || pending_seek_) {
     return;
   }
@@ skipping 27 matching lines @@
   // - either underlying ffmpeg returned an error
   // - or FFMpegDemuxer reached the maximum allowed memory usage.
   if (result < 0 || IsMaxMemoryUsageReached()) {
     LOG(ERROR) << __func__ << " result=" << result
                << " IsMaxMemoryUsageReached=" << IsMaxMemoryUsageReached();
     // Update the duration based on the highest elapsed time across all streams
     // if it was previously unknown.
     if (!duration_known_) {
       base::TimeDelta max_duration;
 
-      for (StreamVector::iterator iter = streams_.begin();
-           iter != streams_.end();
-           ++iter) {
-        if (!*iter)
+      for (const auto& stream : streams_) {
+        if (!stream)
           continue;
 
-        base::TimeDelta duration = (*iter)->GetElapsedTime();
+        base::TimeDelta duration = stream->GetElapsedTime();
         if (duration != kNoTimestamp && duration > max_duration)
           max_duration = duration;
       }
 
       if (max_duration > base::TimeDelta()) {
         host_->SetDuration(max_duration);
         duration_known_ = true;
       }
     }
     // If we have reached the end of stream, tell the downstream filters about
@@ skipping 14 matching lines @@
     // when av_read_frame() returns success code. See bug comment for ideas:
     //
     // https://code.google.com/p/chromium/issues/detail?id=169133#c10
     if (!packet->data) {
       ScopedAVPacket new_packet(new AVPacket());
       av_new_packet(new_packet.get(), 0);
       av_packet_copy_props(new_packet.get(), packet.get());
       packet.swap(new_packet);
     }
 
-    FFmpegDemuxerStream* demuxer_stream = streams_[packet->stream_index];
+    FFmpegDemuxerStream* demuxer_stream = streams_[packet->stream_index].get();
     if (demuxer_stream->enabled())
       demuxer_stream->EnqueuePacket(std::move(packet));
   }
 
   // Keep reading until we've reached capacity.
   ReadFrameIfNeeded();
 }
 
 bool FFmpegDemuxer::StreamsHaveAvailableCapacity() {
   DCHECK(task_runner_->BelongsToCurrentThread());
-  StreamVector::iterator iter;
-  for (iter = streams_.begin(); iter != streams_.end(); ++iter) {
-    if (*iter && (*iter)->HasAvailableCapacity()) {
+  for (const auto& stream : streams_) {
+    if (stream && stream->HasAvailableCapacity())
       return true;
-    }
   }
   return false;
 }
 
 bool FFmpegDemuxer::IsMaxMemoryUsageReached() const {
   DCHECK(task_runner_->BelongsToCurrentThread());
 
   // Max allowed memory usage, all streams combined.
   const size_t kDemuxerMemoryLimit = 150 * 1024 * 1024;
 
   size_t memory_left = kDemuxerMemoryLimit;
-  for (StreamVector::const_iterator iter = streams_.begin();
-       iter != streams_.end(); ++iter) {
-    if (!(*iter))
+  for (const auto& stream : streams_) {
+    if (!stream)
       continue;
 
-    size_t stream_memory_usage = (*iter)->MemoryUsage();
+    size_t stream_memory_usage = stream->MemoryUsage();
     if (stream_memory_usage > memory_left)
       return true;
     memory_left -= stream_memory_usage;
   }
   return false;
 }
 
 void FFmpegDemuxer::StreamHasEnded() {
   DCHECK(task_runner_->BelongsToCurrentThread());
-  StreamVector::iterator iter;
-  for (iter = streams_.begin(); iter != streams_.end(); ++iter) {
-    if (!*iter)
-      continue;
-    (*iter)->SetEndOfStream();
+  for (const auto& stream : streams_) {
+    if (stream)
+      stream->SetEndOfStream();
   }
 }
 
 void FFmpegDemuxer::OnDataSourceError() {
   MEDIA_LOG(ERROR, media_log_) << GetDisplayName() << ": data source error";
   host_->OnDemuxerError(PIPELINE_ERROR_READ);
 }
 
 void FFmpegDemuxer::SetLiveness(DemuxerStream::Liveness liveness) {
   DCHECK(task_runner_->BelongsToCurrentThread());
-  for (auto* stream : streams_) {
+  for (const auto& stream : streams_) {
     if (stream)
       stream->SetLiveness(liveness);
   }
 }
 
 }  // namespace media