Browse code

avformat/vobsub: fix several issues.

Here is an extract of fate-samples/sub/vobsub.idx, with an additional
text at the end of each line to better identify each bitmap:

timestamp: 00:04:55:445, filepos: 00001b000 Ace!
timestamp: 00:05:00:049, filepos: 00001b800 Wake up, honey!
timestamp: 00:05:02:018, filepos: 00001c800 I gotta go to work.
timestamp: 00:05:02:035, filepos: 00001d000 <???>
timestamp: 00:05:04:203, filepos: 00001d800 Look after Clayton, okay?
timestamp: 00:05:05:947, filepos: 00001e800 I'll be back tonight.
timestamp: 00:05:07:957, filepos: 00001f800 Bye! Love you.
timestamp: 00:05:21:295, filepos: 000020800 Hey, Ace! What's up?
timestamp: 00:05:23:356, filepos: 000021800 Hey, how's it going?
timestamp: 00:05:24:640, filepos: 000022800 Remember what today is? The 3rd!
timestamp: 00:05:27:193, filepos: 000023800 Look over there!
timestamp: 00:05:28:369, filepos: 000024800 Where are they going?
timestamp: 00:05:28:361, filepos: 000025000 <???>
timestamp: 00:05:29:946, filepos: 000025800 Let's go see.
timestamp: 00:05:31:230, filepos: 000026000 I can't, man. I got Clayton.

Note the two "<???>": they are basically split subtitles (with the
previous one), which the dvdsub decoder is now supposed to reconstruct
with a previous commit. But also note that while the first chunk has
increasing timestamps,

timestamp: 00:05:02:018, filepos: 00001c800
timestamp: 00:05:02:035, filepos: 00001d000

...it's not the case of the second one (and this is not an exception in the
original file):

timestamp: 00:05:28:369, filepos: 000024800
timestamp: 00:05:28:361, filepos: 000025000

For the dvdsub decoder, they need to be "filepos'ed" ordered, but the
FFDemuxSubtitlesQueue is timestamps ordered, which is the reason of the
introduction of a sub sort method in the context, to allow giving
priority to the position, and then the timestamps. With that change, the
dvdsub decoder get fed with ordered packets.

Now the packet size estimation was also broken: the filepos differences
in the vobsub index defines the full data read between two subtitles
chunks, and it is necessary to take into account what is read by the
mpegps_read_pes_header() function since the length returned by that
function doesn't count the size of the data it reads. This is fixed with
the introduction of total_read, and {old,new}_pos. By doing this change,
we can drop the unreliable len16 heuristic and simplify the whole loop.
Note that mpegps_read_pes_header() often read more than one PES packet
(typically in one call it can read 0x1ba and 0x1be chunk along with the
relevant 0x1bd packet), which triggers the "total_read + pkt_size >
psize" check. This is an expected behaviour, which could be avoided by
having a more chunked version of mpegps_read_pes_header().

The latest change is the extraction of each stream into its own
subtitles queue. If we don't do this, the maximum size for a subtitle
chunk is broken, and the previous changes can not work. Having each
stream in a different queue requires some little adjustments in the
seek code of the demuxer.

This commit is only meaningful as a whole change and can not be easily
split. The FATE test changes because it uses the vobsub demuxer.

Clément Bœsch authored on 2013/09/30 05:05:14
Showing 4 changed files
... ...
@@ -113,7 +113,7 @@ typedef struct MpegDemuxContext {
113 113
     int imkh_cctv;
114 114
 #if CONFIG_VOBSUB_DEMUXER
115 115
     AVFormatContext *sub_ctx;
116
-    FFDemuxSubtitlesQueue q;
116
+    FFDemuxSubtitlesQueue q[32];
117 117
 #endif
118 118
 } MpegDemuxContext;
119 119
 
... ...
@@ -693,6 +693,12 @@ static int vobsub_read_header(AVFormatContext *s)
693 693
                 stream_id = 0;
694 694
             }
695 695
 
696
+            if (stream_id >= FF_ARRAY_ELEMS(vobsub->q)) {
697
+                av_log(s, AV_LOG_ERROR, "Maximum number of subtitles streams reached\n");
698
+                ret = AVERROR(EINVAL);
699
+                goto end;
700
+            }
701
+
696 702
             st = avformat_new_stream(s, NULL);
