Rockbox

  • Status Closed
  • Percent Complete
    100%
  • Task Type Bugs
  • Category Music playback
  • Assigned To No-one
  • Operating System SW-codec
  • Severity High
  • Priority Very Low
  • Reported Version Daily build (which?)
  • Due in Version Version 3.0
  • Due Date Undecided
  • Votes
  • Private
Attached to Project: Rockbox
Opened by Peter D'Hoye - 2006-11-11
Last edited by Michael Sevakis - 2007-07-31

FS#6317 - Lockup if stopping while still seeking to resume point

Hitting stop while still reading the file to reach the resume point causes a lockup.

1) Play a fairly large WAV and seek quite far into
stop
start playback using ‘play’ to resume where left
press stop again before playback resumes (= while reading the file to rewach the resume point)

backlight thread still running but paperclip required to get a usable player again.

Observed on h340 but probably an issue on al SWCODEC

Closed by  Michael Sevakis
2007-07-31 11:13
Reason for closing:  Fixed
Additional comments about closing:  

Fixed is as fixed does. I thought I closed this one.

Nils Wallménius commented on 2006-11-12 13:34

This appears to be related to  FS#6129 

Peter D'Hoye commented on 2006-11-13 07:27

so it seems... *looks at pondlife*

Steve Bavin commented on 2006-11-13 07:30

Don’t look at me, I only reported the problem! ;-) In fact, I didn’t even do that - Deb did, but I filed it on Flyspray so it wouldn’t get lost.

I am able to reproduce this, maybe will have time to look into it this week...

Mark Arigo commented on 2006-11-17 18:59

Can reproduce on my H140 with a big flac file, but not the sim (bummer).

The audio thread is getting stuck in an infinite loop in audio_stop_codec_flush(), specifically the loop

  while (audio_codec_loaded)
      yield();

It’s easy to see on the remote if you put a logf with a counter in there.

So, I guess the audio thread is waiting for the codec thread to stop, but the codec thread is still waiting for the audio thread to finish buffering. It looks like the audio thread has buffered some data, however, I’m not sure why control is not returned to the codec thread, but instead the audio thread continues to process the stop request.

Mark Arigo commented on 2006-11-17 20:12

Ok, here’s some logf dumps if anyone wants to take a look. One for a successful resume and one that locks up the player when stop is pressed during the rebuffer (I got that one by calling logfdump after 100 iterations in the infinite loop).

