Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add persistant local storage for CD metadata to Banshee #46

Closed
GoogleCodeExporter opened this issue Jun 3, 2015 · 19 comments
Closed

Add persistant local storage for CD metadata to Banshee #46

GoogleCodeExporter opened this issue Jun 3, 2015 · 19 comments

Comments

@GoogleCodeExporter
Copy link

Currently if the user manually enters metadata about a CD into Banshee,
that metadata will not persist between sessions. If the user closes Banshee
the metadata will be lost and must be re-entered again when Banshee starts
again. CD metadata should be stored in the Banshee library database. This
should be done both for user-provided metadata and metadata retrieved from
the MusicBrainz service so that Banshee does not need to access MusicBrainz
more than once per CD.

If you're interested in this issue, feel free to contact we with any
questions in irc.gnome.org#mono-ghop. I'm scottp.

Original issue reported on code.google.com by lunchtim...@gmail.com on 13 Dec 2007 at 4:37

@GoogleCodeExporter
Copy link
Author

A little more about what this would entail. You would be modifying Banshee's
CD-handling routine (to check for local metadata) and you would also need to 
store
metadata into the local database at an appropriate time. You'd get to
programmatically create, edit, and query SQL tables. This should be a 1-2 day 
project.

Original comment by lunchtim...@gmail.com on 13 Dec 2007 at 4:53

@GoogleCodeExporter
Copy link
Author

I claim this task.

Original comment by renat...@gmail.com on 15 Dec 2007 at 11:21

@GoogleCodeExporter
Copy link
Author

Since the guy was claimed this task I will change status.

Original comment by everaldo...@gmail.com on 15 Dec 2007 at 11:46

  • Added labels: ClaimedBy-renatoat

@GoogleCodeExporter
Copy link
Author

Well, it is just my opinion, but we don't need sql, every CD theres on unique 
ID that
is used to get information in MusicBrainz or CDDB so we can just have a folder 
like:

  $HOME/.banshee/meta-cache/

And for every cd one file with the name of this unique id can be created, so we 
can
easy store and retrieve this metadata.

Hey, they are students, 3~5 days of work in my opinion ;)

Just ignore my comments if non sense :)

Original comment by everaldo...@gmail.com on 16 Dec 2007 at 12:01

@GoogleCodeExporter
Copy link
Author

Oh! Sorry, looks like it already uses sqlite, forget about my last comment ;)

Original comment by everaldo...@gmail.com on 16 Dec 2007 at 12:44

@GoogleCodeExporter
Copy link
Author

renatoat: If you need to get a hold of me and I'm not in IRC, my email is
lunchtimemama@gmail.com

Original comment by lunchtim...@gmail.com on 16 Dec 2007 at 3:07

@GoogleCodeExporter
Copy link
Author

Original comment by lunchtim...@gmail.com on 19 Dec 2007 at 1:35

  • Changed state: Claimed

@GoogleCodeExporter
Copy link
Author

[deleted comment]

@GoogleCodeExporter
Copy link
Author

First patch. Tell me what you think. [FIXED, there was one missing file]

Original comment by renat...@gmail.com on 19 Dec 2007 at 7:21

Attachments:

@GoogleCodeExporter
Copy link
Author

DatabaseBoundTrackInfo.cs:
LoadFromDatabase(params object[] parameters) should become LoadFromDatabase(). 
The
parameters should be accessed via a new abstract property.

In LoadFromDatabase, the DbCommand should be created with String.Format()

All abstract methods and properties should be "protected" rather than "public"


LibraryTrackInfo.cs:
GetId should be restored


AudioCdTrackInfo.cs:
Lines 53 and 71 should use String.Format()

You should get rid of the set accessor for TrackIndex and move any relevant 
code into
the constructor.

Original comment by lunchtim...@gmail.com on 19 Dec 2007 at 10:07

@GoogleCodeExporter
Copy link
Author

Also, you should make the BuildCommand methods use an ArrayList rather than an 
object
array. This will reduce the number of arrays we use (array instantiation is 
expensive).

