[FFmpeg-devel] [PATCH] ogg: Fix seek to zero and packet pts after read through.

[Michael Niedermayer]

On Mon, Apr 23, 2012 at 04:53:13PM -0700, Dale Curtis 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;

this doesnt look entirely right
for codecs with b frames  pts is != dts  for some frames


> > +        }
> > +        os->keyframe_seek = 0;
> > +    }
> > +    avio_seek(s->pb, seek_pos, SEEK_SET);
> >     return ret;
> >  }
[...]
> >
> 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.

as you mention this was tested since a while
have you checked that pts at the start is always 0 as the patch seems
to assume



> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The real ebay dictionary, page 1
"Used only once"    - "Some unspecified defect prevented a second use"
"In good condition" - "Can be repaird by experienced expert"
"As is" - "You wouldnt want it even if you were payed for it, if you knew ..."
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20120424/f76490bf/attachment.asc>