Rockbox

Tasklist

FS#9209 - md5 sum generation can't be interrupted.

Attached to Project: Rockbox
Opened by PaulJam (PaulJam) - Monday, 21 July 2008, 19:41 GMT
Last edited by Antoine Cellerier (dionoea) - Thursday, 09 October 2008, 15:09 GMT
Task Type Bugs
Category Plugins
Status Closed
Assigned To No-one
Operating System All players
Severity Low
Priority Normal
Reported Version Daily build (which?)
Due in Version Version 3.0
Due Date Undecided
Percent Complete 100%
Votes 0
Private No

Details

It is currently not possible to interrupt the md5 sum generation of the md5sum plugin with a button. If you open a large folder or launch the plugin from the apps menu it can render the device unusable for several hours (depending on the number of the files).
The only way to stop it is to shut down or reset the player.
This task depends upon

Closed by  Antoine Cellerier (dionoea)
Thursday, 09 October 2008, 15:09 GMT
Reason for closing:  Accepted
Additional comments about closing:  applied. thanks.
Comment by PaulJam (PaulJam) - Monday, 21 July 2008, 19:50 GMT
Little correction: Shutting down doesn't seem to work while the plugin is running, so the only way is a hard reset which can lead to file system corruption.
Comment by Christopher Williams (christop) - Thursday, 31 July 2008, 15:27 GMT
Here's a patch that adds the ability to quit by pressing the "quit" button as defined for the platform (such as the power button on Sansa e200). Note that it checks for the button after it finishes hashing a file, so with large files it may take a few seconds to quit. If this is unacceptable to you, either deal with it or fix it yourself! :)

Also, I added an rb->yield() to hash_file() so the plugin yields after each file. Feel free to remove this if you think it's unnecessary.
Comment by Christopher Williams (christop) - Thursday, 31 July 2008, 16:04 GMT
Ok, scratch that last patch. I obviously didn't test it. (The problem: I compared the button value to PLA_QUIT, which is not actually a button value) :/

I went ahead and made the plugin check for user quit while it hashes a file so it can respond to the user's request much quicker. This way the user won't be tempted to hard reset their player with really big files. I also fixed a bug in the open() call for opening the output file (it didn't truncate the file before writing to it).
Comment by Bertrik Sikken (bertrik) - Friday, 01 August 2008, 08:31 GMT
I fully agree on the need for an abort mechanism for the md5sum plugin (at least when doing the whole disk).

Instead of defining a quit key for every keypad configuration, can't we just use a default cancel key? Something like:
if(ACTION_STD_CANCEL == get_action(CONTEXT_STD,TIMEOUT_NOBLOCK))
Comment by Christopher Williams (christop) - Friday, 01 August 2008, 19:54 GMT
Bertrik: That's a good suggestion. Now that I figured out how to use get_action(), here's another patch that uses get_action() instead of button_get(). I also added some code that fixes a "bug" with the path name when the root directory is being hashed (it currently adds a slash after the root "/" so paths ended up looking like "//foo/bar" in the output file).

As a side, I noticed that occasionally when I quit some plugins, the Plugin > Applications menu doesn't respond to any button until I press a "cancel" button again (either left or power). Perhaps the button presses or "actions" are not being delivered to the correct queue, though this is only a guess as I don't really understand how events and such work in Rockbox.
Comment by Bertrik Sikken (bertrik) - Friday, 01 August 2008, 21:25 GMT
Christopher, maybe it's just my imagination but it seems to be a bit slower now. Checking every 500 bytes may be a bit too much, I would use a bigger buffer. I think we can safely use a 16 kB buffer (32 kB is the upper limit for the target with the smallest plugin buffer IIRC), but we should not allocate that on the stack as is done now.

You're right about the key response after a exiting a plugin, it's a more or less known problem but it's not clear what causes it yet, see also  FS#9209  (http://www.rockbox.org/tracker/task/8816)
Comment by Christopher Williams (christop) - Friday, 01 August 2008, 23:44 GMT
Ok, try this out for size. It uses a static 16 kB buffer (not on the stack), so it's noticeably faster (maybe up to 2x faster), and I also cleaned up the "quit" logic (no more userquit() function).

It looks like FS#8816 will give me something else to try to fix now. :)
Comment by Jonathan Gordon (jdgordon) - Sunday, 17 August 2008, 10:31 GMT
can you resync the patch? it doesnt apply cleanly and its trying to remove a line which is conufinsg me....
looks good otherwise
Comment by Christopher Williams (christop) - Sunday, 17 August 2008, 18:13 GMT
Ok, here's my patch again, sync'd to r18308. Apparently splash() was changed to splashf(), and this broke my previous patch.

Loading...