[FFmpeg-cvslog] avformat/vobsub: fix several issues.
Clément Bœsch
git at videolan.org
Fri Oct 4 08:02:50 CEST 2013
ffmpeg | branch: master | Clément Bœsch <u at pkh.me> | Sun Sep 29 22:05:14 2013 +0200| [dbfe61100bd7c9c9afb34b35a7e2cbf28807062d] | committer: Clément Bœsch
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.
> http://git.videolan.org/gitweb.cgi/ffmpeg.git/?a=commit;h=dbfe61100bd7c9c9afb34b35a7e2cbf28807062d
---
libavformat/mpeg.c | 69 ++++++++++++++++++++++++++++++++++------------
libavformat/subtitles.c | 18 ++++++++++--
libavformat/subtitles.h | 6 ++++
tests/ref/fate/sub2video | 5 ++--
4 files changed, 76 insertions(+), 22 deletions(-)
diff --git a/libavformat/mpeg.c b/libavformat/mpeg.c
index 0b4fc23..f59cc58 100644
--- a/libavformat/mpeg.c
+++ b/libavformat/mpeg.c
@@ -113,7 +113,7 @@ typedef struct MpegDemuxContext {
int imkh_cctv;
#if CONFIG_VOBSUB_DEMUXER
AVFormatContext *sub_ctx;
- FFDemuxSubtitlesQueue q;
+ FFDemuxSubtitlesQueue q[32];
#endif
} MpegDemuxContext;
@@ -693,6 +693,12 @@ static int vobsub_read_header(AVFormatContext *s)
stream_id = 0;
}
+ if (stream_id >= FF_ARRAY_ELEMS(vobsub->q)) {
+ av_log(s, AV_LOG_ERROR, "Maximum number of subtitles streams reached\n");
+ ret = AVERROR(EINVAL);
+ goto end;
+ }
+
st = avformat_new_stream(s, NULL);
if (!st) {
ret = AVERROR(ENOMEM);
@@ -712,6 +718,12 @@ static int vobsub_read_header(AVFormatContext *s)
int64_t pos, timestamp;
const char *p = line + 10;
+ if (!s->nb_streams) {
+ av_log(s, AV_LOG_ERROR, "Timestamp declared before any stream\n");
+ ret = AVERROR_INVALIDDATA;
+ goto end;
+ }
+
if (sscanf(p, "%02d:%02d:%02d:%03d, filepos: %"SCNx64,
&hh, &mm, &ss, &ms, &pos) != 5) {
av_log(s, AV_LOG_ERROR, "Unable to parse timestamp line '%s', "
@@ -721,7 +733,7 @@ static int vobsub_read_header(AVFormatContext *s)
timestamp = (hh*3600LL + mm*60LL + ss) * 1000LL + ms + delay;
timestamp = av_rescale_q(timestamp, (AVRational){1,1000}, st->time_base);
- sub = ff_subtitles_queue_insert(&vobsub->q, "", 0, 0);
+ sub = ff_subtitles_queue_insert(&vobsub->q[s->nb_streams - 1], "", 0, 0);
if (!sub) {
ret = AVERROR(ENOMEM);
goto end;
@@ -767,7 +779,10 @@ static int vobsub_read_header(AVFormatContext *s)
if (langidx < s->nb_streams)
s->streams[langidx]->disposition |= AV_DISPOSITION_DEFAULT;
- ff_subtitles_queue_finalize(&vobsub->q);
+ for (i = 0; i < s->nb_streams; i++) {
+ vobsub->q[i].sort = SUB_SORT_POS_TS;
+ ff_subtitles_queue_finalize(&vobsub->q[i]);
+ }
if (!av_bprint_is_complete(&header)) {
av_bprint_finalize(&header, NULL);
@@ -792,11 +807,22 @@ end:
static int vobsub_read_packet(AVFormatContext *s, AVPacket *pkt)
{
MpegDemuxContext *vobsub = s->priv_data;
- FFDemuxSubtitlesQueue *q = &vobsub->q;
+ FFDemuxSubtitlesQueue *q;
AVIOContext *pb = vobsub->sub_ctx->pb;
- int ret, psize, len16 = -1;
+ int ret, psize, total_read = 0, i;
AVPacket idx_pkt;
+ int64_t min_ts = INT64_MAX;
+ int sid = 0;
+ for (i = 0; i < s->nb_streams; i++) {
+ FFDemuxSubtitlesQueue *tmpq = &vobsub->q[i];
+ int64_t ts = tmpq->subs[tmpq->current_sub_idx].pts;
+ if (ts < min_ts) {
+ min_ts = ts;
+ sid = i;
+ }
+ }
+ q = &vobsub->q[sid];
ret = ff_subtitles_queue_read_packet(q, &idx_pkt);
if (ret < 0)
return ret;
@@ -819,19 +845,20 @@ static int vobsub_read_packet(AVFormatContext *s, AVPacket *pkt)
do {
int n, to_read, startcode;
int64_t pts, dts;
+ int64_t old_pos = avio_tell(pb), new_pos;
+ int pkt_size;
ret = mpegps_read_pes_header(vobsub->sub_ctx, NULL, &startcode, &pts, &dts);
if (ret < 0)
FAIL(ret);
to_read = ret & 0xffff;
+ new_pos = avio_tell(pb);
+ pkt_size = ret + (new_pos - old_pos);
/* this prevents reads above the current packet */
- if (pkt->size + to_read > psize)
- break;
-
- /* if the len is computed, we check for overread */
- if (len16 != -1 && pkt->size + to_read > len16)
+ if (total_read + pkt_size > psize)
break;
+ total_read += pkt_size;
/* the current chunk doesn't match the stream index (unlikely) */
if ((startcode & 0x1f) != idx_pkt.stream_index)
@@ -844,11 +871,7 @@ static int vobsub_read_packet(AVFormatContext *s, AVPacket *pkt)
n = avio_read(pb, pkt->data + (pkt->size - to_read), to_read);
if (n < to_read)
pkt->size -= to_read - n;
-
- /* first chunk contains the total len of the packet to raise */
- if (len16 == -1 && n > 2)
- len16 = AV_RB16(pkt->data);
- } while (len16 != -1 && pkt->size != len16);
+ } while (total_read < psize);
pkt->pts = pkt->dts = idx_pkt.pts;
pkt->pos = idx_pkt.pos;
@@ -858,6 +881,7 @@ static int vobsub_read_packet(AVFormatContext *s, AVPacket *pkt)
return 0;
fail:
+ av_free_packet(pkt);
av_free_packet(&idx_pkt);
return ret;
}
@@ -871,6 +895,7 @@ static int vobsub_read_seek(AVFormatContext *s, int stream_index,
* same for all subtitles stream within a .idx/.sub). Rescaling is done just
* like in avformat_seek_file(). */
if (stream_index == -1 && s->nb_streams != 1) {
+ int i, ret = 0;
AVRational time_base = s->streams[0]->time_base;
ts = av_rescale_q(ts, AV_TIME_BASE_Q, time_base);
min_ts = av_rescale_rnd(min_ts, time_base.den,
@@ -879,16 +904,26 @@ static int vobsub_read_seek(AVFormatContext *s, int stream_index,
max_ts = av_rescale_rnd(max_ts, time_base.den,
time_base.num * (int64_t)AV_TIME_BASE,
AV_ROUND_DOWN | AV_ROUND_PASS_MINMAX);
+ for (i = 0; i < s->nb_streams; i++) {
+ int r = ff_subtitles_queue_seek(&vobsub->q[i], s, stream_index,
+ min_ts, ts, max_ts, flags);
+ if (r < 0)
+ ret = r;
+ }
+ return ret;
}
- return ff_subtitles_queue_seek(&vobsub->q, s, stream_index,
+ return ff_subtitles_queue_seek(&vobsub->q[stream_index], s, stream_index,
min_ts, ts, max_ts, flags);
}
static int vobsub_read_close(AVFormatContext *s)
{
+ int i;
MpegDemuxContext *vobsub = s->priv_data;
- ff_subtitles_queue_clean(&vobsub->q);
+
+ for (i = 0; i < s->nb_streams; i++)
+ ff_subtitles_queue_clean(&vobsub->q[i]);
if (vobsub->sub_ctx)
avformat_close_input(&vobsub->sub_ctx);
return 0;
diff --git a/libavformat/subtitles.c b/libavformat/subtitles.c
index 6c9e72b..fce2bf1 100644
--- a/libavformat/subtitles.c
+++ b/libavformat/subtitles.c
@@ -57,7 +57,7 @@ AVPacket *ff_subtitles_queue_insert(FFDemuxSubtitlesQueue *q,
return sub;
}
-static int cmp_pkt_sub(const void *a, const void *b)
+static int cmp_pkt_sub_ts_pos(const void *a, const void *b)
{
const AVPacket *s1 = a;
const AVPacket *s2 = b;
@@ -69,11 +69,25 @@ static int cmp_pkt_sub(const void *a, const void *b)
return s1->pts > s2->pts ? 1 : -1;
}
+static int cmp_pkt_sub_pos_ts(const void *a, const void *b)
+{
+ const AVPacket *s1 = a;
+ const AVPacket *s2 = b;
+ if (s1->pos == s2->pos) {
+ if (s1->pts == s2->pts)
+ return 0;
+ return s1->pts > s2->pts ? 1 : -1;
+ }
+ return s1->pos > s2->pos ? 1 : -1;
+}
+
void ff_subtitles_queue_finalize(FFDemuxSubtitlesQueue *q)
{
int i;
- qsort(q->subs, q->nb_subs, sizeof(*q->subs), cmp_pkt_sub);
+ qsort(q->subs, q->nb_subs, sizeof(*q->subs),
+ q->sort == SUB_SORT_TS_POS ? cmp_pkt_sub_ts_pos
+ : cmp_pkt_sub_pos_ts);
for (i = 0; i < q->nb_subs; i++)
if (q->subs[i].duration == -1 && i < q->nb_subs - 1)
q->subs[i].duration = q->subs[i + 1].pts - q->subs[i].pts;
diff --git a/libavformat/subtitles.h b/libavformat/subtitles.h
index dbd9e01..b5a96ec 100644
--- a/libavformat/subtitles.h
+++ b/libavformat/subtitles.h
@@ -25,11 +25,17 @@
#include "avformat.h"
#include "libavutil/bprint.h"
+enum sub_sort {
+ SUB_SORT_TS_POS = 0, ///< sort by timestamps, then position
+ SUB_SORT_POS_TS, ///< sort by position, then timestamps
+};
+
typedef struct {
AVPacket *subs; ///< array of subtitles packets
int nb_subs; ///< number of subtitles packets
int allocated_size; ///< allocated size for subs
int current_sub_idx; ///< current position for the read packet callback
+ enum sub_sort sort; ///< sort method to use when finalizing subtitles
} FFDemuxSubtitlesQueue;
/**
diff --git a/tests/ref/fate/sub2video b/tests/ref/fate/sub2video
index d052c10..86044d2 100644
--- a/tests/ref/fate/sub2video
+++ b/tests/ref/fate/sub2video
@@ -54,7 +54,7 @@
1, 15355, 15355, 4733, 2094, 0x3c171425, F=0x0
1, 48797, 48797, 2560, 2480, 0x7c0edf21, F=0x0
1, 51433, 51433, 2366, 3059, 0xc95b8a05, F=0x0
-1, 53919, 53919, 2696, 2095, 0x61bb15ed, F=0x0
+1, 53910, 53910, 2696, 2095, 0x61bb15ed, F=0x0
1, 56663, 56663, 1262, 1013, 0xc9ae89b7, F=0x0
1, 58014, 58014, 1661, 969, 0xe01878f0, F=0x0
1, 67724, 67724, 1365, 844, 0xe7db4fc1, F=0x0
@@ -85,11 +85,10 @@
1, 191356, 191356, 1228, 1517, 0xae8c5c2b, F=0x0
1, 192640, 192640, 1763, 2506, 0xa458d6d4, F=0x0
1, 195193, 195193, 1092, 1074, 0x397ba9a8, F=0x0
-1, 196369, 196369, 1524, 1715, 0x695ca41e, F=0x0
+1, 196361, 196361, 1524, 1715, 0x695ca41e, F=0x0
1, 197946, 197946, 1160, 789, 0xc63a189e, F=0x0
1, 199230, 199230, 1627, 1846, 0xeea8c599, F=0x0
1, 200924, 200924, 1763, 922, 0xd4a87222, F=0x0
1, 210600, 210600, 1831, 665, 0x55580135, F=0x0
1, 214771, 214771, 1558, 1216, 0x50d1f6c5, F=0x0
1, 225640, 225640, 2127, 2133, 0x670c11a5, F=0x0
-1, 227834, 227834, 1262, 1264, 0xc1d9fc57, F=0x0
More information about the ffmpeg-cvslog
mailing list