[vlc-devel] [PATCH] stream_filter: prefetch: early fail on seek

[Rémi Denis-Courmont]

Le 2016-07-28 10:55, Francois Cartegnie a écrit :
> Le 27/07/2016 à 22:20, Rémi Denis-Courmont a écrit :
>> Nothing says that seeking past the end should fail. This has already 
>> been
>> discussed, and e.g. stream_Memory allows it.
>>
>> Besides this needlessly breaks streams with changing size. And 
>> that´s
>> definitely possible and sometimes requested for local files.
>>
>> No thanks.
>
> So now we have non-deterministic return code on seek,
> depending on the fastseek ability of the access.

Seek return code has never been deterministic. Operating systems often 
do not return error on seek for optimization reasons, essentially the 
same thing that the prefetch filter does. ISO and POSIX explicitly allow 
that, as they do seeking past end offset. Also network streams can 
obviously fail asynchronously; that would be undeterministic to the 
stream consumer.

> Introduces regressions in demuxers relying on stream_Seek return 
> code.

The only reliable indication of end of stream (or fatal error) is the 
read operation return value. stream_Seek() can fail, and of course you 
cannot ignore the failure when it does occur. But the stream API cannot 
warranty that stream_Seek will detect and report failures. Indeed, such 
a warranty simply would not make sense: the stream can always fail after 
seek and before the following read.

Likewise, the size of the file can shrink after seek and before read, 
so no matter what, there will always be a possibility for:
     stream_Tell() > stream_Size().

It might have been that the stream cache built into the stream layer 
used to return an error on seek past end in the common case. That might 
have been a side effect of how "refill" was implemented. But it wouldn't 
have worked in corner cases such as those noted above. More importantly, 
such behaviour cannot be reproduced efficiently in the prefetch filter, 
due to stream with unknown size or changing size.

In other words, if a stream consumer breaks when seek past end 
succeeds, the stream consumer is buggy. And it already was buggy before, 
albeit in perhaps not so commonly experienced fashion.

Finally, you seem to have problems with streams of unknown size. Your 
proposed patch makes no difference whatsoever in that case.

-- 
Rémi Denis-Courmont
http://www.remlab.net/