Allow MP4 parser to handle multiple audio and video tracks

media/formats/mp4/mp4_stream_parser.cc

Index: media/formats/mp4/mp4_stream_parser.cc
diff --git a/media/formats/mp4/mp4_stream_parser.cc b/media/formats/mp4/mp4_stream_parser.cc
index 7d54bd95a1d6287ce620195b772d0ca3c6731dfc..ca4316803505e554750730178155820a3cb316fd 100644
--- a/media/formats/mp4/mp4_stream_parser.cc
+++ b/media/formats/mp4/mp4_stream_parser.cc
@@ -41,12 +41,8 @@ MP4StreamParser::MP4StreamParser(const std::set<int>& audio_object_types,
       highest_end_offset_(0),
       has_audio_(false),
       has_video_(false),
-      audio_track_id_(0),
-      video_track_id_(0),
       audio_object_types_(audio_object_types),
       has_sbr_(has_sbr),
-      is_audio_track_encrypted_(false),
-      is_video_track_encrypted_(false),
       num_top_level_box_skipped_(0) {
 }
 
@@ -186,6 +182,9 @@ bool MP4StreamParser::ParseMoov(BoxReader* reader) {
   moov_.reset(new Movie);
   RCHECK(moov_->Parse(reader));
   runs_.reset();
+  audio_track_ids_.clear();
+  video_track_ids_.clear();
+  is_track_encrypted_.clear();
 
   has_audio_ = false;
   has_video_ = false;
@@ -221,8 +220,6 @@ bool MP4StreamParser::ParseMoov(BoxReader* reader) {
 
     if (track->media.handler.type == kAudio) {
       detected_audio_track_count++;
-      if (audio_config.IsValidConfig())
-        continue;  // Skip other audio tracks once we found a supported one.
 
       RCHECK(!samp_descr.audio_entries.empty());
 
@@ -311,20 +308,23 @@ bool MP4StreamParser::ParseMoov(BoxReader* reader) {
         return false;
       }
 
-      is_audio_track_encrypted_ = entry.sinf.info.track_encryption.is_encrypted;
-      DVLOG(1) << "is_audio_track_encrypted_: " << is_audio_track_encrypted_;
+      uint32_t audio_track_id = track->header.track_id;
+      bool is_track_encrypted = entry.sinf.info.track_encryption.is_encrypted;
+      is_track_encrypted_[audio_track_id] = is_track_encrypted;
       audio_config.Initialize(
           codec, sample_format, channel_layout, sample_per_second, extra_data,
-          is_audio_track_encrypted_ ? AesCtrEncryptionScheme() : Unencrypted(),
+          is_track_encrypted ? AesCtrEncryptionScheme() : Unencrypted(),
           base::TimeDelta(), 0);
+      DVLOG(1) << "audio_track_id=" << audio_track_id
+               << " config=" << audio_config.AsHumanReadableString();
       if (!audio_config.IsValidConfig()) {
         MEDIA_LOG(ERROR, media_log_) << "Invalid audio decoder config: "
                                      << audio_config.AsHumanReadableString();
         return false;
       }
       has_audio_ = true;
-      audio_track_id_ = track->header.track_id;
-      media_tracks->AddAudioTrack(audio_config, audio_track_id_, "main",
+      audio_track_ids_.insert(audio_track_id);
+      media_tracks->AddAudioTrack(audio_config, audio_track_id, "main",

[wolenetz, 2016/08/23 22:51:42]

Are all audio tracks "main" in a multi-track audio mp4? IIRC, the inband
sourcing spec may help answer this.
Consult: https://dev.w3.org/html5/html-sourcing-inband-tracks/#mpeg4 "Track
Attributes for sourced Audio and Video Tracks" section.

[servolk, 2016/08/24 00:53:49]

Done.

                                   track->media.handler.name,
                                   track->media.header.language());
       continue;
@@ -332,8 +332,6 @@ bool MP4StreamParser::ParseMoov(BoxReader* reader) {
 
     if (track->media.handler.type == kVideo) {
       detected_video_track_count++;
-      if (video_config.IsValidConfig())
-        continue;  // Skip other video tracks once we found a supported one.
 
       RCHECK(!samp_descr.video_entries.empty());
       if (desc_idx >= samp_descr.video_entries.size())
@@ -364,23 +362,26 @@ bool MP4StreamParser::ParseMoov(BoxReader* reader) {
             gfx::Size(track->header.width, track->header.height);
       }
 
-      is_video_track_encrypted_ = entry.sinf.info.track_encryption.is_encrypted;
-      DVLOG(1) << "is_video_track_encrypted_: " << is_video_track_encrypted_;
+      uint32_t video_track_id = track->header.track_id;
+      bool is_track_encrypted = entry.sinf.info.track_encryption.is_encrypted;
+      is_track_encrypted_[video_track_id] = is_track_encrypted;

[chcunningham1, 2016/08/23 01:54:32]

Maybe capture the return value and check that its not already present in the set
(i.e. confirm media does not re-use track ids)?

[wolenetz, 2016/08/23 22:51:42]

+1 to unique track id checking (here and in the audio case)

[servolk, 2016/08/24 00:53:49]

Done.

       video_config.Initialize(
           entry.video_codec, entry.video_codec_profile, PIXEL_FORMAT_YV12,
           COLOR_SPACE_HD_REC709, coded_size, visible_rect, natural_size,
           // No decoder-specific buffer needed for AVC;
           // SPS/PPS are embedded in the video stream
           EmptyExtraData(),
-          is_video_track_encrypted_ ? AesCtrEncryptionScheme() : Unencrypted());
+          is_track_encrypted ? AesCtrEncryptionScheme() : Unencrypted());
+      DVLOG(1) << "video_track_id=" << video_track_id
+               << " config=" << video_config.AsHumanReadableString();
       if (!video_config.IsValidConfig()) {
         MEDIA_LOG(ERROR, media_log_) << "Invalid video decoder config: "
                                      << video_config.AsHumanReadableString();
         return false;
       }
       has_video_ = true;
-      video_track_id_ = track->header.track_id;
-      media_tracks->AddVideoTrack(video_config, video_track_id_, "main",
+      video_track_ids_.insert(video_track_id);
+      media_tracks->AddVideoTrack(video_config, video_track_id, "main",

[wolenetz, 2016/08/23 22:51:42]

ditto: "main" is only for the first video track.

[servolk, 2016/08/24 00:53:49]

Done.

                                   track->media.handler.name,
                                   track->media.header.language());
       continue;
@@ -517,8 +518,10 @@ bool MP4StreamParser::EnqueueSample(BufferQueueMap* buffers, bool* err) {
   queue_.Peek(&buf, &buf_size);
   if (!buf_size) return false;
 
-  bool audio = has_audio_ && audio_track_id_ == runs_->track_id();
-  bool video = has_video_ && video_track_id_ == runs_->track_id();
+  bool audio =
+      audio_track_ids_.find(runs_->track_id()) != audio_track_ids_.end();
+  bool video =
+      video_track_ids_.find(runs_->track_id()) != video_track_ids_.end();
 
   // Skip this entire track if it's not one we're interested in
   if (!audio && !video) {
@@ -588,8 +591,7 @@ bool MP4StreamParser::EnqueueSample(BufferQueueMap* buffers, bool* err) {
         subsamples));
     }
     // else, use the existing config.
-  } else if ((audio && is_audio_track_encrypted_) ||
-             (video && is_video_track_encrypted_)) {
+  } else if (is_track_encrypted_[runs_->track_id()]) {
     // The media pipeline requires a DecryptConfig with an empty |iv|.
     // TODO(ddorwin): Refactor so we do not need a fake key ID ("1");
     decrypt_config.reset(
@@ -599,9 +601,6 @@ bool MP4StreamParser::EnqueueSample(BufferQueueMap* buffers, bool* err) {
   StreamParserBuffer::Type buffer_type = audio ? DemuxerStream::AUDIO :
       DemuxerStream::VIDEO;
 
-  // TODO(wolenetz/acolwell): Validate and use a common cross-parser TrackId
-  // type and allow multiple tracks for same media type, if applicable. See
-  // https://crbug.com/341581.
   scoped_refptr<StreamParserBuffer> stream_buf = StreamParserBuffer::CopyFrom(
       &frame_buf[0], frame_buf.size(), runs_->is_keyframe(), buffer_type,
       runs_->track_id());
@@ -613,7 +612,8 @@ bool MP4StreamParser::EnqueueSample(BufferQueueMap* buffers, bool* err) {
   stream_buf->set_timestamp(runs_->cts());
   stream_buf->SetDecodeTimestamp(runs_->dts());
 
-  DVLOG(3) << "Pushing frame: aud=" << audio
+  DVLOG(3) << "Emit " << (audio ? "audio" : "video") << " frame: "

[chcunningham1, 2016/08/23 01:54:32]

what if the track is text? will you say "video" here?

[wolenetz, 2016/08/23 22:51:42]

At the moment, Chrome MSE mp4 parser doesn't parse text tracks.

[servolk, 2016/08/24 00:53:49]

Yup, text tracks in .mp4 are not supported for now.

+           << " track_id=" << runs_->track_id()
            << ", key=" << runs_->is_keyframe()
            << ", dur=" << runs_->duration().InMilliseconds()
            << ", dts=" << runs_->dts().InMilliseconds()