Musepack Forums  

Go Back   Musepack Forums > Main > Development

Reply
 
Thread Tools Search this Thread Display Modes
Old 13 May 2010, 01:12 pm   #1
Buschel
The Man
 
Join Date: Mar 2008
Posts: 9
Default Help in mpc_demux_seek() needed

Hi all,

as you may know I have ported the sv8 decoder to rockbox. When doing so there were some issues that needed to be solved due to changes especially in the buffer code. For example the change to use bytes instead for dwords slowed down decoding a lot on Coldfire targets.
Now a rockbox user found a major issue that I did not recognize during the port: Seeking forward in a file is extremely slow when this seek needs to scan (skip over frames) the first time. Seeking backwards or within the tableized area is as fast as expected. Reason for this slow seek is that within mpc_demux_seek() the buffer is always flushed and re-read again (a "FIXME"-comment tells the same). Is there a possibility to fix this behaviour?

best wishes,
Buschel
Buschel is offline   Reply With Quote
Old 13 May 2010, 03:53 pm   #2
Buschel
The Man
 
Join Date: Mar 2008
Posts: 9
Default

I am searching for a solution that looks similar to this:

// d->bits_reader.buff - d->buffer = position within buffer
// d->bytes_total = total position in file at buffer start
// fpos = file position in bit (not byte)
// min_bytes = minimum number of bytes to be available from fpos on
if ((fpos>>3) >= d->bytes_total &&
d->bits_reader.buff - d->buffer + d->bytes_total + (fpos>>3) + min_bytes <= DEMUX_BUFFER_SIZE)
{
int total_pos_bytes = d->bytes_total + d->bits_reader.buff - d->buffer;
int diff_pos_bytes = (fpos>>3) - total_pos_bytes;
d->bits_reader.buff += diff_pos_bytes;
d->bits_reader.count = 8 - (fpos & 7);
}
else
{
next_pos = fpos >> 3;
if (d->si.stream_version == 7)
next_pos = ((next_pos - d->si.header_position) & (-1 << 2)) + d->si.header_position;
bit_offset = (int) (fpos - (next_pos << 3));

d->r->seek(d->r, (mpc_int32_t) next_pos);
mpc_demux_clear_buff(d);
if (d->si.stream_version == 7)
mpc_demux_fill(d, DEMUX_BUFFER_SIZE, MPC_BUFFER_SWAP);
else
mpc_demux_fill(d, DEMUX_BUFFER_SIZE, 0);
d->bits_reader.buff += bit_offset >> 3;
d->bits_reader.count = 8 - (bit_offset & 7);
}

This code still performs a search. But it still does not enter the first if-path. This is because I am still not sure how to use and interprete the different variables. Help is very much appreciated.

Goal: Check whether the next needed bytes are within the current buffer. if not, refill the buffer completely.
Buschel is offline   Reply With Quote
Old 13 May 2010, 09:03 pm   #3
Buschel
The Man
 
Join Date: Mar 2008
Posts: 9
Default

The following code works on simulation and ARM target. It is by factors faster than the unchanged version. I am not fully satisfied with the code, but it is a proof-of-concept. Ayn further ideas/comments from your side?

static void
mpc_demux_seek(mpc_demux * d, mpc_seek_t fpos, mpc_uint32_t min_bytes) {
mpc_seek_t next_pos;
mpc_int_t bit_offset;
mpc_seek_t tell_pos = d->r->tell(d->r) - d->bytes_total;
// static int no_flush = 0;
// static int cnt_flush = 0;

// d->bits_reader.buff - d->buffer = position within buffer
// d->bytes_total = total position in file at buffer start
// fpos = file position in bit (not byte)
// tell_pos = minimum number of bytes to be available from fpos on
if ((fpos>>3) >= tell_pos &&
d->bits_reader.buff - d->buffer + (fpos>>3) + min_bytes - tell_pos <= DEMUX_BUFFER_SIZE)
{
int total_pos_bytes = d->bits_reader.buff - d->buffer + tell_pos;
int diff_pos_bytes = (fpos>>3) - total_pos_bytes;
d->bits_reader.buff += diff_pos_bytes;
d->bits_reader.count = 8 - (fpos & 7);

// ++no_flush;
}
else
{
next_pos = fpos >> 3;
if (d->si.stream_version == 7)
next_pos = ((next_pos - d->si.header_position) & (-1 << 2)) + d->si.header_position;
bit_offset = (int) (fpos - (next_pos << 3));

d->r->seek(d->r, (mpc_int32_t) next_pos);
mpc_demux_clear_buff(d);
if (d->si.stream_version == 7)
mpc_demux_fill(d, DEMUX_BUFFER_SIZE, MPC_BUFFER_SWAP);
else
mpc_demux_fill(d, DEMUX_BUFFER_SIZE, 0);
d->bits_reader.buff += bit_offset >> 3;
d->bits_reader.count = 8 - (bit_offset & 7);

// ++cnt_flush;
}
// printf("buffer ok = %d, flush = %d\n", no_flush, cnt_flush);
}
Buschel is offline   Reply With Quote
Old 15 May 2010, 05:49 pm   #4
r2d
Musepack developer
 
