Rockbox

  • Status Closed
  • Percent Complete
    100%
  • Task Type Patches
  • Category Music playback
  • Assigned To No-one
  • Operating System All players
  • Severity Low
  • Priority Very Low
  • Reported Version
  • Due in Version Undecided
  • Due Date Undecided
  • Votes
  • Private
Attached to Project: Rockbox
Opened by pondlife - 2006-09-14

FS#5992 - Reworked playback.c to fix voice bugs

This is a fairly major reworking of playback.c/pcmbuf.c, to simplify the code and fix crashes which occur when playback stops naturally while using voice and when skipping tracks quickly.

Having removed quite a few wait loops and a few special cases, I quite expect it will introduce more problems ;-)

Please can you test this and report back with recipes for any problems!

Closed by  pondlife
2006-09-18 11:16
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

Well, it\'s in there and out there now…

pondlife commented on 2006-09-14 12:17

And here’s the patch!

(application/octet-stream)    playbackvoicefixed.patch (22.7 KiB)
pondlife commented on 2006-09-14 15:58

Bug: Pressing stop during a crossfade can leave the CPU boosted (due to PCMBUF.C). I’ve noticed this has been reported before the rework (http://www.rockbox.org/tracker/5520), so hopefully it’s not a backwards step!

lostlogic commented on 2006-09-14 21:47

Ahhh, you fixed what was broken recently after my fixing it previously with the track display being all funky and possibly out of sync. Thanks. (Still reading the rest of the patch)

lostlogic commented on 2006-09-14 21:54

Code wise, all of your changes make sense to me. One of the stylistic only changes in pcmbuf.c addes a few bytes to the code size, but I won’t make too much noise about that. We’re not coding for archoses.

lostlogic commented on 2006-09-14 22:05

Something I just noticed, and I’m not sure if this is from these changes or previously is that it takes longer for the WPS to initially display than previously. Perhaps something to do with when you set playing?

pondlife commented on 2006-09-15 08:35

Hi Brandon - many thanks for checking it over.

I had to move all of the state transitions inside the queue reading thread to fix a stop/skip race condition (at least I think that’s what it was).
Maybe we need a new wps_playing variable to allow the WPS to be updated faster?
I’ll add some comments next time round to the meanings (as I see them) of playing, voice_is_playing, audio_codec_loaded and voice_codec_loaded. There was (and likely still is) a little confusion regarding the these variables.

lostlogic commented on 2006-09-15 18:02

Yeah, that’s why I had moved them into the queue in the first place. It’s where they belong. A major part of my rework from this spring was to tighten down the threading model so that the right threads do things always.

A wps_playing variable might make sense. I think that some combination of other variables used to indicate this state, but simplification can come _after_ functioning.

pondlife commented on 2006-09-17 16:38

Hmm, I see this has now been committed (:-o) so I hope it works ok; I didn’t consider it quite ready for prime time yet (and won’t have much time this week to work further…). The good news is I’ve been running it much of the weekend with no problems, in my configurarion at least!

p.s. Maybe we need a marker on Flyspray patches to say whether work-in-progress or not?

bluebrother commented on 2006-09-17 18:25

Steve, the “Percent Completed” entry could be used for that IMO. If this patch has been committed completely, shouldn’t it get closed? Or was only a part of it applied?

Loading...

Available keyboard shortcuts

Tasklist

Task Details

Task Editing