[FFmpeg-devel] [PATCH] RTSP-MS 15/15: move packet_time_start zero value assignment in asf.c

[Ronald S. Bultje]

Hi,

On Wed, Apr 15, 2009 at 6:24 PM, Michael Niedermayer <michaelni at gmx.at> wrote:
> so what you say is that the asf demuxer has a bug in relation to EOF handling
> ?
>
> and you want to fix that?

Oh! This is a big can of worms and you knew it!

ASF files never reach EOF, they really do loop forever! This is a ASF
packet header/data printf showing current pos / file size:

Reading ASF packet header at 13618013/13639000
Parsing ASF packet data at 13618027/13639000
Parsing ASF packet data at 13619531/13639000
Parsing ASF packet data at 13621131/13639000
Parsing ASF packet data at 13622622/13639000
Parsing ASF packet data at 13624019/13639000
Reading ASF packet header at 13624566/13639000
Parsing ASF packet data at 13624580/13639000
Parsing ASF packet data at 13626151/13639000
Parsing ASF packet data at 13627647/13639000
Reading ASF packet header at 13631119/13639000
Parsing ASF packet data at 13631132/13639000
Parsing ASF packet data at 13632630/13639000
Parsing ASF packet data at 13637672/13639000
[.. this is EOF! See below ..]
Parsing ASF packet data at 13642714/13639000
Parsing ASF packet data at 13647756/13639000
Parsing ASF packet data at 13652798/13639000

At the "[.. see below ..]" point, every round of data reading consists
just of trying to read the header, failing (because there's no data,
all get_byte() and get_l/beXX() operations and such return 0) and thus
calling url_fskip() to skip the data and try again. However,
url_fskip() is broken in that it never sets eof_reached. It calls
url_fseek(SEEK_CUR, skip_size);, which calls file_seek(), which calls
lseek(2):

     "The lseek() function allows the file offset to be set beyond the end of
     the existing end-of-file of the file."

Shit... I hate you. :-). The only calls that ever set eof_reached()
for regular files are get_buffer() or fill_buffer(), neither of which
are used in asfdec.c, at least not as long as all calls to
get_byte()/get_l/beXX() return 0. Any suggested way of fixing this?...
I'd almost introduce a call to get_buffer() just to use some call that
sets eof_reached(), but I suppose it'd be cleaner to have url_fskip()
set eof_reached when we're at EOF (in read-only streams). Agree? If
so, best thing I could think of so far is to add a check in
url_fseek() (could be placed in file_seek() also if you so prefer) to
make sure we don't go beyond EOF, see attached...

Ronald
-------------- next part --------------
Index: ffmpeg-svn/libavformat/aviobuf.c
===================================================================
--- ffmpeg-svn.orig/libavformat/aviobuf.c	2009-04-13 16:10:01.000000000 -0400
+++ ffmpeg-svn/libavformat/aviobuf.c	2009-04-15 21:32:21.000000000 -0400
@@ -159,7 +159,7 @@
             return AVERROR(EPIPE);
         s->buf_ptr = s->buf_end + offset - s->pos;
     } else {
-        int64_t res = AVERROR(EPIPE);
+        int64_t res = AVERROR(EPIPE), size;
 
 #if CONFIG_MUXERS || CONFIG_NETWORK
         if (s->write_flag) {
@@ -167,12 +167,20 @@
             s->must_flush = 1;
         }
 #endif /* CONFIG_MUXERS || CONFIG_NETWORK */
-        if (!s->seek || (res = s->seek(s->opaque, offset, SEEK_SET)) < 0)
+        offset1 = offset;
+        if (!s->write_flag && offset > (size = url_fsize(s)))
+            offset1 = size;
+        if (!s->seek || (res = s->seek(s->opaque, offset1, SEEK_SET)) < 0)
             return res;
         if (!s->write_flag)
             s->buf_end = s->buffer;
         s->buf_ptr = s->buffer;
-        s->pos = offset;
+        s->pos = res;
+        if (res != offset) {
+            if (res < offset)
+                s->eof_reached = 1;
+            return AVERROR(EPIPE);
+        }
     }
     s->eof_reached = 0;
     return offset;