Join Date: Sep 2006
Location: Villeurbanne - France
Posts: 36
Default

I made a small change to make seeking in the buffer faster :
http://trac.musepack.net/trac/changeset/459

I've rapidly checked that it works, but maybe it's buggy or not fast enough for you

You said that seeking is slow the first time : the file doesn't contain an index ? Adding the index may be the best way to fix the issue.

regards,

Nicolas
r2d is offline   Reply With Quote
Old 15 May 2010, 09:07 pm   #5
Buschel
The Man
 
Join Date: Mar 2008
Posts: 9
Default

Hi r2d, thanks for your response!

I already submitted the following change (verified by 3 users to the rockbox depot:

static void
mpc_demux_seek(mpc_demux * d, mpc_seek_t fpos, mpc_uint32_t min_bytes) {
- mpc_seek_t next_pos;
- mpc_int_t bit_offset;
+ // d->bits_reader.buff - d->buffer = current byte position within buffer
+ // d->bytes_total = buffer is filled with bytes_total bytes
+ // fpos = desired file position in bit (not byte)
+ // buf_fpos = desired byte position within buffer
+ mpc_seek_t next_pos = fpos>>3;
+ mpc_int_t buf_fpos = next_pos - d->r->tell(d->r) + d->bytes_total;

- // FIXME : do not flush the buffer if fpos is in the current buffer
-
- next_pos = fpos >> 3;
- if (d->si.stream_version == 7)
- next_pos = ((next_pos - d->si.header_position) & (-1 << 2)) + d->si.header_position;
- bit_offset = (int) (fpos - (next_pos << 3));
-
- d->r->seek(d->r, (mpc_int32_t) next_pos);
- mpc_demux_clear_buff(d);
- if (d->si.stream_version == 7)
- mpc_demux_fill(d, (min_bytes + ((bit_offset + 7) >> 3) + 3) & (~3), MPC_BUFFER_SWAP);
- else
- mpc_demux_fill(d, min_bytes + ((bit_offset + 7) >> 3), 0);
- d->bits_reader.buff += bit_offset >> 3;
- d->bits_reader.count = 8 - (bit_offset & 7);
+ // is desired byte position is within lower and upper boundaries of buffer?
+ if (buf_fpos >= 0 && buf_fpos + min_bytes <= DEMUX_BUFFER_SIZE) {
+ // desired bytes are available in current buffer
+ d->bits_reader.buff += buf_fpos - (d->bits_reader.buff - d->buffer);
+ d->bits_reader.count = 8 - (fpos & 7);
+ } else {
+ // buffer needs to be refilled
+ if (d->si.stream_version == 7)
+ next_pos = ((next_pos - d->si.header_position) & (-1 << 2)) + d->si.header_position;
+ buf_fpos = fpos - (next_pos << 3);
+
+ d->r->seek(d->r, (mpc_int32_t) next_pos);
+ mpc_demux_clear_buff(d);
+ if (d->si.stream_version == 7)
+ mpc_demux_fill(d, DEMUX_BUFFER_SIZE, MPC_BUFFER_SWAP);
+ else
+ mpc_demux_fill(d, DEMUX_BUFFER_SIZE, 0);
+ d->bits_reader.buff += buf_fpos >> 3;
+ d->bits_reader.count = 8 - (buf_fpos & 7);
+ }
}

This change is similar to yours but will avoid to refill the buffer with small amounts of data on every call. Instead it will try to work with already buffered data as long as possible. If the buffer runs out of data it will fill the whole buffer. On my test file I could reduce buffer fills to ~1% of all calls.
Buschel is offline   Reply With Quote
Old 15 May 2010, 10:43 pm   #6
r2d
Musepack developer
 
Join Date: Sep 2006
Location: Villeurbanne - France
Posts: 36
Default

As you will see, mpc_demux_fill() already checks if there is less than min_bytes in the buffer and refill only if there is less than that. And if there is less than min_bytes in the buffer, it will be fully refilled if the flag MPC_BUFFER_FULL is passed (which seems to be used only for sv7 ... maybe there is an improvement to do here).
r2d is offline   Reply With Quote
Reply

Thread Tools Search this Thread
Search this Thread:

Advanced Search
Display Modes

Posting Rules
You may not post new threads
You may not post replies
You may not post attachments
You may not edit your posts

BB code is On
Smilies are On
[IMG] code is On
HTML code is Off



All times are GMT. The time now is 05:36 pm.


Powered by vBulletin® Version 3.8.11 Beta 2
Copyright ©2000 - 2017, vBulletin Solutions Inc.