From: Chris Holt <amiga2k_at_cox.net>
Date: Sat, 1 Feb 2003 09:21:12 -0500

Oh good, I thought it was just me! :)

Chris

----- Original Message -----
From: "Henrik Backe" <backe_at_swipnet.se>
To: <rockbox_at_cool.haxx.se>
Sent: Saturday, February 01, 2003 6:53 AM
Subject: Re: Latest Bookmarking Patch


> Ben,
>
> Could you please check the zipfile. Unzip reports that two bytes of
> ajbrec.ajz
> is corrupted and is probably correct because my recorder wont load it.
> You maybe forgot to upload in binary format to the website?
>
> By the way, auto bookmarking is the best feature since mid-track resume
> for us audiobook listeners. The only thing missing in rockbox for me now
> is queing on playlists (so I can que books of course).
>
> Since this was delurking time for me, a humungous thanks to all the
> developers.
>
> Henrik
>
> ----- Original Message -----
> From: "Benjamin" <mailinglists_at_samuraipanda.com>
> Newsgroups: gmane.comp.systems.archos.rockbox.general
> Sent: Saturday, February 01, 2003 2:32 AM
> Subject: Latest Bookmarking Patch
>
>
> Hi All,
> The latest version of my bookmarking patch (diffed off of rockbox-
> daily-20030131) can be found at http://sourceforge.net/tracker/index.
> php?func=detail&aid=669440&group_id=44306&atid=439120. A compiled
> version can be found at: http://www.samuraipanda.com/bookmarks-20030131.
> zip. Below are the changes in this version. Below that are changes
> that were requested and I didn't implement and my reasoning why.
> Below that is a list of TBDs/known issues.
>
> Please note that this version will not work with previous bookmark
> files. There is also a perl script included with the compiled version
> that will update a 11-field bookmark to the updated format. If you
> have problems updating bookmarks with it, send me a copy of the bookmark
> file and I'll look into it.
>
> Ben
>
>
> Changes:
> -----------------------------------------------------------------
> ---------
> 01. Autobookmarks are now stored in the first 4 bits of memory AF.
> You may need to reset them the first time you load this patch.
> 02. Bookmark functions(create/load) are now in their own menu off
> of the main menu.
> 03. Default Auto-Bookmark and Auto-Load settings are OFF.
> 04. Name the attribute constant in the same form as the already existing
> ones, i.e TREE_ATTR_BMARK
> 06. Bookmark format has been changed. Queue information has been
> removed and the resume file has been moved to the end of the bookmark.
> This breaks previous bookmark compatability, but I have written
> a perl script that should update existing bookmarks (included with
> the compiled version of this patch).
> 07. Bookmark uses a comma-separated syntax.
> 08. bookmark_create() was broken into two funcions. bookmark_write()
> writes the bookmark; bookmark_create() creates the formated string.
>
> 09. Now using a common read_line()
> 10. .bmark is now the official extension for bookmark files (subject
> to change).
> 11. Moved all strings to the english.lang file
> 12. strcpy() calls have been replace with calls to strncpy() calls
> and some code was modified to check for overruns. I probably missed
> others.
> 13. Indentation has been updated to 4 spaces and I tried to be more
> C89 compliant, just for Daniel :-).
> 14. I thought I had done it before and some may be lingering, but
> I ran bookmark.h and bookmark.c through a dos2unix utility.
> 15. On the select menu, bookmarks are now deleted using the ON+Play
> key combination (I kept hitting the off key to exit).
> 16. The MP3 title is no longer displayed on the bookmark select menu.
> This is because the new bookmark format does not store this information
> anymore. This may be fixed in the future.
> 17. Creating a bookmark now indicates if a bookmark was created or not.
> 18. A bunch of code changes I'm sure I forgot about.
> 19. Updated to the rockbox-daily-20030131 daily release.
>
> Changes not done, but requested, along with my reasons why they are
> not done.
> -----------------------------------------------------------------
> ---------
> 01. (Björn Stenberg Stenberg) The Auto Load settings should be removed.
> Use a single Auto Bookmark setting.
> I believe these should be two settings. The both occupy the
> first four bits of memory AF
>
> 02. (Björn Stenberg) Don't use the resume settings as temporary variables
> when you load bookmarks
> This implimentation is based off of the resume functionality
> all ready built into RockBox. Further, the global settings should
> be over written once a bookmark is selected so that a user can resume
> the next time they start the Rockbox up.
>
> 03. (Björn Stenberg) Don't assume the first bookmark in the file
> is the auto bookmark. Mark it specifically.
> I don't want to have to search through a file just to find the
> autobookmark. Further, I don't want to have to open the file and
> read all the way through to determine if there is an auto-bookmark.
> The design is such that if a bookmark file exists, auto-load will
> ask if the user wants to load the first bookmark which is normally
> reserved for the auto-bookmark.
>
> 04. (Björn Stenberg) Parse the bookmark file with strtok_r(), not
> manually byte-by-byte
> Each time I try to use strtok_r(), memory seems to be corrupted.
> It may be something I am doing wrong or a bug in strtok_r that
> my code is revealing. I have included a strok_r version of
bookmark_parse()
> called, surprisingly enough, bookmark_parse_strtok_r(). This fails
> to work when loading a bookmark directly from the .bmark file, but
> it will work if called as part of autoload.
>
> 05. (Björn Stenberg) Create a bookmark struct instead of passing
> 14 parameters to many functions.
> There is only one function that passes in 14 parameters,
> bookmark_parse().
> This data is then used to load a bookmark. Consolodating his into
> a structure would add more overhead to the code as this function
> also validates the bookmarks or returns selected information.
>
> 06. (Björn Stenberg) Fix the code to work on players too (max 2
> lines of text, different keys)
> I don't have a player to test this on and I can't get the simulator
> to build on Win32 (and I don't feel like installing Linux).
>
> 07. (Björn Stenberg) Additionally, I want to separate between file
> and playlist bookmarks.
> This implementation follows the basic design of Rockbox, which
> is that it bookmarks playlists and directories. Further, my understanding
> of Björn's comment is that he wants the bookmark to be created for
> the MP3 even if it's playing in the playlist. It's do-able, but
> I don't voluteer for implementing it as that would require multiple
> writes (one for the playlist, one for the MP3), which I think shouldn't
> be done.
>
> 08. Centralized Bookmarking
> This can be done by modifing the
bookmark_generate_bookmark_file_name().
> Doing so will change where and what name a bookmark is stored or
> where the code looks to load a bookmark from. The main issue with
> this is guareenteing unique names for each bookmark file as I don't
> think duplicating the MP3 directories is a viable option. One possible
> solution is a to generate an MD5 signature for each path+file combination.
> This would almost guareentee uniqueness.
>
> 09. (Daniel Stenberg) bookmark_get_next_field() is overly exessive
> and both sets and copies huge chunks of memory on each call. I can't
> see any obvious error in your use of strtok_r() other than I think
> you should solve the repeated invokes in a loop or similar.
> See 04.
>
> Other Issues:
> -----------------------------------------------------------------
> ---------
> 01. Develop better play capability. Current code is a duplicate
> of tree.c:play_resume().
> 02. Bookmark/resume files are created, but they don't show up until
> the next time the directory is loaded.
> 03. Need to come up with an icon for bookmark file.
> 04. Changing the shuffle mode while playing a filelist or a directory
> can cause the incorrect index information to be stored in the bookmark,
> which may cause the bookmark_load() function to load the wrong MP3.
>
> 05. The bookmark menu should should some identifying information
> about what MP3 the bookmark is referencing. This used to happen,
> but it was taken out for the new bookmark format.
>
>
>
>
>
>
>
Received on 2003-02-01