Rockbox

This seems to be another manifestation of the issue causing  FS#2687 ... I think the problem is that you shut down the player in the middle of a track transition.

I think it is too, a fairly old build that I use at the moment doesn't exhibit this behaviour so I might download that revision and examine it. It's revision 15812 if you're interested...

Oops, tell a lie, it's 13742

Does it still happen with 15841 or later?

Unfortunately yes, the resuming problem still exists. I have noticed however that simply stopping at the end of the song and resuming playback will also cause this problem.

What I did notice though, is that in apps/playback.c, changing

return &prevtrack_id3;

to

return &curtrack_id3;

on line 525 fix this problem, but the WPS information changes prematurely. I only noticed this because I'd been tinkering with the source code...

The external interfaces to playback.c generally use the "WPS track" rather than the "codec track" transition point and resulting track name and offset (which is a good thing for keeping the UI correct).

audio_current_track() does this, as do the calls to playlist_update_resume_info(). However, when resuming we have an offset passed in to audio_play which is assumed to be based on the "codec track". I can't think of a way for audio_play() to convert a "WPS offset" into a "codec offset", which would be more correct.

Perhaps instead we should base the resume point on the "codec track/offset"?

One other point - shouldn't codec_get_file_pos() should use the "codec track" (i.e not rely on audio_current_track(), maybe just curtrack_id3 would do)?

OK, I've been having a root through the code and have more or less found the problem (IMHO). When the playback enters the transition phase, the playlist index is incremented which means that if you press stop during this period then the resume index will be off by 1. This basically means it'll play the following track using the current track's offset which on the simulator can cause segfaults.

I did code a solution by passing in the transition state into playlist_update_resume_info, but it was messy and hacky. I guess I could add another variable to the mp3entry structure and then tell playlist_update_resume_info in playlist.c to use the previous, rather than the current index of the playlist when saving the resume info.

Something like:

int playlist_update_resume_info(const struct mp3entry* id3)
...
global_status.resume_index = id3->transition == 1 ? playlist->index - 1 : playlist->index;
global_status.resume_offset = id3->offset;
status_save();
...

Bit of a mess though

OK, here's a patch based upon the pseudo code I wrote. It appears to work OK but I really need to test it more.

Why did you introduce a new field in the mp3 struct? IMHO the transition status is something that is 'global' and doesn't belong to a particular track. I.e. it should be a global state var. I don't know where it's placed best though.

I'd prefer the transition to be entirely hidden. The playlist->index shouldn't be incremented until the transition is completed (i.e. the WPS has moved on rather than when the codec has moved on). Similar should be true for all variables accessible outside of playback.c.

That sound you hear is a can of worms being opened. ;)

True, it should probably be a global variable instead, I'm still trying to get my head around the code at the moment. Steve's suggestion of making the playlist increment would probably solve the related problem of "rewinding at the end of a song will rewind the following song rather than the current" as well.

I'll have a look into making the variable global instead.

"transition" should NOT be global, it should be only available within playback.c. Where it's used elsewhere, there's probably playback API that could be used instead (e.g. to get the current track).

I once tried moving the playlist index incrementation to after the track transition (it's in the SVN history), but it broke auto folder advance. I guess we could go back to what I had done and then fix the folder advance.

Go for it! ;)

This patch is an attempt to handle the playlist updating at the right time, i.e. after the track transition for most skips, but before it for manual skips or automatic dir change skips.
I haven't checked if it solves the actual issue described by this task, but I think it will.

Thanks for the patch, I'll have a look at it when I get in after the pub^H^H^H^H^Hwork

I just tested and the original issue looks fixed. Please confirm and I'll commit :)

Cool, this appears to have solved the problem, I've only tried it on the simulator but I will put this on my iRiver H120 shortly and see how I get on.

Actually, it looks like there is a problem with the playback. While I was playing an album it played the same song twice after a short hesitation. It only did it once but I'll test it on the simulator and see if I can make it do it again.

Anything new here?

I haven't managed to make it play the same track again so I'm not sure what the problem was. The only other issue I noticed was that stopping during the transition and restarting will resume playback from before where the transition started, or maybe a couple of seconds before that, but it doesn't happen on the simulator (I'm using an iRiver H120). Apart from these two issues I haven't seen anything where the playback corrupts or crashes.

Well I guess I'll commit and wait for the bug reports :)

Changing playlist_check() had undesirable side-effects. This new version takes a slightly different approach.

I've seen a problem similar to this, I added my comment (copied below) to  FS#6215 .

The reason I think I might be seeing this problem is because after listening to a podcast, I will typically stop playback while the host is making his closing remarks, a few seconds before the end of the file. Then I delete the podcast via rockbox and power off. On power on, rockbox tries to resume the podcast and hangs.


-------------------from  FS#6215 :

I see this frequently with podcasts- I will listen to one, delete it via rockbox, and on the next power-up it hangs when trying to resume the file. It looks like it is selecting the next file, but resuming at the position of the last (deleted) file. In the case of podcasts, the new file might be significantly shorter than the previous, maybe trying to resume far enough after the end of a file causes a hang?

The only way I've found to fix this once it occurs is to delete the playlist control file from a PC connection (reset doesn't help).

Player is a H320. I've seen this problem on multiple builds over the last 3-4 months. Currently running r15989-080103.

e200 + r17153 = The first time it displayed a lovely cabbiev2 smear across the screen and required a hard shutdown (something I saw randomly earlier today). The second time it did as described in the original report (playing next track 2/3 the way through).

e200 + r18013 = Still happening.