The sequence of events are as
Start
Play track (150MB
Seek far enough to cause a rebuffer &
Stop (return to file
Press play to resume
For locked up case: Press stop while buffering (don’t know if the timing is important here...I waited until I saw the wps and then pressed stop a couple of
For ok resume case: wait for hdd activity to stop, then logfdump.

Note: there are a couple of extra logf’s added here and there.

Michael Sevakis commented on 2006-11-17 23:55

It’s conceivable that the current codec could finish and the next audio codec could start loading before audio_codec_loaded is checked again. If a message is buffered queue_wait doesn’t yield and neither does mutex_lock if the mutex isn’t taken so audio_codec_loaded will be always seen to be true even if false for a brief period. Just my 0×02.

EDIT: ...though I have an idea of using a counter instead of a bool so every time an audio codec loads it increments the counter by 1 and you’d be able to detect this state. Simply adding yields somewhere is still a race condition.

EDIT2: Having the counter start at zero and increment when loading a codec and increment a again when unloading would make it even when no codec is loaded and odd when a codec is loaded and other checks can simply be if (encoder_version & 0×1) instead of if (audio_codec_loaded) but the aliasing problem would be nonexistent too.

Michael Sevakis commented on 2006-11-18 04:25

Well, forget all my BS. :P That doesn’t stop the problem even though there’s a possibility of what I described.

Mark Arigo commented on 2006-11-18 06:01

I don’t think it’s codec loading/unloading timing. It’s just that the codec thread queue_waits for the rebuffer to finish, but somehow the audio thread processes the stop event before notifying the codec thread that it’s done buffering. Thus, the code path never returns to the codec. If it did, there are checks to see if playback has stopped.

I think there may be some confusion in the buffering process for flac files. When a flac files resumes at a long offset, it must be buffered from the beginning in order to get the stream info & seek tables and then immediately rebuffered to the proper seek point. I think maybe the stop is coming between the initial fill buffer request and secondary seek & rebuffer. That seems to be what the logf dump is indicating for the lockup case (look at when Q_AUDIO_REBUFFER_SEEK is queued).

If that’s the case, maybe a solution is to make codec_seek_buffer_callback return “busy” while the audio thread is filling and make the codec do a yield loop until it’s ready.

Michael Sevakis commented on 2006-11-18 07:03

Works on a WAV too every time. It’s clear the codec thread is locked somewhere too since it should exit the entry point and change the counter. It became apparent it is not and is likely stuck in a callback like you said.

Michael Sevakis commented on 2006-11-18 07:34

Ok, if Q_AUDIO_REBUFFER_SEEK is posted to the audio queue in codec_advance_buffer callback while the audio thread is in a loop not dequeueing messages and the codec waits on the codec_callback_queue but the audio thread is what posts messages there, you have a nice complex deadlock. The codec thread cannot take a path to exit the callback and return and release the audio thread from its loop. Viola!

Michael Sevakis commented on 2006-11-18 08:37

If I post Q_AUDIO_STOP (an unambiguous message that ensures ci.stop_codec is true) to the codec callback queue and call queue_remove_from_head to ensure that the message doesn’t linger in the queue if nothing was dequeueing at the time, the problem goes away.

Michael Sevakis commented on 2006-11-18 08:46

The problem with doing that though is audio is stopped when doing a skip but it at least shows that the problem is a deadlock because of the wait on the callback queue for certain.

Michael Sevakis commented on 2006-11-18 09:02

Posting Q_CODEC_REQUEST_COMPLETE before the loop in audio_stop_codec_flush gives much better results and what seems like a good fix. I still dequeue the message after the loop to make sure it doesn’t abort later if there was no dequeue about to happen. If there’s any pitfalls to doing that perhaps a special message should be used.

Michael Sevakis commented on 2006-11-18 10:49

Check out and test drive...

Mark Arigo commented on 2006-11-20 15:11

Should probably post Q_CODEC_REQUEST_FAILED so the codec exits immediately.

Just thinking out loud...shouldn’t rebuffer_seek be able to abort the current fill processes? It’s very inefficient in these cases to fill the buffer twice. Or maybe we need a function like audio_fill_buffer_exact(Nbytes) that reads at most Nbytes onto the buffer. Flac (and wav?) could use that function for the initial buffering of the stream info & seek table.

Peter D'Hoye commented on 2006-12-09 21:19

yup, this patch fixes the issue

Michael Sevakis commented on 2006-12-10 01:00

It’s not a proper fix so that’s why I haven’t committed it. The audio thread can still get spurious messages after the stop and the codecs do not behave in a consistent way since not all of them check return values from the callbacks. Will have to fix any that do not (wavpack.codec comes to mind). You’ll probably get codec failures later after the stop during seek since the audio thread will still receive messages posted before the codec waited on the private queue.

One thing I’ve wondered about but haven’t tested (off
happens if you have a codec on the buffer followed by several files that use that codec and the first file is invalid and causes a codec failure? Do the remaining files play or die?

Michael Sevakis commented on 2007-01-01 06:51

Working on this fix when simplifying playback. :)

Deb commented on 2007-03-27 03:47

Any update on this or when this going to be addressed?

Michael Sevakis commented on 2007-05-12 00:11

Should be addressed as of a few days ago from this post’s date. I got sick of it still hanging around...no pun.

Deb commented on 2007-05-17 01:52

it solved this problem but created another bug. now when i the player is plugged in my car with a car adapter (car adapter mode is on), player freezes when i stop the car and the player goes into pause mode.

Peter D'Hoye commented on 2007-07-31 07:41

What’s the status on this, Mike?

Loading...

Available keyboard shortcuts

Tasklist

Task Details

Task Editing