[FFmpeg-devel] [PATCH 2/8] lavf/jacosubdec: use subtitles queue API.

Clément Bœsch ubitux at gmail.com
Sun Jun 17 19:50:08 CEST 2012


On Sun, Jun 17, 2012 at 03:29:06PM +0200, Nicolas George wrote:
[...]
> >  static int jacosub_read_packet(AVFormatContext *s, AVPacket *pkt)
> >  {
> > -    int res;
> >      JACOsubContext *jacosub = s->priv_data;
> > -    const SubEntry *sub = &jacosub->subs[jacosub->sid++];
> > -
> > -    if (jacosub->sid == jacosub->nsub)
> > -        return AVERROR_EOF;
> > -    res = av_new_packet(pkt, strlen(sub->line));
> > -    if (res)
> > -        return res;
> > -    strcpy(pkt->data, sub->line);
> > -    pkt->flags |= AV_PKT_FLAG_KEY;
> > -    pkt->pos = sub->pos;
> > -    pkt->pts = pkt->dts = sub->start;
> > -    pkt->duration = sub->end - sub->start;
> > -    return 0;
> > +    return ff_subtitles_queue_read_packet(&jacosub->q, pkt, jacosub->sid++);
> 
> This will cause an invalid access if called again after EOF, because
> jacosub->sid will already be beyond the end and
> ff_subtitles_queue_read_packet checks for equality. And even with a >=
> check, it could cause an integer overflow.
> 
> Maybe make the last argument of ff_subtitles_queue_read_packet a pointer and
> increment it only if the read succeeds.
> 

Fixed as stated in the other thread. See attached.

[...]

-- 
Clément B.
-------------- next part --------------
From 50b610fa2180971bd2c79eddca3c1a66d512cab8 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:57:39 +0200
Subject: [PATCH 02/10] lavf/jacosubdec: use subtitles queue API.

---
 libavformat/jacosubdec.c |   98 ++++++++++++----------------------------------
 1 file changed, 24 insertions(+), 74 deletions(-)

diff --git a/libavformat/jacosubdec.c b/libavformat/jacosubdec.c
index 67f4c37..afc7cc3 100644
--- a/libavformat/jacosubdec.c
+++ b/libavformat/jacosubdec.c
@@ -27,24 +27,16 @@
 
 #include "avformat.h"
 #include "internal.h"
+#include "subtitles.h"
 #include "libavcodec/jacosub.h"
 #include "libavutil/avstring.h"
 #include "libavutil/bprint.h"
 #include "libavutil/intreadwrite.h"
 
 typedef struct {
-    char *line;         ///< null-terminated heap allocated subtitle line
-    int64_t pos;        ///< offset position
-    int start;          ///< timestamp start
-    int end;            ///< timestamp end
-} SubEntry;
-
-typedef struct {
     int shift;
     unsigned timeres;
-    SubEntry *subs;     ///< subtitles list
-    int nsub;           ///< number of subtitles
-    int sid;            ///< current subtitle
+    FFDemuxSubtitlesQueue q;
 } JACOsubContext;
 
 static int timed_line(const char *ptr)
@@ -101,41 +93,39 @@ static int get_jss_cmd(char k)
 
 static int jacosub_read_close(AVFormatContext *s)
 {
-    int i;
     JACOsubContext *jacosub = s->priv_data;
-
-    for (i = 0; i < jacosub->nsub; i++)
-        av_freep(&jacosub->subs[i].line);
-    jacosub->nsub = 0;
-    av_freep(&jacosub->subs);
+    ff_subtitles_queue_free(&jacosub->q);
     return 0;
 }
 
 static const char *read_ts(JACOsubContext *jacosub, const char *buf,
-                           int *ts_start, int *ts_end)
+                           int64_t *start, int *duration)
 {
     int len;
     unsigned hs, ms, ss, fs; // hours, minutes, seconds, frame start
     unsigned he, me, se, fe; // hours, minutes, seconds, frame end
+    int ts_start, ts_end;
 
     /* timed format */
     if (sscanf(buf, "%u:%u:%u.%u %u:%u:%u.%u %n",
                &hs, &ms, &ss, &fs,
                &he, &me, &se, &fe, &len) == 8) {
-        *ts_start = (hs*3600 + ms*60 + ss) * jacosub->timeres + fs;
-        *ts_end   = (he*3600 + me*60 + se) * jacosub->timeres + fe;
+        ts_start = (hs*3600 + ms*60 + ss) * jacosub->timeres + fs;
+        ts_end   = (he*3600 + me*60 + se) * jacosub->timeres + fe;
         goto shift_and_ret;
     }
 
     /* timestamps format */
-    if (sscanf(buf, "@%u @%u %n", ts_start, ts_end, &len) == 2)
+    if (sscanf(buf, "@%u @%u %n", &ts_start, &ts_end, &len) == 2)
         goto shift_and_ret;
 
     return NULL;
 
 shift_and_ret:
-    *ts_start = (*ts_start + jacosub->shift) * 100 / jacosub->timeres;
-    *ts_end   = (*ts_end   + jacosub->shift) * 100 / jacosub->timeres;
+    ts_start  = (ts_start + jacosub->shift) * 100 / jacosub->timeres;
+    ts_end    = (ts_end   + jacosub->shift) * 100 / jacosub->timeres;
+    *start    = ts_start;
+    *duration = ts_start + ts_end;
     return buf + len;
 }
 
