[mythtv] Preview of my MusicPatch for Mythweb/Mythmusic....

[Colin Guthrie]

joggl wrote:
> i'm looking to finish my in the next few days so you can start with yours...
> but i think splitting the music database like it's done in my patches is a 
> good idea, because it removes a lot of unnessecary database entrys and as i 
> said, it speeds up a lot on my machine.

Yup. I totally agree.

FWIW, when/if this is accepted, I will be happy to write the necessary 
SQL to convert your old metadata structure to the new one.

As someone who works quite a lot with DBs I much prefer the layout of 
the database in your patch and the monolithic structure that is there 
just now.


SOme feedback so far (tho' I've not run the patch). Some of the below 
could be totally stupid, so sorry if it is. Please don't feel like you 
need to reply to this either, just act on it if you feel I've made a 
valid point.

Just looking at the db structure (nothing else):

Trivial: Is there a sensible reason why the artist_name field can be 
null? I can't think of one but then I've not looked at the code. I tend 
to avoid the use of Nulls unless 100% necessary and tend towards blank 
strings instead.... saves 1 bit per record if nothing else ;)

Question: I take it that the music_stats table is just a cache to save 
recalculating all the time and it should only have 1 row in it? Sensible 
enough I reckon!

I'm a little confused about the way music_currentsong and music_playlist 
relate to each other... both have a pl_id and song_id field and it would 
seem that song_id is redundant in music_currentsong (i.e. just left join 
on to music_playlist to get the song_id).

Also, am I right in saying that music_saved_playlists will just keep a 
concatenated version of song_ids in the playlist_songs text field? If so 
this isn't the most efficient way, especially when loading a saved 
playlist... if it's just concatenated strings it will have to be loaded, 
parsed and then lots of SQL created in order to insert the correct 
entries in music_playlists where as if it was structured better it could 
probably be done in a single SQL command. Presumably this is how mp3act 
works, and that's fair enough, but there is probably scope for tidying 
these things up once the patch is complete and accepted... (e.g. by the 
creation of a music_saved_playlist_entries table with the fields: 
playlist_id, ordering*, song_id)

*Another point, I could be wrong, but I don't see a way of ordering the 
playlist in the DB. I guess you can go on the pl_id if they are inserted 
in the correct order, but that shouldn't be relied upon as the DB 
backend could theoretically number the pl_id in any way it chooses.

One last thing (hope you don't mind this feedback - esp. as I'm not 
actually running the patch yet!): I notice three uses of user_id in 
music_playhistory, music_playlist and another in music_saved_playlists - 
the definition is inconsistant int(6) and int(11). I think the number 
only affects the display size anyway so not overly important. I don't 
however see any definition of the music_users table :) I guess that the 
"user_id" will ultimately reflect the "frontend" in use at the time? If 
so then it could probably change to a varchar rather than a int. For 
active playlists we will need to know which frontend it belongs to so 
that two frontends can have their own independant active playlists, but 
for saved playlists, it may not be needed? For playhistory I guess it 
doesn't matter whether it exists or not, could be interesting having it 
there for stats purposes. I guess each user who creates a playlist via 
the web would each have their own "frontend" name... (e.g. mythweb1234), 
but then what process is responsible for tidying up timed out sessions 
etc? I'd say that should be handled by the web component as that makes 
most sense.

I could be getting totally the wrong end of the stick for a lot of the 
points above (espcially the last thing) and for that I apologise in 
advance! Just wanted to help give some feedback here as I probably wont 
be able to actually run it for a week or so.

Col.


-- 

+------------------------+
|     Colin Guthrie      |
+------------------------+
| myth(at)colin.guthr.ie |
| http://colin.guthr.ie/ |
+------------------------+