[FFmpeg-devel] [PATCH] Fix Ogg data_offset computation.
Reimar Döffinger
Reimar.Doeffinger
Mon Nov 22 20:34:26 CET 2010
On Thu, Nov 18, 2010 at 03:04:45PM -0800, Aaron Colwell wrote:
> diff --git a/libavformat/oggdec.c b/libavformat/oggdec.c
> index 5e52bb3..1b85eeb 100644
> --- a/libavformat/oggdec.c
> +++ b/libavformat/oggdec.c
> @@ -373,9 +373,14 @@ ogg_packet (AVFormatContext * s, int *str, int *dstart, int *dsize, int64_t *fpo
> if (!os->header){
> os->segp = segp;
> os->psize = psize;
> - if (!ogg->headers)
> - s->data_offset = os->sync_pos;
> - ogg->headers = 1;
> + if (!ogg->headers) {
> + if (!s->data_offset || s->data_offset > os->sync_pos)
> + s->data_offset = os->sync_pos;
> + ogg->headers = 1;
> + for (i = 0; ogg->headers && i < ogg->nstreams; i++)
> + if (ogg->streams[i].header > 0)
> + ogg->headers = 0;
Ok, let me explain some things that confuse me so about this.
The demuxer assumes that the first non-header packet means no
more header packets will follow.
Thus if this !os->header condition triggers we should not read
further in read_headers because we already have all headers.
As far as I can tell, by resetting ogg->headers that read_headers
loop might go on for a long time, and if there is another
packet from this same stream the first one will just be discarded.
I think that the code instead should be (the "if (!ogg->headers)"
condititon should no longer be needed):
av_assert0(!ogg->headers);
ogg->headers = 1;
s->data_offset = os->sync_pos;
for (i = 0; i < ogg->nstreams; i++) {
struct ogg_stream *cur_os = ogg->streams + i;
// all header packets must be complete before the first non-
// header one, so everything that follows must be non-header
cur_os->header = 0;
// if we have a partial non-header packet, its start is
// obviously at or after the data start
if (cur_os->incomplete) {
av_assert0(cur_os->sync_pos >= 0);
s->data_offset = FFMIN(s->data_offset, cur_os->sync_pos);
}
}
Completely untested of course.
More information about the ffmpeg-devel
mailing list