Implement support for multiple tracks in FFmpegDemuxer

media/filters/ffmpeg_demuxer.cc

Index: media/filters/ffmpeg_demuxer.cc
diff --git a/media/filters/ffmpeg_demuxer.cc b/media/filters/ffmpeg_demuxer.cc
index 49d18d3823f115a335a1e0b88df7c572b2c46960..1c805cd4c7e61b724d12286bf656caceee62662e 100644
--- a/media/filters/ffmpeg_demuxer.cc
+++ b/media/filters/ffmpeg_demuxer.cc
@@ -895,10 +895,9 @@ void FFmpegDemuxer::Stop() {
   // 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;
@@ -980,10 +979,9 @@ DemuxerStream* FFmpegDemuxer::GetStream(DemuxerStream::Type 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;
@@ -996,9 +994,8 @@ base::TimeDelta FFmpegDemuxer::GetStartTime() const {
 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();
@@ -1007,14 +1004,14 @@ void FFmpegDemuxer::AddTextStreams() {
 
     // 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();
   }
@@ -1164,10 +1161,6 @@ void FFmpegDemuxer::OnFindStreamInfoDone(const PipelineStatusCB& status_cb,
   }
 
   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());
 
@@ -1189,12 +1182,6 @@ void FFmpegDemuxer::OnFindStreamInfoDone(const PipelineStatusCB& status_cb,
       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.
@@ -1202,12 +1189,6 @@ void FFmpegDemuxer::OnFindStreamInfoDone(const PipelineStatusCB& status_cb,
                                   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
@@ -1244,7 +1225,7 @@ void FFmpegDemuxer::OnFindStreamInfoDone(const PipelineStatusCB& status_cb,
     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_)
@@ -1278,9 +1259,7 @@ void FFmpegDemuxer::OnFindStreamInfoDone(const PipelineStatusCB& status_cb,
     // 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",
@@ -1288,11 +1267,9 @@ void FFmpegDemuxer::OnFindStreamInfoDone(const PipelineStatusCB& status_cb,
       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());
@@ -1302,7 +1279,7 @@ void FFmpegDemuxer::OnFindStreamInfoDone(const PipelineStatusCB& status_cb,
       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());
@@ -1324,7 +1301,7 @@ void FFmpegDemuxer::OnFindStreamInfoDone(const PipelineStatusCB& status_cb,
                                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);
@@ -1362,19 +1339,25 @@ void FFmpegDemuxer::OnFindStreamInfoDone(const PipelineStatusCB& status_cb,
   // 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());
+        }
       }
     }
   }
@@ -1417,54 +1400,67 @@ void FFmpegDemuxer::OnFindStreamInfoDone(const PipelineStatusCB& status_cb,
   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());
-  }
-
-  // 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());
+  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());
+    }
   }
-
+  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(
@@ -1474,10 +1470,10 @@ FFmpegDemuxerStream* FFmpegDemuxer::FindPreferredStreamForSeeking(
   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;
     }
@@ -1491,7 +1487,7 @@ FFmpegDemuxerStream* FFmpegDemuxer::FindPreferredStreamForSeeking(
       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.
@@ -1525,10 +1521,9 @@ void FFmpegDemuxer::OnSeekFrameDone(const PipelineStatusCB& cb, int result) {
   }
 
   // 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.
@@ -1542,32 +1537,56 @@ 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.

+  }
+
+  // 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);
   }
-  DVLOG(1) << __func__ << ": " << (enabled ? "enabling" : "disabling")
-           << " audio stream";
-  audio_stream->set_enabled(enabled, 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());
+  }
+
+  // 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);
   }
-  DVLOG(1) << __func__ << ": " << (enabled ? "enabling" : "disabling")
-           << " video stream";
-  video_stream->set_enabled(enabled, currTime);
 }
 
 void FFmpegDemuxer::ReadFrameIfNeeded() {
@@ -1615,13 +1634,11 @@ void FFmpegDemuxer::OnReadFrameDone(ScopedAVPacket packet, int result) {
     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;
       }
@@ -1656,7 +1673,7 @@ void FFmpegDemuxer::OnReadFrameDone(ScopedAVPacket packet, int result) {
       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));
   }
@@ -1667,11 +1684,9 @@ void FFmpegDemuxer::OnReadFrameDone(ScopedAVPacket packet, int result) {
 
 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;
 }
@@ -1683,12 +1698,11 @@ bool FFmpegDemuxer::IsMaxMemoryUsageReached() const {
   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;
@@ -1698,11 +1712,9 @@ bool FFmpegDemuxer::IsMaxMemoryUsageReached() const {
 
 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();
   }
 }
 
@@ -1713,7 +1725,7 @@ void FFmpegDemuxer::OnDataSourceError() {
 
 void FFmpegDemuxer::SetLiveness(DemuxerStream::Liveness liveness) {
   DCHECK(task_runner_->BelongsToCurrentThread());
-  for (auto* stream : streams_) {
+  for (const auto& stream : streams_) {
     if (stream)
       stream->SetLiveness(liveness);
   }