FS#11776 - Metadata read log

Attached to Project: Rockbox
Opened by Jonas Häggqvist (rasher) - Friday, 26 November 2010, 02:48 GMT
Last edited by MichaelGiacomelli (saratoga) - Saturday, 11 December 2010, 04:31 GMT
Task Type Patches
Category ID3 / meta data
Status Closed
Assigned To No-one
Operating System All players
Severity Low
Priority Normal
Reported Version Rbutil SVN
Due in Version Undecided
Due Date Undecided
Percent Complete 100%
Votes 0
Private No


This patch adds a menu entry to the debug menu which will allow the user to toggle writing of a log of files being read for metadata in /metadata.log. The filename is written before Rockbox attempts to read metadata from the file, so if Rockbox crashes when scanning for the DB, the last filename in the log should be the file that caused the crash. This should greatly help debug metadata reader problems.
This task depends upon

Closed by  MichaelGiacomelli (saratoga)
Saturday, 11 December 2010, 04:31 GMT
Reason for closing:  Accepted
Additional comments about closing:  Committed in r28789.
Comment by Jonathan Gordon (jdgordon) - Friday, 26 November 2010, 03:08 GMT
mentioned in irc but incase it gets forgotten or someone else says the same thing...
1) fd == 0 is valid so that needs to be changed
2) the 2 write()'s should really be a snprintf() and a write() (or just fprintf() if we have that). putting a MAX_BUF array on the stack is no big deal here (get_metadata() shouldnt be being called recursively)
Comment by Jonas Häggqvist (rasher) - Friday, 26 November 2010, 03:10 GMT
Fix the fd check to >= 0 (0 is a valid fd)

Comments from IRC:
"I'd do snprintf() then write instead of two write()'s" - this way a) was simpler b) doesn't use more stack (probably not important)

"opening it every time isnt very nice either" - agreed, it's not nice, but it gets the job done and makes sure no potential buffering messes up if Rockbox were to close with the file open. Performance is unimportant - getting the right result is very important.

I'll try it out later with a target full of music to see how long it takes.
Comment by sideral (sideral) - Friday, 26 November 2010, 12:23 GMT
This patch does not work right in the simulator on any filesystem that supports permissions, as no file mode is provided in the call to open. The new log file is created without write permission, which prevents reopening the file after the first logentry was written.

The applied patch fixes the bug.
Comment by Jonas Häggqvist (rasher) - Friday, 26 November 2010, 14:27 GMT
Well this was not the case in the sim. I don't know where else it would be the case. Can't hurt though.
Comment by Jonas Häggqvist (rasher) - Sunday, 28 November 2010, 01:13 GMT
In my somewhat unscientific testing, building the DB goes from taking about 35 seconds without logging to about 55 seconds with logging (e200 with 1180 music files).

Not sure there's any reason not to commit this?
Comment by MichaelGiacomelli (saratoga) - Thursday, 09 December 2010, 20:33 GMT
IMO this should already be in SVN.
Comment by Jonas Häggqvist (rasher) - Saturday, 11 December 2010, 03:47 GMT
Feel free to commit it. I probably won't in the near future.