[FFmpeg-devel] [PATCH] WebVTT demuxer - save cue id and settings as side data

[Clément Bœsch]

On Thu, May 30, 2013 at 04:37:56PM -0700, Matthew Heaney wrote:
> Currently the WebVTT demuxer parses the cues but throws away
> the cue id (the optional first line of the cue) and cue
> settings (the optional rendering instructions that follow
> the timestamp).
> 
> However, in order to write inband text tracks (to WebM
> files), the entire cue payload from the WebVTT source must
> be preserved.
> 
> This commit makes no change to the data part of the output
> buffer packet (where the actual cue text is stored), but
> does add the cue id and settings as a side data items, if
> they're present in the cue. Existing code that cares only
> about the data part of the packet can continue to ignore the
> side data.
> 
> There are two new packet data type flags,
> AV_PKT_DATA_WEBVTT_IDENTIFIER and
> AV_PKT_DATA_WEBVTT_SETTINGS.
> ---
>  libavcodec/avcodec.h    | 11 ++++++++++
>  libavcodec/version.h    |  2 +-
>  libavformat/webvttdec.c | 53 +++++++++++++++++++++++++++++++++++++++----------
>  3 files changed, 54 insertions(+), 12 deletions(-)
> 
> diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
> index 93d63e8..5ece48c 100644
> --- a/libavcodec/avcodec.h
> +++ b/libavcodec/avcodec.h
> @@ -1008,6 +1008,17 @@ enum AVPacketSideDataType {
>       * by data.
>       */
>      AV_PKT_DATA_MATROSKA_BLOCKADDITIONAL,
> +
> +    /**
> +     * The optional first identifier line of a WebVTT cue.
> +     */
> +    AV_PKT_DATA_WEBVTT_IDENTIFIER,
> +
> +    /**
> +     * The optional settings (rendering instructions) that immediately
> +     * follow the timestamp specifier of a WebVTT cue.
> +     */
> +    AV_PKT_DATA_WEBVTT_SETTINGS,
>  };
>  
>  /**
> diff --git a/libavcodec/version.h b/libavcodec/version.h
> index b904048..4eab7a8 100644
> --- a/libavcodec/version.h
> +++ b/libavcodec/version.h
> @@ -29,7 +29,7 @@
>  #include "libavutil/avutil.h"
>  
>  #define LIBAVCODEC_VERSION_MAJOR 55
> -#define LIBAVCODEC_VERSION_MINOR  12
> +#define LIBAVCODEC_VERSION_MINOR  13
>  #define LIBAVCODEC_VERSION_MICRO 102
>  
>  #define LIBAVCODEC_VERSION_INT  AV_VERSION_INT(LIBAVCODEC_VERSION_MAJOR, \
> diff --git a/libavformat/webvttdec.c b/libavformat/webvttdec.c
> index 694a020..894a762 100644
> --- a/libavformat/webvttdec.c
> +++ b/libavformat/webvttdec.c
> @@ -74,8 +74,8 @@ static int webvtt_read_header(AVFormatContext *s)
>          int i;
>          int64_t pos;
>          AVPacket *sub;
> -        const char *p, *identifier;
> -        //const char *settings = NULL;
> +        const char *p, *identifier, *settings;
> +        int identifier_len, settings_len;
>          int64_t ts_start, ts_end;
>  
>          ff_subtitles_read_chunk(s->pb, &cue);
> @@ -92,15 +92,23 @@ static int webvtt_read_header(AVFormatContext *s)
>              continue;
>  
>          /* optional cue identifier (can be a number like in SRT or some kind of
> -         * chaptering id), silently skip it */
> -        for (i = 0; p[i] && p[i] != '\n'; i++) {
> +         * chaptering id) */
> +        for (i = 0; p[i] && p[i] != '\n' && p[i] != '\r'; i++) {
>              if (!strncmp(p + i, "-->", 3)) {
>                  identifier = NULL;
>                  break;
>              }
>          }
> -        if (identifier)
> -            p += strcspn(p, "\n");
> +        if (!identifier)
> +            identifier_len = 0;
> +        else {
> +            identifier_len = strcspn(p, "\r\n");
> +            p += identifier_len;
> +            if (*p == '\r')
> +                p++;
> +            if (*p == '\n')
> +                p++;
> +        }
>  
>          /* cue timestamps */
>          if ((ts_start = read_ts(p)) == AV_NOPTS_VALUE)
> @@ -112,14 +120,15 @@ static int webvtt_read_header(AVFormatContext *s)
>          if ((ts_end = read_ts(p)) == AV_NOPTS_VALUE)
>              break;
>  
> -        /* optional cue settings, TODO: store in side_data */
> +        /* optional cue settings */
>          p += strcspn(p, "\n\t ");
>          while (*p == '\t' || *p == ' ')
>              p++;
> -        if (*p != '\n') {
> -            //settings = p;
> -            p += strcspn(p, "\n");
> -        }
> +        settings = p;
> +        settings_len = strcspn(p, "\r\n");
> +        p += settings_len;
> +        if (*p == '\r')
> +            p++;
>          if (*p == '\n')
>              p++;
>  
> @@ -132,6 +141,28 @@ static int webvtt_read_header(AVFormatContext *s)
>          sub->pos = pos;
>          sub->pts = ts_start;
>          sub->duration = ts_end - ts_start;
> +
> +        if (identifier_len) {
> +            uint8_t *buf = av_packet_new_side_data(sub,
> +                                                   AV_PKT_DATA_WEBVTT_IDENTIFIER,
> +                                                   identifier_len);
> +            if (!buf) {
> +                res = AVERROR(ENOMEM);
> +                goto end;
> +            }
> +            memcpy(buf, identifier, identifier_len);
> +        }
> +
> +        if (settings_len) {
> +            uint8_t *buf = av_packet_new_side_data(sub,
> +                                                   AV_PKT_DATA_WEBVTT_SETTINGS,
> +                                                   settings_len);
> +            if (!buf) {
> +                res = AVERROR(ENOMEM);
> +                goto end;
> +            }
> +            memcpy(buf, settings, settings_len);
> +        }
>      }
>  
>      ff_subtitles_queue_finalize(&webvtt->q);

LGTM.

I'll test and apply in the next 24 hours unless someone object.

Note: I'm assuming you at least tested fate-sub-webvtt

-- 
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/20130531/a825062b/attachment.asc>