Rockbox

  • Status Closed
  • Percent Complete
    100%
  • Task Type Bugs
  • Category Plugins
  • Assigned To No-one
  • Operating System All players
  • Severity Low
  • 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 PaulJam - 2008-07-21
Last edited by dionoea - 2008-10-09

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

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.

Closed by  dionoea
2008-10-09 15:09
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

applied. thanks.

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.

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.

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).

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))

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.

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)

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. :)

can you resync the patch? it doesnt apply cleanly and its trying to remove a line which is conufinsg me….
looks good otherwise

Ok, here’s my patch again, sync’d to r18308. Apparently splash() was changed to splashf(), and this broke my previous patch.

Loading...

Available keyboard shortcuts

Tasklist

Task Details

Task Editing