Rockbox

Tasklist

FS#10614 - Deleting files in USB mode can confuse resume playback/bookmark functionality.

Attached to Project: Rockbox
Opened by Alex Bennee (ajb) - Wednesday, 23 September 2009, 09:13 GMT
Task Type Bugs
Category Music playback
Status Unconfirmed
Assigned To No-one
Operating System All players
Severity Low
Priority Normal
Reported Version Daily build (which?)
Due in Version Undecided
Due Date Undecided
Percent Complete 0%
Votes 0
Private No

Details

Steps to reproduce:

1. Start listening to a series of podcasts, say 1-5
2. After listening to >1 plug player into computer during playback of subsequent podcast
3. Delete the podcasts you have already listened to (not what your currently listening to)
4. Eject/Unplug player from computer
5. Attempt to resume playback

This fails for both resume and Recent Bookmarks functionality. I assume this is somehow tied in with playlist support as files from the playlist have been deleted.

Where does Recent Bookmarks keep it's information? Should the bookmark also keep the individual file referenced so it can resume in this case?

Tested on R21184M-090604

Will test on latest version once my main machine is back on internet.
This task depends upon

Comment by Kory Postma (Kory) - Friday, 25 September 2009, 00:26 GMT
Yes this problem exists when I listen to the daily Alex Jones radio show. I delete the older ones and the newer one will not Resume Playback.

I investigated the code and here is the problem:

The resume info somehow gets loaded into global_status and the resume happens in apps/root_menu.c in the wpsscrn function. There is even a DEBUGF statement there. As you can tell it only gets the resume_index and resume_offset. What it really needs to do is to check the filename against the one stored in the bookmark or wherever. If the index is not the same file then it will need to get the proper index and then play the proper file.

This bug has been irritating me for months and I have been hoping to find a solution for it but I have no idea how global_status gets loaded. Does someone know? I've looked all over in the code but I can't see how the global_status struct is getting loaded.

Thanks,
Kory
Comment by Magnus Holmgren (learman) - Saturday, 26 September 2009, 10:29 GMT
Bookmarks do contain the name of the file. When resuming a directory bookmark, it looks for the file by name. A bookmark contains a resume index too, but for a directory bookmark, it is mainly used as an optimization; if first checks that index in the directory. So this case should work.

"Resume Playback" uses the playlist control file to get the filename, but in the case of directory playback, that only contains the name of the folder. As the resume index refers to an entry in that folder, it can pick the wrong file. This case isn't aware of bookmarks, and doesn't really know where to look for them (other than the recent bookmarks list, which might not contain the file of interest anyway).
Comment by Kory Postma (Kory) - Sunday, 27 September 2009, 04:29 GMT
Where is the playlist control file and how can one add the ability to "resume playback" based on filename instead of index?
Comment by Magnus Holmgren (learman) - Monday, 28 September 2009, 15:02 GMT
The file is "/.rockbox/.playlist_control" and contains information about the active playlist. To always resume based on filename, the first thing to do is to store the filename somewhere. One reason this hasn't been done is that some targets have small amounts of NVRAM, where certain (e.g., frequently changing) settings are stored (thus not requiring the HD to spin up when saving them). This NVRAM has room for a resume index/offset, but not a filename.
Comment by Kory Postma (Kory) - Monday, 28 September 2009, 23:16 GMT
So maybe we should add support for those targets that can support it and then mention those targets that are affected by this and for the reason you stated. Where would be the best place to store the filename?
Comment by Kory Postma (Kory) - Saturday, 17 October 2009, 14:25 GMT
I have a solution, finally!

Here is what you need to do:
In the apps/bookmark.c file move this line to the apps/bookmark.h file after the #include line
#define RECENT_BOOKMARK_FILE ROCKBOX_DIR "/most-recent.bmark"

So what you are doing is removing this line from the .c file and moving it into the .h file so that root_menu.c will be able to access it as well.

Open the root_menu.c file and find the wpsscrn() function.
static int wpsscrn(void* param)

Find this block of code in that function:
if (playlist_resume() != -1)
{
playlist_start(global_status.resume_index,
global_status.resume_offset);
ret_val = gui_wps_show();
}


Replace it with the following:
if (playlist_resume() != -1)
{
bookmark_load(RECENT_BOOKMARK_FILE, true);
ret_val = gui_wps_show();
}

Then just do a make and then a make zip or a make fullzip and copy that onto your device. This works for me on a Sansa e200 series player.
Comment by Magnus Holmgren (learman) - Sunday, 18 October 2009, 10:16 GMT
That means you're resuming the most recent bookmark. Which could be the most recently played track, but it doesn't have to be. It all depends on the bookmark settings and actions of the user. And if recent bookmarks are disabled, it wouldn't work well at all...

If you do use recent bookmarks, and always save bookmarks (for the relevant files), then it is simply a matter going to the main menu, select "Recent Bookmarks" and then the first entry. Not quite as easy to use as "Resume Playback", but for me it only requires two button presses (as I have the main menu as the start screen).
Comment by Kory Postma (Kory) - Sunday, 18 October 2009, 12:43 GMT
Yep, I always save bookmarks and I'm sure most others that complain about this issue do as well. Anyhow, it has annoyed me for a long time so this work-around works for me and may help a few others at least until they get it fixed.
Comment by Richard Dymond (rjdymond) - Monday, 18 January 2010, 21:03 GMT
I have noticed that 'Resume Playback' resumes the wrong file after *adding* files in USB mode, too. I will have to see whether this bookmark-based workaround is OK. (I'm a new user and I'm not sure how bookmarks work yet.)
Comment by Jeffrey Goode (Blue_Dude) - Friday, 09 April 2010, 20:15 GMT
I have partially fixed this problem. You can now resume a bookmark even if the index is incorrect, e.g. you've deleted some files from the directory and you're trying to resume one of the remaining files. The resume playback function still needs work, but I think that one might be beyond help if you go behind the playlist engine's back and delete files it's expecting to see.
Comment by Kory Postma (Kory) - Friday, 09 April 2010, 22:43 GMT
Can you explain what you did to fix the problem? What files you had to change and what not?
Comment by Jeffrey Goode (Blue_Dude) - Friday, 09 April 2010, 22:59 GMT
I made a slight modification to tree.c in r25558. play_bookmark in tree.c takes some of the bookmark info and attempts to find the correct resume point for directory play (as opposed to playlist) by searching for the file at the specified index. If it's not found at that index, it's supposed to search the rest of the directory to find it. But in the case where files were deleted, it would search for a file at that index and return NULL if there weren't that many files in the directory. Then the function would give up and return rather than search from the beginning. I simply made sure it started searching from the beginning rather than give up too soon. Only if the directory is empty would it give up that easily. Now the index is used for its intended purpose, i.e. make it quicker to get started if no changes are made, but it's no longer essential that it be valid to play the bookmark.

Resume playback uses a different function to resume playback and relies on the index being valid. Perhaps the resume point could be stored much as a bookmark is stored, with filename and directory, vs. just the playlist position. Or even ensuring that the dynamic playlist somehow stays in sync with file additions and deletions. That's a more extensive rework. It may not even be worth the effort since bookmarks behave themselves now.
Comment by Kory Postma (Kory) - Friday, 09 April 2010, 23:36 GMT Comment by Jeffrey Goode (Blue_Dude) - Friday, 09 April 2010, 23:43 GMT
And also: http://svn.rockbox.org/viewvc.cgi/trunk/apps/tree.c?r1=25558&r2=25560 . Fixed a bug since the last comment.

Loading...