[vlc-devel] [PATCH] Bug fix: Do not close an http stream when seeking past the EOF

[Bill C. Riemers]

Rémi Denis-Courmont wrote:
> Le mardi 17 juin 2008 01:08:24 Bill C. Riemers, vous avez écrit :
>   
>> +    if( i_pos < 0 ) {
>> +        msg_Err( p_access, "seek to soon" );
>> +        i_pos = 0;
>> +    }
>>     
>
> As has been discussed a few months ago, the access position really is 
> unsigned, and the file access plugins treat it as such. Hence there should be 
> no need to special case.
>
> I know the (buggy) AVI demux sometimes seeks (or at least used to seek) 
> to "very large" negative offsets. The file access plugins cast the position 
> to unsigned, pending a later disruptive fix of the access_t.pf_seek 
> prototype. As such, they effectively seek to the end of the file.
>
>   
This is just more of defensive programming.  I tend to assume just
because a value is currently
unsigned for existing platforms, does not mean it will always be
unsigned for all platforms.


>> +    if(p_access->info.i_size > 0 && i_pos >= p_access->info.i_size ) {
>> +        msg_Err( p_access, "seek to far" );
>> +        int retval = Seek( p_access, p_access->info.i_size - 1 );
>> +        if( retval == VLC_SUCCESS ) {
>> +            uint8_t p_buffer[2];
>> +            Read( p_access, p_buffer, 1);
>> +            p_access->info.b_eof  = false;
>> +        }
>> +        return retval;
>> +    }
>>     
>
> As far as I can tell, at (and beyond) end of file, b_eof should be set. It 
> should be reset to false only if the reader seeks backward. That's how both 
> file access plugins operate at the moment.
>   
Actually no.  The code in file.c is:

static int Seek (access_t *p_access, int64_t i_pos)
{
    p_access->info.i_pos = i_pos;
    p_access->info.b_eof = false;

    lseek (p_access->p_sys->fd, i_pos, SEEK_SET);
    return VLC_SUCCESS;
}


Which means it just tries the seek, and ignores the result.  The file is
not set to EOF by the seek operation.  Since avi demux and possibly
other modules assume this behaviour.  I suppose if I just wanted to
exactly replicate this behaviour, I would simply set the
p_access->info.i_pos in the advent of a seek past the end of the file.  
But it seemed more defensive to actually move to the end of the file.  
Probably a better solution would be to change file.c to actually check
the lseek return value and set EOF and return an error when failure
occurs.   Then correct every demux module to work this way.   But I was
just going for the functional solution, not necessarily the best solution.

Bill



> While this might seem to work for the AVI demux, it will break the input 
> stream code and screw up other demuxers. I believe the proper solution is 
> simply to reset b_eof to false in the Seek callback. Laurent should triple 
> check.
>
>