@@ -161,11 +151,6 @@ static int get_shift(int timeres, const char *buf)
     return 0;
 }
 
-static int cmp_timed_sub(const void *a, const void *b)
-{
-    return ((const SubEntry*)a)->start - ((const SubEntry*)b)->start;
-}
-
 static int jacosub_read_header(AVFormatContext *s)
 {
     AVBPrint header;
@@ -191,41 +176,19 @@ static int jacosub_read_header(AVFormatContext *s)
         int cmd_len;
         const char *p = line;
         int64_t pos = avio_tell(pb);
-
-        ff_get_line(pb, line, sizeof(line));
+        int len = ff_get_line(pb, line, sizeof(line));
 
         p = jss_skip_whitespace(p);
 
         /* queue timed line */
         if (merge_line || timed_line(p)) {
-            SubEntry *subs, *sub;
-            const int len = strlen(line);
-
-            if (merge_line) {
-                char *tmp;
-                const int old_len = strlen(sub->line);
-
-                sub = &subs[jacosub->nsub];
-                tmp = av_realloc(sub->line, old_len + len + 1);
-                if (!tmp)
-                    return AVERROR(ENOMEM);
-                sub->line = tmp;
-                strcpy(sub->line + old_len, line);
-            } else {
-                subs = av_realloc(jacosub->subs,
-                                  sizeof(*jacosub->subs) * (jacosub->nsub+1));
-                if (!subs)
-                    return AVERROR(ENOMEM);
-                jacosub->subs = subs;
-                sub = &subs[jacosub->nsub];
-                sub->pos  = pos;
-                sub->line = av_strdup(line);
-                if (!sub->line)
-                    return AVERROR(ENOMEM);
-            }
+            FFDemuxSubEntry *sub;
+
+            sub = ff_subtitles_queue_insert_event(&jacosub->q, line, len, merge_line);
+            if (!sub)
+                return AVERROR(ENOMEM);
+            sub->pos = pos;
             merge_line = len > 1 && !strcmp(&line[len - 2], "\\\n");
-            if (!merge_line)
-                jacosub->nsub++;
             continue;
         }
 
@@ -267,32 +230,19 @@ static int jacosub_read_header(AVFormatContext *s)
 
     /* SHIFT and TIMERES affect the whole script so packet timing can only be
      * done in a second pass */
-    for (i = 0; i < jacosub->nsub; i++) {
-        SubEntry *sub = &jacosub->subs[i];
-        read_ts(jacosub, sub->line, &sub->start, &sub->end);
+    for (i = 0; i < jacosub->q.nsub; i++) {
+        FFDemuxSubEntry *sub = &jacosub->q.subs[i];
+        read_ts(jacosub, sub->event, &sub->start, &sub->duration);
     }
-    qsort(jacosub->subs, jacosub->nsub, sizeof(*jacosub->subs), cmp_timed_sub);
+    ff_subtitles_queue_finalize(&jacosub->q);
 
     return 0;
 }
 
 static int jacosub_read_packet(AVFormatContext *s, AVPacket *pkt)
 {
-    int res;
     JACOsubContext *jacosub = s->priv_data;
-    const SubEntry *sub = &jacosub->subs[jacosub->sid++];
-
-    if (jacosub->sid == jacosub->nsub)
-        return AVERROR_EOF;
-    res = av_new_packet(pkt, strlen(sub->line));
-    if (res)
-        return res;
-    strcpy(pkt->data, sub->line);
-    pkt->flags |= AV_PKT_FLAG_KEY;
-    pkt->pos = sub->pos;
-    pkt->pts = pkt->dts = sub->start;
-    pkt->duration = sub->end - sub->start;
-    return 0;
+    return ff_subtitles_queue_read_packet(&jacosub->q, pkt);
 }
 
 AVInputFormat ff_jacosub_demuxer = {
-- 
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/77e30e4c/attachment.asc>


More information about the ffmpeg-devel mailing list