Allow media track switching.

media/blink/webmediaplayer_impl.cc

 // Copyright 2013 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/blink/webmediaplayer_impl.h"
 
 #include <algorithm>
 #include <cmath>
 #include <limits>
 #include <string>
@@ skipping 1022 matching lines @@
 }
 
 void WebMediaPlayerImpl::OnFFmpegMediaTracksUpdated(
     std::unique_ptr<MediaTracks> tracks) {
   // For MSE/chunk_demuxer case the media track updates are handled by
   // WebSourceBufferImpl.
   DCHECK(demuxer_.get());
   DCHECK(!chunk_demuxer_);
 
   // Report the media track information to blink.
+  bool first_audio_track = true;

[xhwang, 2017/03/27 19:13:53]

is_first_audio_track ?

[servolk, 2017/03/27 22:34:12]

Done.

+  bool first_video_track = true;
   for (const auto& track : tracks->tracks()) {
     if (track->type() == MediaTrack::Audio) {
       client_->addAudioTrack(blink::WebString::fromUTF8(track->id()),
                              blink::WebMediaPlayerClient::AudioTrackKindMain,
                              blink::WebString::fromUTF8(track->label()),
                              blink::WebString::fromUTF8(track->language()),
-                             /*enabled*/ true);
+                             /*enabled*/ first_audio_track);

[xhwang, 2017/03/27 19:13:53]

This line doesn't make sense now :)

[servolk, 2017/03/27 22:34:12]

If you are talking about the comment, it still does make sense. The comment
clarifies that the last parameter is 'bool enabled' and tells the client whether
the added media track should be in the enabled or disabled state initially. It
just so happens that we want only the first track to be enabled initially, and
so we pass the is_first_audio_track in there. Would it be clearer to change
comment to something like /* enabled= */ ?

[xhwang, 2017/03/29 00:16:47]

I see. Thanks for the context. I think this style where you put comments before
the variable is very rare and IMHO confusing.

Also, probably add comments about why we only enable/select the first track?

[servolk, 2017/03/29 01:49:33]

Ok, I guess I can completely remove the parameter comments, if that's confusing.
Done.

+      first_audio_track = false;
     } else if (track->type() == MediaTrack::Video) {
       client_->addVideoTrack(blink::WebString::fromUTF8(track->id()),
                              blink::WebMediaPlayerClient::VideoTrackKindMain,
                              blink::WebString::fromUTF8(track->label()),
                              blink::WebString::fromUTF8(track->language()),
-                             /*selected*/ true);
+                             /*selected*/ first_video_track);

[xhwang, 2017/03/27 19:13:53]

ditto

[servolk, 2017/03/27 22:34:12]

Acknowledged.

+      first_video_track = false;
     } else {
       // Text tracks are not supported through this code path yet.
       NOTREACHED();
     }
   }
 }
 
 void WebMediaPlayerImpl::SetCdm(blink::WebContentDecryptionModule* cdm) {
   DCHECK(main_task_runner_->BelongsToCurrentThread());
   DCHECK(cdm);
@@ skipping 1266 matching lines @@
 
 void WebMediaPlayerImpl::RecordUnderflowDuration(base::TimeDelta duration) {
   DCHECK(data_source_ || chunk_demuxer_);
   if (data_source_)
     UMA_HISTOGRAM_TIMES("Media.UnderflowDuration", duration);
   else
     UMA_HISTOGRAM_TIMES("Media.UnderflowDuration.MSE", duration);
 }
 
 }  // namespace media