Musepack Forums

Musepack Forums (https://forum.musepack.net/index.php)
-   Development (https://forum.musepack.net/forumdisplay.php?f=11)
-   -   Help in mpc_demux_seek() needed (https://forum.musepack.net/showthread.php?t=624)

Buschel 13 May 2010 01:12 pm

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 13 May 2010 03:53 pm

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 13 May 2010 09:03 pm

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);
}

r2d 15 May 2010 05:49 pm

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

Buschel 15 May 2010 09:07 pm

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.

r2d 15 May 2010 10:43 pm

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).


All times are GMT. The time now is 03:40 pm.

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