[FFmpeg-devel] [PATCH 02/11] lavf: add internal demuxer helpers for subtitles.

[Clément Bœsch]

On Sat, Jun 23, 2012 at 12:30 AM, Stefano Sabatini <stefasab at gmail.com> wrote:
> On date Friday 2012-06-22 22:43:57 +0200, Clément Bœsch encoded:
>> ---
>>  libavformat/Makefile    |   1 +
>>  libavformat/subtitles.c | 101 ++++++++++++++++++++++++++++++++++++++++++++++++
>>  libavformat/subtitles.h |  61 +++++++++++++++++++++++++++++
>>  3 files changed, 163 insertions(+)
>>  create mode 100644 libavformat/subtitles.c
>>  create mode 100644 libavformat/subtitles.h
>>
>> diff --git a/libavformat/Makefile b/libavformat/Makefile
>> index 9032654..0060296 100644
>> --- a/libavformat/Makefile
>> +++ b/libavformat/Makefile
>> @@ -19,6 +19,7 @@ OBJS = allformats.o         \
>>         riff.o               \
>>         sdp.o                \
>>         seek.o               \
>> +       subtitles.o          \
>>         utils.o              \
>>
>>  OBJS-$(CONFIG_NETWORK)                   += network.o
>> diff --git a/libavformat/subtitles.c b/libavformat/subtitles.c
>> new file mode 100644
>> index 0000000..ce6cc35
>> --- /dev/null
>> +++ b/libavformat/subtitles.c
>> @@ -0,0 +1,101 @@
>> +/*
>> + * Copyright (c) 2012 Clément Bœsch
>> + *
>> + * This file is part of FFmpeg.
>> + *
>> + * FFmpeg is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU Lesser General Public
>> + * License as published by the Free Software Foundation; either
>> + * version 2.1 of the License, or (at your option) any later version.
>> + *
>> + * FFmpeg is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>> + * Lesser General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU Lesser General Public
>> + * License along with FFmpeg; if not, write to the Free Software
>> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
>> + */
>> +
>> +#include "avformat.h"
>> +#include "subtitles.h"
>> +
>> +AVPacket *ff_subtitles_queue_insert(FFDemuxSubtitlesQueue *q,
>> +                                    const uint8_t *event, int len, int merge)
>> +{
>> +    AVPacket *subs, *sub;
>> +
>> +    if (merge && q->nsub > 0) {
>> +        /* merge with previous event */
>> +
>> +        int old_len;
>> +        sub = &q->subs[q->nsub - 1];
>> +        old_len = sub->size;
>> +        if (av_grow_packet(sub, len) < 0)
>> +            return NULL;
>> +        memcpy(sub->data + old_len, event, len);
>> +    } else {
>> +        /* new event */
>> +
>> +        if (q->nsub >= INT_MAX/sizeof(*q->subs) - 1)
>> +            return NULL;
>> +        subs = av_fast_realloc(q->subs, &q->allocated_size,
>> +                               (q->nsub + 1) * sizeof(*q->subs));
>> +        if (!subs)
>> +            return NULL;
>> +        q->subs = subs;
>> +        sub = &subs[q->nsub++];
>> +        if (av_new_packet(sub, len) < 0)
>> +            return NULL;
>> +        sub->destruct = NULL;
>> +        sub->flags |= AV_PKT_FLAG_KEY;
>> +        sub->pts = sub->dts = 0;
>> +        memcpy(sub->data, event, len);
>> +    }
>> +    return sub;
>> +}
>> +
>> +static int cmp_pkt_sub(const void *a, const void *b)
>> +{
>> +    const AVPacket *s1 = a;
>> +    const AVPacket *s2 = b;
>> +    if (s1->pts == s2->pts) {
>> +        if (s1->pos == s2->pos)
>> +            return 0;
>> +        return s1->pos > s2->pos ? 1 : -1;
>> +    }
>> +    return s1->pts > s2->pts ? 1 : -1;
>> +}
>> +
>> +void ff_subtitles_queue_finalize(FFDemuxSubtitlesQueue *q)
>> +{
>> +    int i;
>> +
>> +    qsort(q->subs, q->nsub, sizeof(*q->subs), cmp_pkt_sub);
>> +    for (i = 0; i < q->nsub; i++)
>> +        if (q->subs[i].duration == -1 && i < q->nsub - 1)
>> +            q->subs[i].duration = q->subs[i + 1].pts - q->subs[i].pts;
>> +}
>> +
>> +int ff_subtitles_queue_read_packet(FFDemuxSubtitlesQueue *q, AVPacket *pkt)
>> +{
>> +    AVPacket *sub = q->subs + q->current_sub;
>> +
>
>> +    if (q->current_sub == q->nsub)
>> +        return AVERROR_EOF;
>
> I'd consider safer to set q to NULL in that case, rather than point to
> foo data.
>

I don't understand what you mean; if you mean making current_sub
pointing on something valid, I can't even set it to 0 (since there
might not be any subtitles at all. Also, I don't want to automatically
destroy q since we may want to seek after that.

>> +    *pkt = *sub;
>> +    pkt->dts = pkt->pts;
>> +    q->current_sub++;
>> +    return 0;
>> +}
>> +
>> +void ff_subtitles_queue_free(FFDemuxSubtitlesQueue *q)
>> +{
>> +    int i;
>> +
>> +    for (i = 0; i < q->nsub; i++)
>> +        av_destruct_packet(&q->subs[i]);
>> +    av_freep(&q->subs);
>> +    q->nsub = q->allocated_size = q->current_sub = 0;
>> +}
>> diff --git a/libavformat/subtitles.h b/libavformat/subtitles.h
>> new file mode 100644
>> index 0000000..3373027
>> --- /dev/null
>> +++ b/libavformat/subtitles.h
>> @@ -0,0 +1,61 @@
>> +/*
>> + * Copyright (c) 2012 Clément Bœsch
>> + *
>> + * This file is part of FFmpeg.
>> + *
>> + * FFmpeg is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU Lesser General Public
>> + * License as published by the Free Software Foundation; either
>> + * version 2.1 of the License, or (at your option) any later version.
>> + *
>> + * FFmpeg is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>> + * Lesser General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU Lesser General Public
>> + * License along with FFmpeg; if not, write to the Free Software
>> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
>> + */
>> +
>> +#ifndef AVFORMAT_SUBTITLES_H
>> +#define AVFORMAT_SUBTITLES_H
>> +
>> +#include <stdint.h>
>> +#include "avformat.h"
>> +
>> +typedef struct {
>> +    AVPacket *subs;         ///< array of subtitles packets
>> +    int nsub;               ///< number of subtitles packets
>
> nit++: nb_subs?
>

ok, renamed.

>> +    int allocated_size;     ///< allocated size for subs
>> +    int current_sub;        ///< current position for the read packet callback
>
> nit++: current_sub_idx?
>

whatever, renamed.

>> +} FFDemuxSubtitlesQueue;
>> +
>
>> +/**
>> + * Insert a new subtitle event
>
> Nit: here and below, missing terminating dots.
>

Added here and below.

>> + *
>> + * @param event the subtitle line, may not be zero terminated
>
>> + * @param len   the length of the event
>
> length is always a bit ambiguous when dealing with string (it is a
> buffer size or a length in the sense of strlen()?).
>

Added "(in strlen() sense, so without '\0')"

>> + * @param merge set to 1 if the current event should be concatenated with the
>> + *              previous one instead of adding a new entry, 0 otherwise
>> + */
>> +AVPacket *ff_subtitles_queue_insert(FFDemuxSubtitlesQueue *q,
>> +                                    const uint8_t *event, int len, int merge);
>> +
>> +/**
>> + * Set missing durations and sort subtitles by PTS, and then byte position
>> + */
>> +void ff_subtitles_queue_finalize(FFDemuxSubtitlesQueue *q);
>> +
>> +/**
>> + * Generic read_packet() callback for subtitles demuxers using this queue
>> + * system
>> + */
>> +int ff_subtitles_queue_read_packet(FFDemuxSubtitlesQueue *q, AVPacket *pkt);
>> +
>
>> +/**
>> + * Remove and destroy all the subtitles packets
>> + */
>> +void ff_subtitles_queue_free(FFDemuxSubtitlesQueue *q);
>
> This is slightly confusing, since the queue structure itself is not
> freed (maybe "clean" may be less ambiguous).
>

Renamed.

[...]

-- 
Clément B.