Wiki > Main > BufferingAPIProposal (compare)
Difference: BufferingAPIProposal (r14 vs. r13)
Currently, audio buffering and playback are interconnected, both being handled by playback.c. This has (IMHO) resulted in code which is becoming hard to maintain, with unclear responsibilities.
It would be useful to seperate these functions:
The following is a scratchpad for us to develop the idea of a buffering API that will control all access to the audio buffer.
The API is a simplified read-only file i/o API. It may be useful to think of it as a Rockbox-ed "SMARTDRV" that aims to pre-buffer all required data so that playback etc. can operate with minimal waiting for disk access.
int bufopen(const char *pathname, const long offset)
Purpose:Requests that a file should be buffered.
Inputs: pathname - Pointer to path to file that will need accessing. offset - offset from start of file to read from (included here so we don't always buffer from the start of the file when not needed)
Returns: handle, or < 0 to indicate failure
int bufseek(const int handle, const long offset)
Purpose: Sets the offset of the next byte to be read
Inputs: handle - handle as returned from bufopen offset - offset from start of file to read from
Returns: 0 to indicate success, or < 0 to indicate failure
long bufread(const int handle, const long size, unsigned char *dest)
Purpose: Copies data to the caller, blocking as required
Inputs: handle - handle as returned from bufopen, size - maximum number of bytes to be copied, dest - pointer to buffer to return data
Returns: the number of bytes copied, 0 to mark EOF or < 0 to indicate failure
long bufgetdata(const int handle, const long size, unsigned char **destptr)
Purpose: Makes data available to the caller, blocking as required. No data is copied.
Inputs: handle - handle as returned from bufopen, size - minimum number of bytes to be made available, destptr - pointer to pointer to data
Returns: the number of bytes available (with *destptr updated to point to the data), 0 to mark EOF or < 0 to indicate failure Note that there is no guarantee how long that the data will remain valid.
int bufclose(const int handle)
Purpose: Indicates that access to a file is no longer required
Inputs: handle - handle as returned from bufopen
Returns: 0 to indicate success, or < 0 to indicate failure
It may be useful to incorporate the existing buffer_init() and buffer_alloc() functions into this API. Access to the global variables audiobufend and audiobuf should become read-only via this API (or, later, removed). There are some places (tagcache.c for example) that make temporary use of the audio buffer without using buffer_alloc(); these will need work. Of course, we are not providing a malloc()/free() model here!
Currently there are callback functions supplied by playback.c to indicate when a file is buffered or unbuffered. These are (currently) only used by tagtree.c to update run-time statistics. These should NOT be provided by the buffering API as they are actually related to playback, rather than buffering - some work may be required here.
As well as the API, which may be called from various other threads (so may need to consider mutexing or very careful construction), there will be a a buffering thread that aims to make best use of the buffer to contain data for expected calls to bufread().
For simplicity, the initial implementation should be a straight port of the current playback scheme. This uses a ring buffer (including a guard buffer) and allows any file to be buffered in one piece only, with a fixed limit of the number of files that can be buffered. Currently the playback code will buffer up to 32 tracks so we must support at least 64 files (one audio plus one codec per track).
Note that this development is nothing to do with Metadata-On-Buffer, but it should make development of that rather simpler.
Anyone who'd like to help design the implementation is invited to add notes here.
The first code version has been scratched as I'm 99% sure it was completly useless. My current version is running as a standalone app now at svn://jdgordon.mine.nu/mob if anyone wants to make testing have a look, both mine and Nico's code is there. (email me ( muchJdGordon?easier untill we get )) if you want access to a stage where help out -- we can think about getting it into the core. JonathanGordon - 02 Jul 2007
I'm going in little steps, and I think the attached code is a good base. It seems to be able to buffer a list of files and "read" them back successfully. The debug output suggests that anyway. (read was in "" because it doesnt read anything yet, it simply moves pointers.) next step is to do proper reading to dump the buffer and make sure everything is working fine.
Because this is all in a single thread it obviously works slightly differently to what the final version will look like, but atm if there is room in the buffer a track is added, if there isnt then the "playback" part reads data from the open handles.
1 problem I thought of which I dont belive has been brought up before is what to do if a handle hasnt buffered the data that was requested? my test code only reads in one direction so this shouldnt happen, but will need fixing well before this is finished.
UPDATE I have it almost working correctly. I have it buffering 9 tracks (~63mb), and reading them back and writing the data to a new file and only 1 track has a different md5 from the origional set. the one track is the one where the data wraps the buffer end. update 2 bah, there is a problem somehwere and im not sure where.. adding a few more tracks so there is 148mb to buffer causes only 4 tracks to be correct...-- JonathanGordon - 01 Jul 2007
Please shoot these proposals to bits, or at least point out weaknesses and suggest improvements.
The current code actually copies the data from the audio buffer into a given pointer instead of just setting data to a pointer in the audio buffer, this is so files can wrap around the end of the ring buffer (probably other reasons also). -- JonathanGordon - 14 May 2007
You mean in codec_request_buffer_callback()? We could have a conventional "long bufread(const int handle, const long size, unsigned char *dst)" but there may be cases when copying isn't required. -- SteveBavin - 15 May 2007
Wouldn't it be better to completly ignore the current implementation (except the codec interface for simplicity) and start the buffering/playback code from scratch using the new API? This way we dont have bugs appearing later on because of possible hacks which need to be added for it to work with the current code? -- JonathanGordon - 14 May 2007
The buffering/playback code will effectively be redone to implement this proposal; there should certainly be no need for "hacks" (unless this API is incorrect). -- SteveBavin - 15 May 2007
And on that note, I think we should even talk about how to make MoB work with this now so its not hacked on later. -- JonathanGordon - 14 May 2007
It would be useful to consider how this will work with MoB, but I see this as a very general (and dumb) service that handles disk reading regardless of the file content. For MoB I assume we would parse all metadata information directly from the buffer, so the "original file format" would be a sensible format to have in memory. -- SteveBavin - 15 May 2007
That said (and remembering we dont want malloc()) would a buf_tempalloc(size_t size) function be acceptable? Depends how its done; I'm scared adding this would cause nasty memory fragmentation... Unless we have this sort of thing for each track in memory (this only works if the whole track fits though, but could work as a base)... |codec (unless codec is in memory or buffer already)|MoB buffer|file buffer ....|...| this shouldn't fragment memory if once the track file is closed the codec and MoB are freed up. I guess the file buffer part would be overwritten if the file doesn't fit in the buffer, but it will need a better way of knowing when to free RAM than bufclose(). -- JonathanGordon - 14 May 2007
The memory layout should (initially, at least) be identical to the current playback code. A caution - don't think of this in terms of buffer allocation. For example, bufclose() doesn't need to "free" anything, and a particular file's data may be unbuffered before the corresponding bufclose() is called if the implementation decides there is other data which will be needed more urgently. -- SteveBavin - 15 May 2007
On the 24th May 2007 there was an interesting conversation on the IRC channel about this proposal and Metadata On Buffer : IRC log of the discussion -- NicolasPennequin - 27 May 2007 another link to the conversation
r14 - 02 Jul 2007 - 01:35:07 - JonathanGordonRevision r14 - 02 Jul 2007 - 01:35 - JonathanGordon
Revision r13 - 01 Jul 2007 - 11:44 - JonathanGordon
Copyright © by the contributing authors.