[FFmpeg-devel] [PATCH] Matroska demuxer adds WebVTT support
Clément Bœsch
ubitux at gmail.com
Thu Aug 8 08:10:30 CEST 2013
On Mon, Jul 15, 2013 at 05:02:55PM -0700, Matthew Heaney wrote:
> WebM files now support inband text tracks, as described in the
> following specification:
>
> http://wiki.webmproject.org/webm-metadata/temporal-metadata/webvtt-in-webm
>
> The Matroska demuxer now detects the presence of WebVTT tracks,
> synthesizing WebVTT packets (having codec id AV_CODEC_ID_WEBVTT) and
> pushing them downstream in the normal way.
> ---
> libavformat/matroska.c | 5 ++
> libavformat/matroskadec.c | 150 +++++++++++++++++++++++++++++++++++++++++++++-
> 2 files changed, 154 insertions(+), 1 deletion(-)
>
I'm assuming this is the latest version.
> diff --git a/libavformat/matroska.c b/libavformat/matroska.c
> index ee57c18..953c572 100644
> --- a/libavformat/matroska.c
> +++ b/libavformat/matroska.c
> @@ -57,6 +57,11 @@ const CodecTags ff_mkv_codec_tags[]={
> {"A_VORBIS" , AV_CODEC_ID_VORBIS},
> {"A_WAVPACK4" , AV_CODEC_ID_WAVPACK},
>
> + {"D_WEBVTT/SUBTITLES" , AV_CODEC_ID_WEBVTT},
> + {"D_WEBVTT/CAPTIONS" , AV_CODEC_ID_WEBVTT},
> + {"D_WEBVTT/DESCRIPTIONS", AV_CODEC_ID_WEBVTT},
> + {"D_WEBVTT/METADATA" , AV_CODEC_ID_WEBVTT},
> +
> {"S_TEXT/UTF8" , AV_CODEC_ID_SUBRIP},
> {"S_TEXT/UTF8" , AV_CODEC_ID_TEXT},
> {"S_TEXT/UTF8" , AV_CODEC_ID_SRT},
> diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
> index 9a15a2b..7466258 100644
> --- a/libavformat/matroskadec.c
> +++ b/libavformat/matroskadec.c
> @@ -1566,7 +1566,8 @@ static int matroska_read_header(AVFormatContext *s)
> /* Apply some sanity checks. */
> if (track->type != MATROSKA_TRACK_TYPE_VIDEO &&
> track->type != MATROSKA_TRACK_TYPE_AUDIO &&
> - track->type != MATROSKA_TRACK_TYPE_SUBTITLE) {
> + track->type != MATROSKA_TRACK_TYPE_SUBTITLE &&
> + track->type != MATROSKA_TRACK_TYPE_METADATA) {
> av_log(matroska->ctx, AV_LOG_INFO,
> "Unknown or unsupported track type %"PRIu64"\n",
> track->type);
> @@ -1860,6 +1861,16 @@ static int matroska_read_header(AVFormatContext *s)
> st->codec->bits_per_coded_sample = track->audio.bitdepth;
> if (st->codec->codec_id != AV_CODEC_ID_AAC)
> st->need_parsing = AVSTREAM_PARSE_HEADERS;
> + } else if (codec_id == AV_CODEC_ID_WEBVTT) {
> + st->codec->codec_type = AVMEDIA_TYPE_SUBTITLE;
> +
> + if (!strcmp(track->codec_id, "D_WEBVTT/CAPTIONS")) {
> + st->disposition |= AV_DISPOSITION_CAPTIONS;
> + } else if (!strcmp(track->codec_id, "D_WEBVTT/DESCRIPTIONS")) {
> + st->disposition |= AV_DISPOSITION_DESCRIPTIONS;
> + } else if (!strcmp(track->codec_id, "D_WEBVTT/METADATA")) {
> + st->disposition |= AV_DISPOSITION_METADATA;
> + }
> } else if (track->type == MATROSKA_TRACK_TYPE_SUBTITLE) {
> st->codec->codec_type = AVMEDIA_TYPE_SUBTITLE;
> #if FF_API_ASS_SSA
> @@ -2226,6 +2237,135 @@ fail:
> return ret;
> }
>
> +static int matroska_parse_webvtt(MatroskaDemuxContext *matroska,
> + MatroskaTrack *track,
> + AVStream *st,
> + uint8_t *data, int data_len,
> + uint64_t timecode,
> + uint64_t duration,
> + int64_t pos)
> +{
> + AVPacket *pkt;
> + uint8_t *id, *settings, *text, *buf;
> + int id_len, settings_len, text_len;
> + uint8_t *p, *q;
> +
> + if (data_len <= 0)
> + return AVERROR_INVALIDDATA;
> +
> + p = data;
> + q = data + data_len;
> +
> + id = p;
> + id_len = -1;
> + while (p != q && id_len < 0) {
> + switch (*p) {
> + case '\r':
> + case '\n':
> + id_len = p - id;
> + break;
> + default:
> + p++;
> + break;
> + }
> + }
You can save a few lines with:
while (p < q) {
if (*p == '\r' || *p == '\n') {
id_len = p - id;
break;
}
p++;
}
(untested)
> +
> + if (p >= q)
> + return AVERROR_INVALIDDATA;
> +
(A) see below
> + if (*p == '\r')
> + p++;
I would include that check just after defining id_len in the previous
loop...
> + if (p >= q || *p != '\n')
> + return AVERROR_INVALIDDATA;
...which will make (A) uneeded because of that p >= q check here.
> + p++;
> +
> + if (p >= q)
> + return AVERROR_INVALIDDATA;
> +
Just check for p >= q - 1 previously to remove that redundancy.
> + settings = p;
> + settings_len = -1;
> + while (p != q && settings_len < 0) {
> + switch (*p) {
> + case '\r':
> + case '\n':
> + settings_len = p - settings;
> + break;
> + default:
> + p++;
> + break;
> + }
> + }
> +
> + if (p >= q)
> + return AVERROR_INVALIDDATA;
> +
> + if (*p == '\r')
> + p++;
> + if (p >= q || *p != '\n')
> + return AVERROR_INVALIDDATA;
> + p++;
> +
> + if (p >= q)
> + return AVERROR_INVALIDDATA;
> +
It might be possible to simplify that whole chunk the same way.
> + text = p;
> + text_len = q - p;
> + while (text_len > 0) {
> + const int len = text_len - 1;
> + const uint8_t c = p[len];
> + if (c != '\r' && c != '\n')
> + break;
> + text_len = len;
> + }
> +
> + pkt = av_mallocz(sizeof(AVPacket));
sizeof(*pkt)
> + if (av_new_packet(pkt, text_len) < 0) {
> + av_free(pkt);
> + return AVERROR(ENOMEM);
> + }
> +
While it's certainly an ENOMEM, it might be better to raise the return
value from av_new_packet().
> + memcpy(pkt->data, text, text_len);
> +
> + if (id_len > 0) {
> + buf = av_packet_new_side_data(pkt,
> + AV_PKT_DATA_WEBVTT_IDENTIFIER,
> + id_len);
> + if (buf == NULL) {
> + av_free(pkt);
> + return AVERROR(ENOMEM);
> + }
> + memcpy(buf, id, id_len);
> + }
> +
> + if (settings_len > 0) {
> + buf = av_packet_new_side_data(pkt,
> + AV_PKT_DATA_WEBVTT_SETTINGS,
> + settings_len);
> + if (buf == NULL) {
> + av_free(pkt);
> + return AVERROR(ENOMEM);
> + }
> + memcpy(buf, settings, settings_len);
> + }
> +
> + // Do we need this for subtitles?
> + // pkt->flags = AV_PKT_FLAG_KEY;
> +
That's a good question
> + pkt->stream_index = st->index;
> + pkt->pts = timecode;
> +
> + // Do we need this for subtitles?
> + // pkt->dts = timecode;
> +
> + pkt->duration = duration;
> + pkt->pos = pos;
> +
> + dynarray_add(&matroska->packets, &matroska->num_packets, pkt);
> + matroska->prev_pkt = pkt;
> +
> + return 0;
> +}
> +
> static int matroska_parse_frame(MatroskaDemuxContext *matroska,
> MatroskaTrack *track,
> AVStream *st,
> @@ -2454,6 +2594,14 @@ static int matroska_parse_block(MatroskaDemuxContext *matroska, uint8_t *data,
> if (res)
> goto end;
>
> + } else if (st->codec->codec_id == AV_CODEC_ID_WEBVTT) {
> + res = matroska_parse_webvtt(matroska, track, st,
> + data, lace_size[n],
> + timecode, lace_duration,
> + pos);
> + if (res)
> + goto end;
> +
> } else {
> res = matroska_parse_frame(matroska, track, st, data, lace_size[n],
> timecode, lace_duration,
I'm not the mkv/webm maintainer, but it looks correct otherwise.
--
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/20130808/a3da3868/attachment.asc>
More information about the ffmpeg-devel
mailing list