[FFmpeg-devel] [PATCH] WebM muxer writes WebVTT subtitle track

Clément Bœsch ubitux at gmail.com
Sat Jun 29 13:30:31 CEST 2013


On Fri, Jun 28, 2013 at 11:16:57AM -0700, Matthew Heaney wrote:
> The Matroska muxer now allows WebVTT subtitle tracks to be written
> while in WebM muxing mode.
> 
> WebVTT subtitle tracks have four kinds: "subtitles", "captions",
> "descriptions", and "metadata". Each text track kind has a distinct
> Mastroska CodecID and track type, as described in the temporal
> metadata guidelines here:
> 
> http://wiki.webmproject.org/webm-metadata/temporal-metadata/webvtt-in-webm
> 
> When the stream has codec id AV_CODEC_ID_WEBVTT, the stream packet is
> serialized per the temporal metadata guidelines cited above. The
> WebVTT cue is written as a Matroska block group. The block frame
> comprises the WebVTT cue id, followed by the cue settings, followed by
> the cue text.  (The block timestamp is synthesized from the cue
> timestamp.)
> ---
>  libavformat/matroska.h    |   1 +
>  libavformat/matroskaenc.c | 106 ++++++++++++++++++++++++++++++++++++++--------
>  2 files changed, 89 insertions(+), 18 deletions(-)
> 
> diff --git a/libavformat/matroska.h b/libavformat/matroska.h
> index af662be..d97474b 100644
> --- a/libavformat/matroska.h
> +++ b/libavformat/matroska.h
> @@ -230,6 +230,7 @@ typedef enum {
>    MATROSKA_TRACK_TYPE_LOGO     = 0x10,
>    MATROSKA_TRACK_TYPE_SUBTITLE = 0x11,
>    MATROSKA_TRACK_TYPE_CONTROL  = 0x20,

> +  MATROSKA_TRACK_TYPE_METADATA = 0x21

If you add a trailing comma so next addition won't affect this line

>  } MatroskaTrackType;
>  
>  typedef enum {
> diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
> index f34813c..a9a02e6 100644
> --- a/libavformat/matroskaenc.c
> +++ b/libavformat/matroskaenc.c
> @@ -599,8 +599,13 @@ static int mkv_write_tracks(AVFormatContext *s)
>  

>          if ((tag = av_dict_get(st->metadata, "title", NULL, 0)))
>              put_ebml_string(pb, MATROSKA_ID_TRACKNAME, tag->value);
> +

nit: I don't mind but this looks unrelated

>          tag = av_dict_get(st->metadata, "language", NULL, 0);
> -        put_ebml_string(pb, MATROSKA_ID_TRACKLANGUAGE, tag ? tag->value:"und");
> +        if (mkv->mode != MODE_WEBM || codec->codec_id != AV_CODEC_ID_WEBVTT) {
> +            put_ebml_string(pb, MATROSKA_ID_TRACKLANGUAGE, tag ? tag->value:"und");
> +        } else if (tag && tag->value) {
> +            put_ebml_string(pb, MATROSKA_ID_TRACKLANGUAGE, tag->value);
> +        }
>  
>          if (default_stream_exists) {
>              put_ebml_uint(pb, MATROSKA_ID_TRACKFLAGDEFAULT, !!(st->disposition & AV_DISPOSITION_DEFAULT));
> @@ -608,21 +613,32 @@ static int mkv_write_tracks(AVFormatContext *s)
>          if (st->disposition & AV_DISPOSITION_FORCED)
>              put_ebml_uint(pb, MATROSKA_ID_TRACKFLAGFORCED, 1);
>  

> -        // look for a codec ID string specific to mkv to use,
> -        // if none are found, use AVI codes
> -        for (j = 0; ff_mkv_codec_tags[j].id != AV_CODEC_ID_NONE; j++) {
> -            if (ff_mkv_codec_tags[j].id == codec->codec_id) {
> -                put_ebml_string(pb, MATROSKA_ID_CODECID, ff_mkv_codec_tags[j].str);
> -                native_id = 1;
> -                break;

Please keep this block not re-indented, this eases review within mailers;
see how it's done for example in 3db3b278. I'll take care of the re-indent
in the next commit, don't bother sending another patch on top of this one.

> +        if (mkv->mode == MODE_WEBM && codec->codec_id == AV_CODEC_ID_WEBVTT) {

> +            const char* codec_id;

nitstyle: const char *codec_id

> +            if (st->disposition & AV_DISPOSITION_CAPTIONS) {
> +                codec_id = "D_WEBVTT/CAPTIONS";
> +                native_id = MATROSKA_TRACK_TYPE_SUBTITLE;
> +            } else if (st->disposition & AV_DISPOSITION_DESCRIPTIONS) {
> +                codec_id = "D_WEBVTT/DESCRIPTIONS";
> +                native_id = MATROSKA_TRACK_TYPE_METADATA;
> +            } else if (st->disposition & AV_DISPOSITION_METADATA) {
> +                codec_id = "D_WEBVTT/METADATA";
> +                native_id = MATROSKA_TRACK_TYPE_METADATA;
> +            } else {
> +                codec_id = "D_WEBVTT/SUBTITLES";
> +                native_id = MATROSKA_TRACK_TYPE_SUBTITLE;
> +            }
> +            put_ebml_string(pb, MATROSKA_ID_CODECID, codec_id);
> +        } else {
> +            // look for a codec ID string specific to mkv to use,
> +            // if none are found, use AVI codes
> +            for (j = 0; ff_mkv_codec_tags[j].id != AV_CODEC_ID_NONE; j++) {
> +                if (ff_mkv_codec_tags[j].id == codec->codec_id) {
> +                    put_ebml_string(pb, MATROSKA_ID_CODECID, ff_mkv_codec_tags[j].str);
> +                    native_id = 1;
> +                    break;
> +                }
>              }
> -        }
> -

> -        if (mkv->mode == MODE_WEBM && !(codec->codec_id == AV_CODEC_ID_VP8 ||
> -                                        codec->codec_id == AV_CODEC_ID_VORBIS)) {
> -            av_log(s, AV_LOG_ERROR,
> -                   "Only VP8 video and Vorbis audio are supported for WebM.\n");
> -            return AVERROR(EINVAL);

How is this checked now?

>          }
>  
>          switch (codec->codec_type) {
> @@ -715,18 +731,27 @@ static int mkv_write_tracks(AVFormatContext *s)
>                  break;
>  

>              case AVMEDIA_TYPE_SUBTITLE:
> -                put_ebml_uint(pb, MATROSKA_ID_TRACKTYPE, MATROSKA_TRACK_TYPE_SUBTITLE);
>                  if (!native_id) {
>                      av_log(s, AV_LOG_ERROR, "Subtitle codec %d is not supported.\n", codec->codec_id);
>                      return AVERROR(ENOSYS);
>                  }
> +
> +                if (mkv->mode == MODE_WEBM && codec->codec_id == AV_CODEC_ID_WEBVTT) {
> +                    put_ebml_uint(pb, MATROSKA_ID_TRACKTYPE, native_id);
> +                } else {
> +                    put_ebml_uint(pb, MATROSKA_ID_TRACKTYPE, MATROSKA_TRACK_TYPE_SUBTITLE);
> +                }
> +

Can't you use native_id all the time here? It looks better to set
native_id to MATROSKA_TRACK_TYPE_SUBTITLE when appropriate.

>                  break;
>              default:
>                  av_log(s, AV_LOG_ERROR, "Only audio, video, and subtitles are supported for Matroska.\n");
>                  return AVERROR(EINVAL);
>          }
> -        ret = mkv_write_codecprivate(s, pb, codec, native_id, qt_id);
> -        if (ret < 0) return ret;
> +
> +        if (mkv->mode != MODE_WEBM || codec->codec_id != AV_CODEC_ID_WEBVTT) {
> +            ret = mkv_write_codecprivate(s, pb, codec, native_id, qt_id);
> +            if (ret < 0) return ret;
> +        }
>  
>          end_ebml_master(pb, track);
>  
> @@ -1306,6 +1331,48 @@ static int mkv_write_srt_blocks(AVFormatContext *s, AVIOContext *pb, AVPacket *p
>      return duration;
>  }
>  

> +static int mkv_write_vtt_blocks(AVFormatContext *s, AVIOContext *pb, AVPacket *pkt) {

stylenit: break line between ) and {

> +    MatroskaMuxContext *mkv = s->priv_data;
> +    ebml_master blockgroup;
> +    int id_size, settings_size, size;
> +    uint8_t *id, *settings;
> +    int64_t ts = mkv->tracks[pkt->stream_index].write_dts ? pkt->dts : pkt->pts;
> +    const int flags = 0;
> +    const uint8_t EOL[] = "\n";
> +
> +    id_size = 0;
> +    id = av_packet_get_side_data(pkt, AV_PKT_DATA_WEBVTT_IDENTIFIER,
> +                                 &id_size);
> +
> +    settings_size = 0;
> +    settings = av_packet_get_side_data(pkt, AV_PKT_DATA_WEBVTT_SETTINGS,
> +                                       &settings_size);
> +
> +    size = id_size + 1 + settings_size + 1 + pkt->size;
> +
> +    av_log(s, AV_LOG_DEBUG, "Writing block at offset %" PRIu64 ", size %d, "
> +           "pts %" PRId64 ", dts %" PRId64 ", duration %d, flags %d\n",
> +           avio_tell(pb), size, pkt->pts, pkt->dts, pkt->duration, flags);
> +
> +    blockgroup = start_ebml_master(pb, MATROSKA_ID_BLOCKGROUP, mkv_blockgroup_size(size));
> +
> +    put_ebml_id(pb, MATROSKA_ID_BLOCK);
> +    put_ebml_num(pb, size+4, 0);
> +    avio_w8(pb, 0x80 | (pkt->stream_index + 1));     // this assumes stream_index is less than 126
> +    avio_wb16(pb, ts - mkv->cluster_pts);
> +    avio_w8(pb, flags);

> +    avio_write(pb, id, id_size);
> +    avio_write(pb, EOL, 1);
> +    avio_write(pb, settings, settings_size);
> +    avio_write(pb, EOL, 1);
> +    avio_write(pb, pkt->data, pkt->size);

AFAICT you can do that in one avio_printf()

> +
> +    put_ebml_uint(pb, MATROSKA_ID_BLOCKDURATION, pkt->duration);
> +    end_ebml_master(pb, blockgroup);
> +
> +    return pkt->duration;
> +}
> +
>  static void mkv_flush_dynbuf(AVFormatContext *s)
>  {
>      MatroskaMuxContext *mkv = s->priv_data;
> @@ -1361,6 +1428,8 @@ static int mkv_write_packet_internal(AVFormatContext *s, AVPacket *pkt)
>  #endif
>      } else if (codec->codec_id == AV_CODEC_ID_SRT) {
>          duration = mkv_write_srt_blocks(s, pb, pkt);
> +    } else if (codec->codec_id == AV_CODEC_ID_WEBVTT) {
> +        duration = mkv_write_vtt_blocks(s, pb, pkt);
>      } else {
>          ebml_master blockgroup = start_ebml_master(pb, MATROSKA_ID_BLOCKGROUP, mkv_blockgroup_size(pkt->size));
>          /* For backward compatibility, prefer convergence_duration. */
> @@ -1604,6 +1673,7 @@ AVOutputFormat ff_webm_muxer = {
>      .priv_data_size    = sizeof(MatroskaMuxContext),
>      .audio_codec       = AV_CODEC_ID_VORBIS,
>      .video_codec       = AV_CODEC_ID_VP8,
> +    .subtitle_codec    = AV_CODEC_ID_WEBVTT,
>      .write_header      = mkv_write_header,
>      .write_packet      = mkv_write_packet,
>      .write_trailer     = mkv_write_trailer,

Note: do you have a patch for our demuxer to test this?

-- 
Clément B.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 490 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20130629/9b75a972/attachment.asc>


More information about the ffmpeg-devel mailing list