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){
On 10/25/14, reimar <subversion@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). Please revert. Best Regards Ivan Kalvachev iive
On 02.12.2014, at 22:22, Ivan Kalvachev <ikalvachev@gmail.com> wrote:
On 10/25/14, reimar <subversion@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).
Please revert.
Reverting it breaks auto-detection with non-seekable inputs, thus is not a real solution either.
On Wed, Dec 03, 2014 at 09:24:18AM +0100, Reimar Döffinger wrote:
On 02.12.2014, at 22:22, Ivan Kalvachev <ikalvachev@gmail.com> wrote:
On 10/25/14, reimar <subversion@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; }
On 12/3/14, Reimar Döffinger <Reimar.Doeffinger@gmx.de> wrote:
On Wed, Dec 03, 2014 at 09:24:18AM +0100, Reimar Döffinger wrote:
On 02.12.2014, at 22:22, Ivan Kalvachev <ikalvachev@gmail.com> wrote:
On 10/25/14, reimar <subversion@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; }
Sorry I missed your patch. I just tested it and it seems to work. Feel free to commit it. If I encounter problem I'll tell you right away.
On Sat, Dec 06, 2014 at 02:13:48AM +0200, Ivan Kalvachev wrote:
On 12/3/14, Reimar Döffinger <Reimar.Doeffinger@gmx.de> wrote:
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; }
Sorry I missed your patch. I just tested it and it seems to work.
Feel free to commit it. If I encounter problem I'll tell you right away.
Committed, thanks.
participants (3)
-
Ivan Kalvachev -
reimar -
Reimar Döffinger