Original comment by lunchtim...@gmail.com on 19 Dec 2007 at 10:23

@GoogleCodeExporter
Copy link
Author

Updated patch. I used a "SaveParamsCount" property which I don't know if is 
really
needed. Tell me what you think.

Original comment by renat...@gmail.com on 20 Dec 2007 at 11:24

Attachments:

@GoogleCodeExporter
Copy link
Author

Looking good. Sorry to change things on you again, but I've been thinking about 
the
ArrayList/array/Dictionary problem and I've come up with a new solution. Here's 
what
I'd like you to do:

In DatabaseBoundTrackInfo, add:
protected virtual IEnumerable<KeyValuePair<string, object>> DbColumns {
    get {
        yield return new KeyValuePair<string, object>("TrackId", track_id <= 0 ? null
   : (object)track_id));
        yield return new KeyValuePair<string, object>("Artist", artist);
        yield return new KeyValuePair<string, object>("Performer", performer);
        ...
    }
}

and modify the BuildCommand methods to iterate through DbColumns (like so:
"foreach(KeyValuePair<string, object> pair in DbColumns")

In AudioCdTrackInfo, add this:
protected override IEnumerable<KeyValuePair<string, object>> DbColumns {
    get {
        foreach(KeyValuePair<string, object> pair in base.DbColumns) {
            yield return pair;
        }
        yield return new KeyValuePair<string, object>("cd_id", disk.CDID);
        yield return new KeyValuePair<string, object>("track_index", track_index);
    }
}

and do something comparable for LibraryTrackInfo. This gets rid of the need for 
a
Dictionary object or an array and this is a good object oriented design. 
Everything
else looks good though.

Original comment by lunchtim...@gmail.com on 21 Dec 2007 at 7:31

@GoogleCodeExporter
Copy link
Author

Well, I didn't like it very much, as the person who will extend the base class 
will
have to iterate over base.DbColumns and the "yield return new 
KeyValuePair<string,
object>(" part is repeated too many times. And I had to make a workaround about 
the
fact that a IEnumerable doesn't have a Count property (which was needed by
BuildCommand in order to know when to put "," or not, in the end of the query).
Anyway, it's done and attached. Tell me what you think.

Original comment by renat...@gmail.com on 22 Dec 2007 at 1:06

Attachments:

@GoogleCodeExporter
Copy link
Author

Rather than removing the coma after the end of the loop, append the coma
conditionally at the start of the loop, like this:

bool first_time = true;
foreach(...) {
    if(first_time)
        first_time = false;
    else
        columns.Append(", ");

Also, please make the Build methods non-static. That will remove the need for 
the
parameters to those methods and it makes more sense.

The reason for the using yield return is that the only object allocated to the 
heap
are the enumerators (KeyValuePair is a structure which get's pushed on the 
stack).
I'm not concerned about derived classes having to iterate over the base 
DbColumns:
it's common for an override method to invoke the base method. In an enumerator
method, this takes the form of a foreach loop.

Original comment by lunchtim...@gmail.com on 22 Dec 2007 at 6:23

@GoogleCodeExporter
Copy link
Author

Update.

Original comment by renat...@gmail.com on 22 Dec 2007 at 6:43

Attachments:

@GoogleCodeExporter
Copy link
Author

Another one.

Original comment by renat...@gmail.com on 22 Dec 2007 at 7:29

Attachments:

@GoogleCodeExporter
Copy link
Author

I'm not able to test this patch, but it looks good to me so I'm marking this 
issue as
closed. We'll work to integrate this patch into the trunk code where it will 
make its
way into the next major release. Great work, renatoat!

Original comment by lunchtim...@gmail.com on 22 Dec 2007 at 7:40

  • Changed state: Closed

@GoogleCodeExporter
Copy link
Author

Thanks. Anyway, as I said you, I can still work on it if I may be useful :)

Original comment by renat...@gmail.com on 22 Dec 2007 at 8:05

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant