Rockbox

This is the bug/patch tracker for Rockbox. Click here for more information.

Quick links: Bugs · Patches · Rockbox frontpage

Tasklist

FS#8092 - rebuffer_handle is hopelessly broken

Attached to Project: Rockbox
Opened by Brandon Low (lostlogic) - Monday, 05 November 2007, 04:22 GMT+2
Last edited by Nicolas Pennequin (nicolas_p) - Monday, 26 November 2007, 22:14 GMT+2
Task Type Bugs
Category Music playback
Status Closed
Assigned To Nicolas Pennequin (nicolas_p)
Player Type All players
Severity High
Priority High
Reported Version Daily build (which?)
Due in Version Undecided
Due Date Undecided
Percent Complete 100%
Private No

Details

rebuffer_handle for backwards seeks is hopelessly broken. it will only work in the very isolated case that there is no track after the playing track on buffer. Otherwise it will end up with a short allocation as it tries to rebuffer the new file from earlier than it was originally buffered.

I tried various solutions but none look good. First I tried to move it earlier on the buffer to make space. This doesn't work because if the buffer is filling we could be mid-read and could then put the current track back on space that is about to get overwritten. Even if the buffer is not filling, if there are any tracks on buffer earlier than the playing track then it can't safely move earlier.

My next solution was to return an error code which tells playback that it needs to do a full rebuffer. I have a patch with this proposed solution (as well as an associated but not equivalent fix to put the PRESEEK functionality back into the world and manage it from the playback side.

See attached patch, which breaks horribly and I don't see why.

This FS is here for a tracker as either Nico or I get to fixing this for real.
   20071104_rebuffer.patch (7.2 KiB) 
 apps/playback.c  |   36 +++++++++++++++++++++---------------
 apps/buffering.c |   36 +++++++++++++++++-------------------
 apps/buffering.h |    2 +-
 3 files changed, 39 insertions(+), 35 deletions(-)

This task depends upon

Closed by  Nicolas Pennequin (nicolas_p)
Monday, 26 November 2007, 22:14 GMT+2
Reason for closing:  Fixed
Comment by Steve Bavin (pondlife) - Thursday, 15 November 2007, 10:14 GMT+2
I probably don't understand the issue, but it seems that any solution should be internal to buffering and not involve playback. The need to rebuffer data in this manner could apply to any routine that uses buffering, no?

If it's doable, just do a complete rebuffer in this case. It's not often someone will rewind (or skip back?) before the start of buffered data, so not worth complicating the code to preserve later tracks.

Although ...Like I said, I probably don't understand the issue.... :)
Comment by Nicolas Pennequin (nicolas_p) - Monday, 19 November 2007, 19:19 GMT+2
A solution is to let the buffering code close a few handles to make room for the one that needs rebuffering. This isn't possible currently because we have no way of notifying users that handles have been closed, but I'm working on an idea suggested by jhMikeS where the buffer_low_callback (or whatever it's called) is generalised to be able to pass any message to the users. With such a callback, users could be informed that handles were closed to make room.
Comment by Magnus Holmgren (learman) - Sunday, 25 November 2007, 10:13 GMT+2
A full rebuffer is what happened before MoB, AFAIK, so that could be an acceptable fix until a better solution is found.

Loading...