• Status Closed
  • Percent Complete
  • Task Type Patches
  • Category Music playback
  • Assigned To No-one
  • Operating System SW-codec
  • Severity Low
  • Priority Very Low
  • Reported Version Release 3.4
  • Due in Version Undecided
  • Due Date Undecided
  • Votes
  • Private
Attached to Project: Rockbox
Opened by Blue_Dude - 2009-10-29
Last edited by Blue_Dude - 2009-11-01

FS#10739 - Playback.c refactor

This is an initial refactoring of playback.h and playback.c. This first attempt separates the codec thread from the audio thread. There are some similarities in a shared common codebase, mostly a few common variables. It compiles cleanly for a couple of targets and there’s no apparent harm, but this project needs a fresh set of eyes.

Other than fixing a couple of small items, there should be no change. That comes later. ;)

Closed by  Blue_Dude
2009-11-01 00:13
Reason for closing:  Accepted
Additional comments about closing:   Warning: Undefined array key "typography" in /home/rockbox/flyspray/plugins/dokuwiki/inc/parserutils.php on line 371 Warning: Undefined array key "camelcase" in /home/rockbox/flyspray/plugins/dokuwiki/inc/parserutils.php on line 407

Committed as r23444

Here’s a much cleaner version. It removes the common code files and uses a SVN copy to produce the new codec_thread files. This gives a much clearer sense of what has been changed.

Bug fix. Missing #defines etc.

I’m not sure how relevant this is to your work, but could you change the way codecs wait for data to be ready and start decoding? (the “while (!*ci→taginfo_ready && !ci→stop_codec) ci→sleep(1);” part that is in every codec)

This should definitely use wakeups (or queues if you don’t want the increased binsize) instead of the busy-looping, but I’m not sure if this applies to your rework. If not, could you try generalizing the part that sets ci→stop_codec and ci→taginfo_ready, so it’ll be easier for someone to change it to wakeups/queues later on?

To apply this patch cleanly, you should make subversion copies of playback.c and playback.h. Use: “svn cp apps/playback.c apps/codec_thread.c” and “svn cp apps/playback.h apps/codec_thread.h”. This will give the new files something to patch to.

mcuelenaere: with queues there will be some sort of looping anyway, something like “while(queue_empty) yield()”, or even “while(queue_empty) queue_wait_tmo()”. Maybe that’s not the same thing. Anyway, I’m not there yet. I’m trying to do a coarse code sort before starting refinements.

Does it? Doesn’t queue_wait() block/sleep indefinitely till someone posts a message on that queue (which wakes up the thread)?

If not, wakeup objects should be used (which definitely have that behaviour).


Available keyboard shortcuts


Task Details

Task Editing