Render inband text tracks in the media pipeline

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 30cb0c0029857c8f19cf0b0d33dc75c143635163..7ff86ecb7bb80b71aa6dea81af0cd81ce3196930 100644
--- a/media/filters/ffmpeg_demuxer.cc
+++ b/media/filters/ffmpeg_demuxer.cc
@@ -64,6 +64,9 @@ FFmpegDemuxerStream::FFmpegDemuxerStream(
       AVStreamToVideoDecoderConfig(stream, &video_config_, true);
       is_encrypted = video_config_.is_encrypted();
       break;
+    case AVMEDIA_TYPE_SUBTITLE:
+      type_ = TEXT;
+      break;
     default:
       NOTREACHED();
       break;
@@ -110,36 +113,65 @@ void FFmpegDemuxerStream::EnqueuePacket(ScopedAVPacket packet) {
     LOG(ERROR) << "Format conversion failed.";
   }
 
-  // Get side data if any. For now, the only type of side_data is VP8 Alpha. We
-  // keep this generic so that other side_data types in the future can be
-  // handled the same way as well.
-  av_packet_split_side_data(packet.get());
-  int side_data_size = 0;
-  uint8* side_data = av_packet_get_side_data(
-      packet.get(),
-      AV_PKT_DATA_MATROSKA_BLOCKADDITIONAL,
-      &side_data_size);
-
-  // If a packet is returned by FFmpeg's av_parser_parse2() the packet will
-  // reference inner memory of FFmpeg.  As such we should transfer the packet
-  // into memory we control.
   scoped_refptr<DecoderBuffer> buffer;
-  if (side_data_size > 0) {
+
+  // Get side data if any. For now, the only types of side_data are VP8 Alpha,
+  // and WebVTT id and settings. We keep this generic so that other side_data
+  // types in the future can be handled the same way as well.
+  av_packet_split_side_data(packet.get());
+  if (type() == DemuxerStream::TEXT) {
+    int id_size = 0;
+    uint8* id_data = av_packet_get_side_data(
+        packet.get(),
+        AV_PKT_DATA_WEBVTT_IDENTIFIER,
+        &id_size);
+
+    int settings_size = 0;
+    uint8* settings_data = av_packet_get_side_data(
+        packet.get(),
+        AV_PKT_DATA_WEBVTT_SETTINGS,
+        &settings_size);
+
+    // The DecoderBuffer only supports a single side data item. In the case of
+    // a WebVTT cue, we can have potentially two side data items. In order to
+    // avoid disrupting DecoderBuffer any more than we need to, we copy both
+    // side data items onto a single one, and separate them with a marker
+    // (the byte value 0xFF is not part of the representation of any UTF8
+    // character).
+    std::basic_string<uint8> side_data;
+    side_data.append(id_data, id_size);
+    side_data.append(1, 0xFF);

[acolwell GONE FROM CHROMIUM, 2013/09/12 00:15:15]

nit: Consider just using the null terminator and std::vector<uint8> instead.

[Matthew Heaney (Chromium), 2013/09/13 19:51:54]

I appended a NUL to each sub-string, instead of using 0xFF as a separator.

+    side_data.append(settings_data, settings_size);
+
     buffer = DecoderBuffer::CopyFrom(packet.get()->data, packet.get()->size,
-                                     side_data, side_data_size);
+                                     side_data.data(), side_data.length());
   } else {
-    buffer = DecoderBuffer::CopyFrom(packet.get()->data, packet.get()->size);
-  }
+    int side_data_size = 0;
+    uint8* side_data = av_packet_get_side_data(

[acolwell GONE FROM CHROMIUM, 2013/09/12 00:15:15]

Why is this change needed for non-texttrack data?

[Matthew Heaney (Chromium), 2013/09/13 19:51:54]

I'm not quite following here.  What change are you referring to?

[acolwell GONE FROM CHROMIUM, 2013/09/13 20:57:30]

IIUC This is the non-text track code path. The original code does not appear to
deal with side_data, so I was wondering why you added it here. It just seems
like a change in behavior that is unrelated to the text track work.

[Matthew Heaney (Chromium), 2013/09/20 23:53:54]

I checked master again -- this bit of code was already there.  It only applied
to A/V streams so I moved it to the else part of the predicate.

+        packet.get(),
+        AV_PKT_DATA_MATROSKA_BLOCKADDITIONAL,
+        &side_data_size);
+
+    // If a packet is returned by FFmpeg's av_parser_parse2() the packet will
+    // reference inner memory of FFmpeg.  As such we should transfer the packet
+    // into memory we control.
+    if (side_data_size > 0) {
+      buffer = DecoderBuffer::CopyFrom(packet.get()->data, packet.get()->size,
+                                       side_data, side_data_size);
+    } else {
+      buffer = DecoderBuffer::CopyFrom(packet.get()->data, packet.get()->size);
+    }
 
-  if ((type() == DemuxerStream::AUDIO && audio_config_.is_encrypted()) ||
-      (type() == DemuxerStream::VIDEO && video_config_.is_encrypted())) {
-    scoped_ptr<DecryptConfig> config(WebMCreateDecryptConfig(
-        packet->data,  packet->size,
-        reinterpret_cast<const uint8*>(encryption_key_id_.data()),
-        encryption_key_id_.size()));
-    if (!config)
-      LOG(ERROR) << "Creation of DecryptConfig failed.";
-    buffer->set_decrypt_config(config.Pass());
+    if ((type() == DemuxerStream::AUDIO && audio_config_.is_encrypted()) ||

[acolwell GONE FROM CHROMIUM, 2013/09/12 00:15:15]

nit: I don't think you need to move this into the else block.

[Matthew Heaney (Chromium), 2013/09/13 19:51:54]

Done.

+        (type() == DemuxerStream::VIDEO && video_config_.is_encrypted())) {
+      scoped_ptr<DecryptConfig> config(WebMCreateDecryptConfig(
+          packet->data, packet->size,
+          reinterpret_cast<const uint8*>(encryption_key_id_.data()),
+          encryption_key_id_.size()));
+      if (!config)
+        LOG(ERROR) << "Creation of DecryptConfig failed.";
+      buffer->set_decrypt_config(config.Pass());
+    }
   }
 
   buffer->set_timestamp(ConvertStreamTimestamp(
@@ -288,6 +320,7 @@ FFmpegDemuxer::FFmpegDemuxer(
     const scoped_refptr<base::MessageLoopProxy>& message_loop,
     DataSource* data_source,
     const NeedKeyCB& need_key_cb,
+    const FFmpegAddTextTrackCB& add_text_track_cb,
     const scoped_refptr<MediaLog>& media_log)
     : host_(NULL),
       message_loop_(message_loop),
@@ -303,7 +336,8 @@ FFmpegDemuxer::FFmpegDemuxer(
       duration_known_(false),
       url_protocol_(data_source, BindToLoop(message_loop_, base::Bind(
           &FFmpegDemuxer::OnDataSourceError, base::Unretained(this)))),
-      need_key_cb_(need_key_cb) {
+      need_key_cb_(need_key_cb),
+      add_text_track_cb_(add_text_track_cb) {
   DCHECK(message_loop_.get());
   DCHECK(data_source_);
 }
@@ -393,6 +427,18 @@ DemuxerStream* FFmpegDemuxer::GetStream(DemuxerStream::Type type) {
   return GetFFmpegStream(type);
 }
 
+int FFmpegDemuxer::GetStreamCount() const {
+  return streams_.size();
+}
+
+DemuxerStream* FFmpegDemuxer::GetStreamByIndex(int idx) {
+  if (idx < 0 || StreamVector::size_type(idx) >= streams_.size()) {

[acolwell GONE FROM CHROMIUM, 2013/09/12 00:15:15]

nit: These should be DCHECKs.

[Matthew Heaney (Chromium), 2013/09/13 19:51:54]

Done.

+    return NULL;
+  } else {
+    return streams_[idx];
+  }
+}
+
 FFmpegDemuxerStream* FFmpegDemuxer::GetFFmpegStream(
     DemuxerStream::Type type) const {
   StreamVector::const_iterator iter;
@@ -494,6 +540,8 @@ void FFmpegDemuxer::OnFindStreamInfoDone(const PipelineStatusCB& status_cb,
   AVStream* video_stream = NULL;
   VideoDecoderConfig video_config;
 
+  std::vector<AVStream*> text_streams(format_context->nb_streams);
+
   base::TimeDelta max_duration;
   for (size_t i = 0; i < format_context->nb_streams; ++i) {
     AVStream* stream = format_context->streams[i];
@@ -527,6 +575,13 @@ void FFmpegDemuxer::OnFindStreamInfoDone(const PipelineStatusCB& status_cb,
       if (!video_config.IsValidConfig())
         continue;
       video_stream = stream;
+    } else if (codec_type == AVMEDIA_TYPE_SUBTITLE) {
+      if (codec_context->codec_id != AV_CODEC_ID_WEBVTT ||
+          add_text_track_cb_.is_null()) {
+        continue;
+      }
+
+      text_streams[i] = stream;

[acolwell GONE FROM CHROMIUM, 2013/09/12 00:15:15]

nit: Any harm in doing the work below, right here? Perhaps in a helper function?

[Matthew Heaney (Chromium), 2013/09/13 19:51:54]

We iterate through all the streams first, in order to determine whether we have
at least one audio or video stream.

The work we do after the iteration is to add a text track for each text steam. 
I was trying to eliminate any chance of a problem should we create text tracks,
and then report DEMUXER_ERROR_NO_SUPPORTED_STREAMS because no audio or video was
present.

[acolwell GONE FROM CHROMIUM, 2013/09/13 20:57:30]

I don't think this should be a problem. Reads on the streams should occur until
the demuxer has successfully initialized so I think you should be ok. We can
always add DCHECKS to enforce this.

[Matthew Heaney (Chromium), 2013/09/20 23:53:54]

I ended up moving that loop into its own subprogram.  I couldn't notify the
DemuxerHost of the text streams until actual DemuxerStream objects had been
created.

     } else {
       continue;
     }
@@ -547,6 +602,44 @@ void FFmpegDemuxer::OnFindStreamInfoDone(const PipelineStatusCB& status_cb,
     return;
   }
 
+  for (size_t idx = 0; idx < text_streams.size(); ++idx) {
+    AVStream* text_stream = text_streams[idx];
+    if (text_stream == NULL)
+      continue;
+
+    TextKind kind;
+
+    if (text_stream->disposition & AV_DISPOSITION_CAPTIONS) {
+      kind = kTextCaptions;
+    } else if (text_stream->disposition & AV_DISPOSITION_DESCRIPTIONS) {
+      kind = kTextDescriptions;
+    } else if (text_stream->disposition & AV_DISPOSITION_METADATA) {
+      kind = kTextMetadata;
+    } else {
+      kind = kTextSubtitles;
+    }
+
+    AVDictionaryEntry* text_title =
+        av_dict_get(text_stream->metadata, "title", NULL, 0);
+
+    std::string title;
+
+    if (text_title != NULL && text_title->value != NULL) {

[acolwell GONE FROM CHROMIUM, 2013/09/12 00:15:15]

nit: move av_dict_get() & NULL checks to a helper method so this code can look
like

std::string title = getMetadata(text_stream, "title");
std::string language = getMetadata(text_stream, "language");

I think that would make this a little easier to follow.

[Matthew Heaney (Chromium), 2013/09/13 19:51:54]

Done.

+      title = text_title->value;
+    }
+
+    AVDictionaryEntry* text_language =
+        av_dict_get(text_stream->metadata, "language", NULL, 0);
+
+    std::string language;
+
+    if (text_language != NULL && text_language->value != NULL) {
+      language = text_language->value;
+    }
+
+    add_text_track_cb_.Run(kind, title, language, idx);
+  }
+
   if (format_context->duration != static_cast<int64_t>(AV_NOPTS_VALUE)) {
     // If there is a duration value in the container use that to find the
     // maximum between it and the duration from A/V streams.