[FFmpeg-devel] [PATCH] ogg: Fix seek to zero and packet pts after read through.
Reimar Döffinger
Reimar.Doeffinger at gmx.de
Tue Apr 24 07:50:25 CEST 2012
On 24 Apr 2012, at 01:53, Dale Curtis <dalecurtis at chromium.org> wrote:
> On Thu, Apr 19, 2012 at 11:07 AM, <dalecurtis at chromium.org> wrote:
>
>> From: Dale Curtis <dalecurtis at chromium.org>
>>
>> Seeking to zero doesn't always work for some ogg files. Even the
>> one included with the FATE test.
>>
>> Additionally, pts values are not equivalent between the first and
>> second pass through an ogg file with a zero seek in between.
>> Specifically:
>>
>> while(av_read_frame(packet))
>> print(packet.pts);
>> av_seek_frame(0)
>> while(av_read_frame(packet))
>> print(packet.pts);
>>
>> Will print different values. Previously this lead to a problem
>> where seeking to zero was broken. Some concrete numbers:
>>
>> 1st pass pts: -128 0 128 256 832
>> 2nd pass pts: 0 576 704 832 1408
>>
>> Reproducible using FATE and the following sample program and test case:
>>
>> http://commondatastorage.googleapis.com/dalecurtis-shared/ogg-test.tar.bz2
>>
>> Signed-off-by: Dale Curtis <dalecurtis at chromium.org>
>> ---
>> libavformat/oggdec.c | 28 ++++++++++++++++++++++++++++
>> tests/ref/seek/lavf_ogg | 24 ++++++++++++++++--------
>> 2 files changed, 44 insertions(+), 8 deletions(-)
>>
>> diff --git a/libavformat/oggdec.c b/libavformat/oggdec.c
>> index 6a55f89..f823e85 100644
>> --- a/libavformat/oggdec.c
>> +++ b/libavformat/oggdec.c
>> @@ -657,6 +657,8 @@ static int64_t ogg_read_timestamp(AVFormatContext *s,
>> int stream_index,
>> int64_t pts = AV_NOPTS_VALUE;
>> int64_t keypos = -1;
>> int i = -1;
>> + int packet = 0;
>> + int64_t start_pos = *pos_arg;
>> int pstart, psize;
>> avio_seek(bc, *pos_arg, SEEK_SET);
>> ogg_reset(ogg);
>> @@ -676,6 +678,12 @@ static int64_t ogg_read_timestamp(AVFormatContext *s,
>> int stream_index,
>> else
>> pts = AV_NOPTS_VALUE;
>> }
>> +
>> + // This is for the special case for the first packet in the
>> stream.
>> + if (pts == AV_NOPTS_VALUE && start_pos <= s->data_offset &&
>> !packet) {
>> + pts = 0;
>> + }
>> + ++packet;
>> }
>> if (pts != AV_NOPTS_VALUE)
>> break;
>> @@ -690,6 +698,10 @@ static int ogg_read_seek(AVFormatContext *s, int
>> stream_index,
>> struct ogg *ogg = s->priv_data;
>> struct ogg_stream *os = ogg->streams + stream_index;
>> int ret;
>> + int64_t seek_pos;
>> + int64_t pos_arg;
>> + int64_t seek_pts;
>> + int i;
>>
>> av_assert0(stream_index < ogg->nstreams);
>> // Ensure everything is reset even when seeking via
>> @@ -706,6 +718,22 @@ static int ogg_read_seek(AVFormatContext *s, int
>> stream_index,
>> os = ogg->streams + stream_index;
>> if (ret < 0)
>> os->keyframe_seek = 0;
>> +
>> + // Save the position seeked to.
>> + pos_arg = seek_pos = avio_tell(s->pb);
>> + seek_pts = ogg_read_timestamp(s, stream_index, &pos_arg,
>> avio_size(s->pb));
>> + os = ogg->streams + stream_index;
>> +
>> + // Since we have seeked to the beginning then reset lastpts and
>> lastdts to 0.
>> + if (!seek_pts) {
>> + for (i = 0; i < ogg->nstreams; i++){
>> + struct ogg_stream *stream = ogg->streams + i;
>> + stream->lastpts = 0;
>> + stream->lastdts = 0;
>> + }
>> + os->keyframe_seek = 0;
>> + }
>> + avio_seek(s->pb, seek_pos, SEEK_SET);
>> return ret;
>> }
>>
>> diff --git a/tests/ref/seek/lavf_ogg b/tests/ref/seek/lavf_ogg
>> index a9704f3..2898d10 100644
>> --- a/tests/ref/seek/lavf_ogg
>> +++ b/tests/ref/seek/lavf_ogg
>> @@ -1,27 +1,35 @@
>> ret: 0 st: 0 flags:1 dts: 0.000000 pts: 0.000000 pos: 143
>> size: 1364
>> -ret:-1 st:-1 flags:0 ts:-1.000000
>> +ret: 0 st:-1 flags:0 ts:-1.000000
>> +ret: 0 st: 0 flags:1 dts: 0.000000 pts: 0.000000 pos: 143
>> size: 1364
>> ret:-1 st:-1 flags:1 ts: 1.894167
>> ret:-1 st: 0 flags:0 ts: 0.788345
>> -ret:-1 st: 0 flags:1 ts:-0.317506
>> +ret: 0 st: 0 flags:1 ts:-0.317506
>> +ret: 0 st: 0 flags:1 dts: 0.000000 pts: 0.000000 pos: 143
>> size: 1364
>> ret:-1 st:-1 flags:0 ts: 2.576668
>> ret:-1 st:-1 flags:1 ts: 1.470835
>> ret:-1 st: 0 flags:0 ts: 0.365011
>> -ret:-1 st: 0 flags:1 ts:-0.740839
>> +ret: 0 st: 0 flags:1 ts:-0.740839
>> +ret: 0 st: 0 flags:1 dts: 0.000000 pts: 0.000000 pos: 143
>> size: 1364
>> ret:-1 st:-1 flags:0 ts: 2.153336
>> ret:-1 st:-1 flags:1 ts: 1.047503
>> -ret:-1 st: 0 flags:0 ts:-0.058322
>> +ret: 0 st: 0 flags:0 ts:-0.058322
>> +ret: 0 st: 0 flags:1 dts: 0.000000 pts: 0.000000 pos: 143
>> size: 1364
>> ret:-1 st: 0 flags:1 ts: 2.835828
>> ret:-1 st:-1 flags:0 ts: 1.730004
>> ret:-1 st:-1 flags:1 ts: 0.624171
>> -ret:-1 st: 0 flags:0 ts:-0.481655
>> +ret: 0 st: 0 flags:0 ts:-0.481655
>> +ret: 0 st: 0 flags:1 dts: 0.000000 pts: 0.000000 pos: 143
>> size: 1364
>> ret:-1 st: 0 flags:1 ts: 2.412494
>> ret:-1 st:-1 flags:0 ts: 1.306672
>> ret:-1 st:-1 flags:1 ts: 0.200839
>> -ret:-1 st: 0 flags:0 ts:-0.904989
>> +ret: 0 st: 0 flags:0 ts:-0.904989
>> +ret: 0 st: 0 flags:1 dts: 0.000000 pts: 0.000000 pos: 143
>> size: 1364
>> ret:-1 st: 0 flags:1 ts: 1.989184
>> ret:-1 st:-1 flags:0 ts: 0.883340
>> -ret:-1 st:-1 flags:1 ts:-0.222493
>> +ret: 0 st:-1 flags:1 ts:-0.222493
>> +ret: 0 st: 0 flags:1 dts: 0.000000 pts: 0.000000 pos: 143
>> size: 1364
>> ret:-1 st: 0 flags:0 ts: 2.671678
>> ret:-1 st: 0 flags:1 ts: 1.565850
>> ret:-1 st:-1 flags:0 ts: 0.460008
>> -ret:-1 st:-1 flags:1 ts:-0.645825
>> +ret: 0 st:-1 flags:1 ts:-0.645825
>> +ret: 0 st: 0 flags:1 dts: 0.000000 pts: 0.000000 pos: 143
>> size: 1364
>> --
>> 1.7.7.3
>>
>>
> Ping? This is another patch we've had in place for a while, testing shows
> it's still necessary for seeking to zero in any ogg file. Thanks in advance.
There is no reason at all to assume that the first pts should be 0, a lot of times it is not.
In some applications, this can cause a long hang when the pts jumps from 0 to the correct value (which can be in the millions of seconds).
More information about the ffmpeg-devel
mailing list