[MPlayer-cvslog] r37309 - trunk/stream/stream.h

[Reimar Döffinger]

On Wed, Dec 03, 2014 at 09:24:18AM +0100, Reimar Döffinger wrote:
> On 02.12.2014, at 22:22, Ivan Kalvachev <ikalvachev at gmail.com> wrote:
> > On 10/25/14, reimar <subversion at mplayerhq.hu> wrote:
> >> Author: reimar
> >> Date: Sat Oct 25 17:27:08 2014
> >> New Revision: 37309
> >> 
> >> Log:
> >> stream: Avoid unnecessary seek when our buffer is empty.
> >> 
> >> Modified:
> >>   trunk/stream/stream.h
> >> 
> >> Modified: trunk/stream/stream.h
> >> ==============================================================================
> >> --- trunk/stream/stream.h    Sat Oct 25 15:40:40 2014    (r37308)
> >> +++ trunk/stream/stream.h    Sat Oct 25 17:27:08 2014    (r37309)
> >> @@ -324,6 +324,8 @@ static inline int stream_seek(stream_t *
> >>            "Invalid seek to negative position %"PRIx64"!\n", pos);
> >>     pos = 0;
> >>   }
> >> +  if (s->buf_len == 0 && s->pos == pos)
> >> +    return 1;
> >>   if(pos<s->pos){
> >>     int64_t x=pos-(s->pos-s->buf_len);
> >>     if(x>=0){
> > 
> > This breaks sub/subreader.c::guess_cp() code.
> > The code reads the file once and feeds it to enca, in order to guess
> > the encoding/code_page. Then it calls:
> >    stream_reset();
> >    stream_seek();.
> > 
> > With the above change, data structures are changed, but actual file
> > seek is not performed. So when subtitle parsing really starts there is
> > nothing to parse.
> > 
> > The interesting part is that stream_reset() have a commented out call
> > to stream_seek(), in order to ensure actual seek, but I guess it have
> > been removed as it is unnecessary in the most common case (opening
> > file).

I think the intention is that stream_reset actually should only
reset the buffers etc. but not actually seek.

> > Please revert.
> 
> Reverting it breaks auto-detection with non-seekable inputs, thus is not a real solution either.

Does the below work? It looks to me like it should be correct, and the pos=0 assignment
is from a very early revision, so there's no indication it is actually required
for anything from the logs.

--- a/stream/stream.c
+++ b/stream/stream.c
@@ -482,7 +482,6 @@ while(stream_fill_buffer(s) > 0 && pos >= 0) {
 
 void stream_reset(stream_t *s){
   if(s->eof){
-    s->pos=0;
     s->buf_pos=s->buf_len=0;
     s->eof=0;
   }