Code review comment for lp:~pwhelan/mixxx/shoutcast

Revision history for this message
Albert Santoni (gamegod) wrote :

This looks like it includes your recent shoutcast-metadata-update.patch and shoutcast-scons-autodeps.patch patches, am I correct?

On lines 295-297 of the merge proposal, you convert QStrings to latin1-encoded char* strings. Does libshout support utf8? If so, use toUtf8() instead of toLatin1(). If not, leave it the way it is.

Just a heads up - In my sqlite branch, I have a real "Player" class which should probably deprecate the PlayerInfo class. It'll probably be my job to modify your code to use that since your code will be in trunk before mine. :)

Anyways, looks good otherwise. For sanity, I also think this should go in the 1.7 branch A) because that's where the latest shoutcast code is otherwise (it might save us some merging headaches later) and B) all of your changes only apply when Mixxx is built with shoutcast=1.

Thanks!

review: Needs Information

« Back to merge proposal