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

Clément Bœsch ubitux at gmail.com
Sun Jun 17 00:25:20 CEST 2012


On Sat, Jun 16, 2012 at 07:29:11PM +0200, Michael Niedermayer wrote:
> On Fri, Jun 15, 2012 at 07:29:08PM +0200, Clément Bœsch wrote:
> > ---
> [...]
> > +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_realloc_f(q->subs, q->nsub + 1, sizeof(*q->subs));
> 
> av_fast_realloc() should be faster
> 

Changed.

> 
> > +        if (!subs)
> > +            return NULL;
> > +        q->subs = subs;
> > +        sub = &subs[q->nsub++];
> > +        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;
> > +}
> > +
> 
> > +static int cmp_pkt_sub(const void *a, const void *b)
> > +{
> > +    return ((const FFDemuxSubEntry*)a)->start - ((const FFDemuxSubEntry*)b)->start;
> > +}
> 
> could be writen like:
> static int cmp_pkt_sub(const FFDemuxSubEntry *a, const FFDemuxSubEntry *b)
> {
>     return a->start - b->start;
> }
> 
> but would then need a cast of some sort when used to avoid a warning
> 

Yes I wanted to avoid the cast in the caller. Anyway, I changed this
function to avoid the possible overflow (start is int64_t).

> 
> > +
> > +void ff_subtitles_queue_finalize(FFDemuxSubtitlesQueue *q)
> > +{
> > +    int i;
> > +
> > +    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;
> > +    qsort(q->subs, q->nsub, sizeof(*q->subs), cmp_pkt_sub);
> 
> shouldnt the duration fill in be after qsort ?

Yeah, I moved the qsort() before; this could be made configurable at some
point depending on the format.

> also it might make sense to check if things are ordered before
> doing a qsort in case the array is scaned fully
> 

Since I moved the qsort() before I can't do that anymore. I thought about
adding a flag in the insert function when notifying a different PTS, but
that function doesn't take a PTS (since it can only be known later
sometimes). qsort() should be fast enough if the array is ordered
anyway... Also, it's only at init time.

> 
> > +}
> > +
> > +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);
> > +    pkt->data[sub->len] = 0;
> > +    pkt->flags |= AV_PKT_FLAG_KEY;
> > +    pkt->pos = sub->pos;
> > +    pkt->pts = pkt->dts = sub->start;
> > +    pkt->duration = sub->duration;
> 
> this would be simpler if each FFDemuxSubEntry contained a AVPacket
> and used its fields where applicable

Doing this will require a bigger allocated queue (AVPacket is bigger), and
I think it's important to see which fields can be accessed and modified.
The heavy part in creating the AVPacket is the data allocation, and it
will be needed anyway because a same sub event can be raised several times
(with a seek).

> or maybe plain AVPackets would be sufficient and FFDemuxSubEntry
> wouldnt be needed but that may be too unflexible dunno
> 

No you can't do that easily because of the reason above.

> 
> [...]
> > +/**
> > + * Insert a new subtitle event
> > + */
> > +FFDemuxSubEntry *ff_subtitles_queue_insert_event(FFDemuxSubtitlesQueue *q,
> > +                                                 const uint8_t *event, int len,
> > +                                                 int merge);
> 
> the parameters should be documented, from just reading this i wouldnt
> know what "merge" does
> 

Added some documentation.

I also fixed a few other things, like memset'ing to 0 the new sub entry in
the insert_event function, zero terminating the events, and fixing some
doxy.

-- 
Clément B.
-------------- next part --------------
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));
+        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;
+}
+
+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;
+}
+
+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;
+
+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);
+
+/**
+ * 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 */
-- 
1.7.10.4

-------------- 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/20120617/1f6da6ad/attachment.asc>


More information about the ffmpeg-devel mailing list