697 703
             if (!st) {
698 704
                 ret = AVERROR(ENOMEM);
... ...
@@ -712,6 +718,12 @@ static int vobsub_read_header(AVFormatContext *s)
712 712
             int64_t pos, timestamp;
713 713
             const char *p = line + 10;
714 714
 
715
+            if (!s->nb_streams) {
716
+                av_log(s, AV_LOG_ERROR, "Timestamp declared before any stream\n");
717
+                ret = AVERROR_INVALIDDATA;
718
+                goto end;
719
+            }
720
+
715 721
             if (sscanf(p, "%02d:%02d:%02d:%03d, filepos: %"SCNx64,
716 722
                        &hh, &mm, &ss, &ms, &pos) != 5) {
717 723
                 av_log(s, AV_LOG_ERROR, "Unable to parse timestamp line '%s', "
... ...
@@ -721,7 +733,7 @@ static int vobsub_read_header(AVFormatContext *s)
721 721
             timestamp = (hh*3600LL + mm*60LL + ss) * 1000LL + ms + delay;
722 722
             timestamp = av_rescale_q(timestamp, (AVRational){1,1000}, st->time_base);
723 723
 
724
-            sub = ff_subtitles_queue_insert(&vobsub->q, "", 0, 0);
724
+            sub = ff_subtitles_queue_insert(&vobsub->q[s->nb_streams - 1], "", 0, 0);
725 725
             if (!sub) {
726 726
                 ret = AVERROR(ENOMEM);
727 727
                 goto end;
... ...
@@ -767,7 +779,10 @@ static int vobsub_read_header(AVFormatContext *s)
767 767
     if (langidx < s->nb_streams)
768 768
         s->streams[langidx]->disposition |= AV_DISPOSITION_DEFAULT;
769 769
 
770
-    ff_subtitles_queue_finalize(&vobsub->q);
770
+    for (i = 0; i < s->nb_streams; i++) {
771
+        vobsub->q[i].sort = SUB_SORT_POS_TS;
772
+        ff_subtitles_queue_finalize(&vobsub->q[i]);
773
+    }
771 774
 
772 775
     if (!av_bprint_is_complete(&header)) {
773 776
         av_bprint_finalize(&header, NULL);
... ...
@@ -792,11 +807,22 @@ end:
792 792
 static int vobsub_read_packet(AVFormatContext *s, AVPacket *pkt)
793 793
 {
794 794
     MpegDemuxContext *vobsub = s->priv_data;
795
-    FFDemuxSubtitlesQueue *q = &vobsub->q;
795
+    FFDemuxSubtitlesQueue *q;
796 796
     AVIOContext *pb = vobsub->sub_ctx->pb;
797
-    int ret, psize, len16 = -1;
797
+    int ret, psize, total_read = 0, i;
798 798
     AVPacket idx_pkt;
799 799
 
800
+    int64_t min_ts = INT64_MAX;
801
+    int sid = 0;
802
+    for (i = 0; i < s->nb_streams; i++) {
803
+        FFDemuxSubtitlesQueue *tmpq = &vobsub->q[i];
804
+        int64_t ts = tmpq->subs[tmpq->current_sub_idx].pts;
805
+        if (ts < min_ts) {
806
+            min_ts = ts;
807
+            sid = i;
808
+        }
809
+    }
810
+    q = &vobsub->q[sid];
800 811
     ret = ff_subtitles_queue_read_packet(q, &idx_pkt);
801 812
     if (ret < 0)
802 813
         return ret;
... ...
@@ -819,19 +845,20 @@ static int vobsub_read_packet(AVFormatContext *s, AVPacket *pkt)
819 819
     do {
820 820
         int n, to_read, startcode;
821 821
         int64_t pts, dts;
822
+        int64_t old_pos = avio_tell(pb), new_pos;
823
+        int pkt_size;
822 824
 
823 825
         ret = mpegps_read_pes_header(vobsub->sub_ctx, NULL, &startcode, &pts, &dts);
824 826
         if (ret < 0)
825 827
             FAIL(ret);
826 828
         to_read = ret & 0xffff;
829
+        new_pos = avio_tell(pb);
830
+        pkt_size = ret + (new_pos - old_pos);
827 831
 
828 832
         /* this prevents reads above the current packet */
829
-        if (pkt->size + to_read > psize)
830
-            break;
831
-
832
-        /* if the len is computed, we check for overread */
833
-        if (len16 != -1 && pkt->size + to_read > len16)
833
+        if (total_read + pkt_size > psize)
834 834
             break;
835
+        total_read += pkt_size;
835 836
 
836 837
         /* the current chunk doesn't match the stream index (unlikely) */
837 838
         if ((startcode & 0x1f) != idx_pkt.stream_index)
... ...
@@ -844,11 +871,7 @@ static int vobsub_read_packet(AVFormatContext *s, AVPacket *pkt)
844 844
         n = avio_read(pb, pkt->data + (pkt->size - to_read), to_read);
845 845
         if (n < to_read)
846 846
             pkt->size -= to_read - n;
847
-
848
-        /* first chunk contains the total len of the packet to raise */
849
-        if (len16 == -1 && n > 2)
850
-            len16 = AV_RB16(pkt->data);
851
-    } while (len16 != -1 && pkt->size != len16);
847
+    } while (total_read < psize);
852 848
 
853 849
     pkt->pts = pkt->dts = idx_pkt.pts;
854 850
     pkt->pos = idx_pkt.pos;
... ...
@@ -858,6 +881,7 @@ static int vobsub_read_packet(AVFormatContext *s, AVPacket *pkt)
858 858
     return 0;
859 859
 
860 860
 fail:
861
+    av_free_packet(pkt);
861 862
     av_free_packet(&idx_pkt);
862 863
     return ret;
863 864
 }
... ...
@@ -871,6 +895,7 @@ static int vobsub_read_seek(AVFormatContext *s, int stream_index,
871 871
      * same for all subtitles stream within a .idx/.sub). Rescaling is done just
872 872
      * like in avformat_seek_file(). */
873 873
     if (stream_index == -1 && s->nb_streams != 1) {
874
+        int i, ret = 0;
874 875
         AVRational time_base = s->streams[0]->time_base;
875 876
         ts = av_rescale_q(ts, AV_TIME_BASE_Q, time_base);
876 877
         min_ts = av_rescale_rnd(min_ts, time_base.den,
... ...
@@ -879,16 +904,26 @@ static int vobsub_read_seek(AVFormatContext *s, int stream_index,
879 879
         max_ts = av_rescale_rnd(max_ts, time_base.den,
880 880
                                 time_base.num * (int64_t)AV_TIME_BASE,
881 881
                                 AV_ROUND_DOWN | AV_ROUND_PASS_MINMAX);
882
+        for (i = 0; i < s->nb_streams; i++) {
883
+            int r = ff_subtitles_queue_seek(&vobsub->q[i], s, stream_index,
884
+                                            min_ts, ts, max_ts, flags);
885
+            if (r < 0)
886
+                ret = r;
887
+        }
888
+        return ret;
882 889
     }
883 890
 
884
-    return ff_subtitles_queue_seek(&vobsub->q, s, stream_index,
891
+    return ff_subtitles_queue_seek(&vobsub->q[stream_index], s, stream_index,
885 892
                                    min_ts, ts, max_ts, flags);
886 893
 }
887 894
 
888 895
 static int vobsub_read_close(AVFormatContext *s)
889 896
 {
897
+    int i;
890 898
     MpegDemuxContext *vobsub = s->priv_data;
891
-    ff_subtitles_queue_clean(&vobsub->q);
899
+
900
+    for (i = 0; i < s->nb_streams; i++)
901
+        ff_subtitles_queue_clean(&vobsub->q[i]);
892 902
     if (vobsub->sub_ctx)
893 903
         avformat_close_input(&vobsub->sub_ctx);
894 904
     return 0;
... ...
@@ -57,7 +57,7 @@ AVPacket *ff_subtitles_queue_insert(FFDemuxSubtitlesQueue *q,
57 57
     return sub;
58 58
 }
59 59
 
60
-static int cmp_pkt_sub(const void *a, const void *b)
60
+static int cmp_pkt_sub_ts_pos(const void *a, const void *b)
61 61
 {
62 62
     const AVPacket *s1 = a;
63 63
     const AVPacket *s2 = b;
... ...
@@ -69,11 +69,25 @@ static int cmp_pkt_sub(const void *a, const void *b)
69 69
     return s1->pts > s2->pts ? 1 : -1;
70 70
 }
71 71
 
72
+static int cmp_pkt_sub_pos_ts(const void *a, const void *b)
73
+{
74
+    const AVPacket *s1 = a;
75
+    const AVPacket *s2 = b;
76
+    if (s1->pos == s2->pos) {
77
+        if (s1->pts == s2->pts)
78
+            return 0;
79
+        return s1->pts > s2->pts ? 1 : -1;
80
+    }
81
+    return s1->pos > s2->pos ? 1 : -1;
82
+}
83
+
72 84
 void ff_subtitles_queue_finalize(FFDemuxSubtitlesQueue *q)
73 85
 {
74 86
     int i;
75 87
 
76
-    qsort(q->subs, q->nb_subs, sizeof(*q->subs), cmp_pkt_sub);
88
+    qsort(q->subs, q->nb_subs, sizeof(*q->subs),
89
+          q->sort == SUB_SORT_TS_POS ? cmp_pkt_sub_ts_pos
90
+                                     : cmp_pkt_sub_pos_ts);
77 91
     for (i = 0; i < q->nb_subs; i++)
78 92
         if (q->subs[i].duration == -1 && i < q->nb_subs - 1)
79 93
             q->subs[i].duration = q->subs[i + 1].pts - q->subs[i].pts;
... ...
@@ -25,11 +25,17 @@
25 25
 #include "avformat.h"
26 26
 #include "libavutil/bprint.h"
27 27
 
28
+enum sub_sort {
29
+    SUB_SORT_TS_POS = 0,    ///< sort by timestamps, then position
30
+    SUB_SORT_POS_TS,        ///< sort by position, then timestamps
31
+};
32
+
28 33
 typedef struct {
29 34
     AVPacket *subs;         ///< array of subtitles packets
30 35
     int nb_subs;            ///< number of subtitles packets
31 36
     int allocated_size;     ///< allocated size for subs
32 37
     int current_sub_idx;    ///< current position for the read packet callback
38
+    enum sub_sort sort;     ///< sort method to use when finalizing subtitles
33 39
 } FFDemuxSubtitlesQueue;
34 40
 
35 41
 /**
... ...
@@ -54,7 +54,7 @@
54 54
 1,      15355,      15355,     4733,     2094, 0x3c171425, F=0x0
55 55
 1,      48797,      48797,     2560,     2480, 0x7c0edf21, F=0x0
56 56
 1,      51433,      51433,     2366,     3059, 0xc95b8a05, F=0x0
57
-1,      53919,      53919,     2696,     2095, 0x61bb15ed, F=0x0
57
+1,      53910,      53910,     2696,     2095, 0x61bb15ed, F=0x0
58 58
 1,      56663,      56663,     1262,     1013, 0xc9ae89b7, F=0x0
59 59
 1,      58014,      58014,     1661,      969, 0xe01878f0, F=0x0
60 60
 1,      67724,      67724,     1365,      844, 0xe7db4fc1, F=0x0
... ...
@@ -85,11 +85,10 @@
85 85
 1,     191356,     191356,     1228,     1517, 0xae8c5c2b, F=0x0
86 86
 1,     192640,     192640,     1763,     2506, 0xa458d6d4, F=0x0
87 87
 1,     195193,     195193,     1092,     1074, 0x397ba9a8, F=0x0
88
-1,     196369,     196369,     1524,     1715, 0x695ca41e, F=0x0
88
+1,     196361,     196361,     1524,     1715, 0x695ca41e, F=0x0
89 89
 1,     197946,     197946,     1160,      789, 0xc63a189e, F=0x0
90 90
 1,     199230,     199230,     1627,     1846, 0xeea8c599, F=0x0
91 91
 1,     200924,     200924,     1763,      922, 0xd4a87222, F=0x0
92 92
 1,     210600,     210600,     1831,      665, 0x55580135, F=0x0
93 93
 1,     214771,     214771,     1558,     1216, 0x50d1f6c5, F=0x0
94 94
 1,     225640,     225640,     2127,     2133, 0x670c11a5, F=0x0
95
-1,     227834,     227834,     1262,     1264, 0xc1d9fc57, F=0x0