[FFmpeg-devel] [PATCH 1/8] lavf: add internal demuxer helpers for subtitles.
Nicolas George
nicolas.george at normalesup.org
Sun Jun 17 14:29:56 CEST 2012
Le decadi 30 prairial, an CCXX, Clément Bœsch a écrit :
> From 4aad37987bcc6248555462d47264a097e4e1063e Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?Cl=C3=A9ment=20B=C5=93sch?= <ubitux at gmail.com>
> Date: Fri, 15 Jun 2012 18:54:52 +0200
> Subject: [PATCH 1/9] lavf: add internal demuxer helpers for subtitles.
>
> ---
> libavformat/Makefile | 1 +
> libavformat/subtitles.c | 107 +++++++++++++++++++++++++++++++++++++++++++++++
> libavformat/subtitles.h | 67 +++++++++++++++++++++++++++++
> 3 files changed, 175 insertions(+)
> create mode 100644 libavformat/subtitles.c
> create mode 100644 libavformat/subtitles.h
>
> diff --git a/libavformat/Makefile b/libavformat/Makefile
> index 5e4f002..d53a3e0 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..5e3cdee
> --- /dev/null
> +++ b/libavformat/subtitles.c
> @@ -0,0 +1,107 @@
> +/*
> + * 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"
> +
> +FFDemuxSubEntry *ff_subtitles_queue_insert_event(FFDemuxSubtitlesQueue *q,
> + const uint8_t *event, int len,
> + int merge)
> +{
> + FFDemuxSubEntry *subs, *sub;
> +
> + if (merge && q->nsub > 0) {
> + /* merge with previous event */
> +
> + uint8_t *tmp;
> + sub = &q->subs[q->nsub - 1];
> + tmp = av_realloc(sub->event, sub->len + len + 1);
> + if (!tmp)
> + return NULL;
> + sub->event = tmp;
> + memcpy(sub->event + sub->len, event, len);
> + sub->len += len;
> + } else {
> + /* new event */
> +
> + subs = av_fast_realloc(q->subs, &q->allocated_size,
> + (q->nsub + 1) * sizeof(*q->subs));
Beware of integer overflows.
> + if (!subs)
> + return NULL;
> + q->subs = subs;
> + sub = &subs[q->nsub++];
> + memset(sub, 0, sizeof(*sub));
> + sub->event = av_malloc(len + 1);
> + sub->len = len;
> + if (!sub->event)
> + return NULL;
> + memcpy(sub->event, event, len);
> + }
> + sub->event[sub->len] = 0;
> + return sub;
> +}
It looks like the merge and !merge branches are really two completely
different functions.
Also, the "&& q->nsub > 0" looks like defensive programming;
av_assert0(q->nsub > 0) would look saner to me.
> +
> +static int cmp_pkt_sub(const void *a, const void *b)
> +{
> + const int64_t t1 = ((const FFDemuxSubEntry *)a)->start;
> + const int64_t t2 = ((const FFDemuxSubEntry *)b)->start;
> + if (t1 == t2)
> + return 0;
> + return t1 > t2 ? 1 : -1;
> +}
The qsort function from the libc is not guaranteed to be stable. In
subtitles, the order of arrival of simultaneous events may be relevant. A
secondary compare on the pos field should be enough.
> +
> +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].start - q->subs[i].start;
> +}
> +
> +int ff_subtitles_queue_read_packet(FFDemuxSubtitlesQueue *q,
> + AVPacket *pkt, int i)
> +{
> + int res;
> + FFDemuxSubEntry *sub = q->subs + i;
> +
> + if (i == q->nsub)
> + return AVERROR_EOF;
> + res = av_new_packet(pkt, sub->len + 1);
> + if (res)
> + return res;
> + memcpy(pkt->data, sub->event, sub->len + 1);
> + pkt->flags |= AV_PKT_FLAG_KEY;
> + pkt->pos = sub->pos;
> + pkt->pts = pkt->dts = sub->start;
> + pkt->duration = sub->duration;
> + return 0;
> +}
> +
> +void ff_subtitles_queue_free(FFDemuxSubtitlesQueue *q)
> +{
> + int i;
> +
> + for (i = 0; i < q->nsub; i++)
> + av_freep(&q->subs[i].event);
> + av_freep(&q->subs);
> + q->nsub = q->allocated_size = 0;
> +}
> diff --git a/libavformat/subtitles.h b/libavformat/subtitles.h
> new file mode 100644
> index 0000000..85edbcd
> --- /dev/null
> +++ b/libavformat/subtitles.h
> @@ -0,0 +1,67 @@
> +/*
> + * 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
> +
> +typedef struct {
> + uint8_t *event; ///< allocated subtitle line, zero terminated
> + int len; ///< subtitle event length
> + int64_t pos; ///< offset position
> + int64_t start; ///< start time (PTS)
> + int duration; ///< event duration
> +} FFDemuxSubEntry;
It looks like you did not include some space for side data. As this is an
internal API, it can be added later though.
> +
> +typedef struct {
> + FFDemuxSubEntry *subs; ///< array of subtitles events
> + int nsub; ///< number of events
> + int allocated_size; ///< allocated size for subs
> +} FFDemuxSubtitlesQueue;
> +
> +/**
> + * Insert a new subtitle event
> + *
> + * @param event the subtitle line, may not be zero terminated
> + * @param len the length of the event
> + * @param merge set to 1 if the current event should be concatenated with the
> + * previous one instead of adding a new entry, 0 otherwise
> + */
> +FFDemuxSubEntry *ff_subtitles_queue_insert_event(FFDemuxSubtitlesQueue *q,
> + const uint8_t *event, int len,
> + int merge);
Do you insist on such a long name for all the functions? It makes the code
really hard to indent, especially if one wants to keep the 80-chars limit.
What about ff_subevents_append? It is not really a queue anyway.
> +
> +/**
> + * Set missing durations and sort subtitles by PTS
> + */
> +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, int i);
> +
> +/**
> + * Remove and free all the events
> + */
> +void ff_subtitles_queue_free(FFDemuxSubtitlesQueue *q);
> +
> +#endif /* AVFORMAT_SUBTITLES_H */
Regards,
--
Nicolas George
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20120617/18086aa3/attachment.asc>
More information about the ffmpeg-devel
mailing list