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

[Matthew Heaney]

Oh, I was just trying to conserve flag bits.  If there's budget for
WEBVTT flags, then no problem to use separate side data items for id
and settings.

I'll push up a revised patch this afternoon (PST).

Thanks,
Matt

On Thu, May 30, 2013 at 3:47 AM, Clément Bœsch <ubitux at gmail.com> wrote:
> On Wed, May 29, 2013 at 04:26:13PM -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 change 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 single new side data
>> item.  Existing code that cares only about the data part of
>> the packet can continue to ignore the side data.
>> ---
>>  libavformat/webvttdec.c | 32 ++++++++++++++++++++++----------
>>  1 file changed, 22 insertions(+), 10 deletions(-)
>>
>> diff --git a/libavformat/webvttdec.c b/libavformat/webvttdec.c
>> index 694a020..a169d11 100644
>> --- a/libavformat/webvttdec.c
>> +++ b/libavformat/webvttdec.c
>> @@ -74,8 +74,9 @@ 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 len, identifier_len, settings_len;
>> +        uint8_t* buf;
>>          int64_t ts_start, ts_end;
>>
>>          ff_subtitles_read_chunk(s->pb, &cue);
>> @@ -92,15 +93,15 @@ 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 */
>> +         * chaptering id) */
>>          for (i = 0; p[i] && p[i] != '\n'; i++) {
>>              if (!strncmp(p + i, "-->", 3)) {
>>                  identifier = NULL;
>>                  break;
>>              }
>>          }
>> -        if (identifier)
>> -            p += strcspn(p, "\n");
>> +        identifier_len = identifier ? strcspn(p, "\n") : 0;
>> +        p += identifier_len;
>>
>>          /* cue timestamps */
>>          if ((ts_start = read_ts(p)) == AV_NOPTS_VALUE)
>> @@ -112,16 +113,18 @@ 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++;
>> +        settings = p;
>> +        settings_len = strcspn(p, "\n");
>> +        p += settings_len;
>>          if (*p != '\n') {
>> -            //settings = p;
>> -            p += strcspn(p, "\n");
>> +            av_log(s, AV_LOG_ERROR, "Bad WebVTT cue.\n");
>> +            return AVERROR(EINVAL);
>>          }
>> -        if (*p == '\n')
>> -            p++;
>> +        p++;
>>
>>          /* create packet */
>>          sub = ff_subtitles_queue_insert(&webvtt->q, p, strlen(p), 0);
>> @@ -132,6 +135,15 @@ static int webvtt_read_header(AVFormatContext *s)
>>          sub->pos = pos;
>>          sub->pts = ts_start;
>>          sub->duration = ts_end - ts_start;
>> +
>> +        len = identifier_len + 1 + settings_len + 1;
>> +        buf = av_packet_new_side_data(sub, AV_PKT_DATA_NEW_EXTRADATA, len);
>> +        memcpy(buf, identifier, identifier_len);
>> +        buf += identifier_len;
>> +        *buf++ = '\n';
>> +        memcpy(buf, settings, settings_len);
>> +        buf += settings_len;
>> +        *buf++ = '\n';
>
> Exploiting this might be troublesome: won't you need to split this buffer
> again in the webm muxer (and any potential other muxer such as... webvtt)?
>
> While that may sound overkill, I believe the correct solution is to
> introduce AV_PKT_DATA_WEBVTT_{IDENTIFIER,SETTINGS} side data types.
>
> Those '\n' are likely unwanted too, BTW (of course the middle one looks
> necessary for splitting).
>
> --
> Clément B.
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>