Merge lp:~pwhelan/mixxx/shoutcast into lp:mixxx/1.7

[Phillip Whelan]

[Phillip Whelan (pwhelan)]

Several changes and bugfixes to Shoutcast, including:

  * Check in EngineShoutcast::writePage for the connection status
  * Better Tracking of the Active track for Metadata
  * Metadata Updates for Shoutcast support

I know 1.7 is feature frozen, but my previous changes were also accepted, as well, none of the binaries are rolled out with shoutcast support.

If there was a trunk branch I'd be proposing it to be merged there... but oh well...

[lp:~pwhelan/mixxx/shoutcast]

2452. By Phillip Whelan <madjester@voidwalker> on 2009-07-10

More stability changes for Shoutcast, as well as reseting the metadata lifetime
back to a sane value.

  * Check metadata every 32 invocations, as before.
  * Reconnect on server connect error
  * Reconnect on shout_send error

2453. By Phillip Whelan <madjester@voidwalker> on 2009-07-12

Comments for EngineShoutcast.

[Albert Santoni (gamegod)]

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

[Phillip Whelan (pwhelan)]

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

More for the most part, yes you are correct. I did extensive testing on the
metadata update code and fixed it to actually work. I was doing the
crossfader conditional totally wrong. I also changed the code to use
properly instantiated ControlObjectThread objects which are instantiated
once, instead of for each pass of the code which detects which is the
active track.

>
> 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.

I'll check into that, but I seriously doubt that libshout supports UTF-8,
and if so, Shoutcast might not support metadata in UTF-8.

> 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.
> :)

That'd be great, on both accounts.

> 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.

I also thought the same.

> Thanks!
Your welcome!

[Albert Santoni (gamegod)]

review: Approve

[Sean M. Pappalardo (pegasus-renegadetech)]

> 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.

Does that mean we should offer it as a new feature in 1.7.1? Or are we going to reserve point releases strictly for bug fixes?

[Albert Santoni (gamegod)]

For the time being, point releases are going to be strictly bug fix
releases. If you're doing feature development, it should stay in a
branch until it's ready to be merged into the 1.8 branch. The case of
madjesta's shoutcast code is special because we're transitioning to
this new development cycle still.

On Mon, Jul 13, 2009 at 7:59 PM, Pegasus<email address hidden> wrote:
>> 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.
>
> Does that mean we should offer it as a new feature in 1.7.1? Or are we going to reserve point releases strictly for bug fixes?
> --
> https://code.launchpad.net/~pwhelan/mixxx/shoutcast/+merge/8518
> You are reviewing the proposed merge of lp:~pwhelan/mixxx/shoutcast into lp:mixxx/1.7.
>

=== modified file 'mixxx/src/SConscript'
--- mixxx/src/SConscript	2009-07-07 21:55:55 +0000
+++ mixxx/src/SConscript	2009-07-08 03:00:57 +0000
@@ -872,16 +872,42 @@
 
 #Experimental Shoutcast
 flags_shoutcast = getFlags(env, 'shoutcast', 0)
+
 if int(flags_shoutcast):
-#TODO: check for libshout
-	env.Append(LIBS = 'shout');
-	env.Append(LIBS = 'vorbisenc');
-	env.Append(LIBS = 'mp3lame');
+	shoutmp3 = 0
+	shoutogg = 0
+	
+	conf = Configure(env, custom_tests = { 'CheckForPKGConfig' : CheckForPKGConfig, 'CheckForPKG' : CheckForPKG })
+	conf.CheckLib('shout')
+	
 	env.Append(CPPDEFINES = '__SHOUTCAST__')
-	sources += Split(""" dlgprefshoutcast.cpp engine/engineshoutcast.cpp encoder.cpp encodervorbis.cpp encodermp3.cpp""" )
+	
+	sources += Split(""" dlgprefshoutcast.cpp engine/engineshoutcast.cpp encoder.cpp """ )
+	build_flags += 'shoutcast '
+	
+	
+	if conf.CheckLib('mp3lame'):
+		env.Append(CPPDEFINES = '__SHOUTCAST_LAME__')
+		sources += Split(""" encodermp3.cpp """)
+		shoutmp3 = 1
+	
+	if conf.CheckLib('vorbisenc'):
+		env.Append(CPPDEFINES = '__SHOUTCAST_VORBIS__')
+		sources += Split("""  encodervorbis.cpp """)
+		shoutogg = 1
+	
+	
+	if shoutmp3 and shoutogg:
+		print "Shoutcast support (OGG/MP3)... enabled"
+	elif shoutmp3:
+		print "Shoutcast support (MP3)... enabled"
+	elif shoutogg:
+		print "Shoutcast support (OGG)... enabled"
+	else:
+		print "Shoutcast support... enabled"
+	
 	env.Uic4('dlgprefshoutcastdlg.ui')
-	print "Shoutcast support... enabled"
-	build_flags += 'shoutcast '
+	
 else:
 	print "Shoutcast support... disabled"
 

=== modified file 'mixxx/src/engine/engineshoutcast.cpp'
--- mixxx/src/engine/engineshoutcast.cpp	2009-02-18 06:27:27 +0000
+++ mixxx/src/engine/engineshoutcast.cpp	2009-07-08 17:47:11 +0000
@@ -19,8 +19,13 @@
 #include "configobject.h"
 #include "dlgprefshoutcast.h"
 
+#ifdef __SHOUTCAST_VORBIS__
 #include "encodervorbis.h"
+#endif // __SHOUTCAST_VORBIS__
+#ifdef __SHOUTCAST_LAME__
 #include "encodermp3.h"
+#endif // __SHOUTCAST_LAME__
+
 #include "playerinfo.h"
 #include "trackinfoobject.h"
 
@@ -45,6 +50,10 @@
     m_pConfig = _config;
     m_pUpdateShoutcastFromPrefs = new ControlObjectThreadMain(ControlObject::getControl(ConfigKey(SHOUTCAST_PREF_KEY, "update_from_prefs")));
     
+    m_pCrossfader = new ControlObjectThread(ControlObject::getControl(ConfigKey("[Master]","crossfader")));
+    m_pVolume1 = new ControlObjectThread(ControlObject::getControl(ConfigKey("[Channel1]","volume")));
+    m_pVolume2 = new ControlObjectThread(ControlObject::getControl(ConfigKey("[Channel2]","volume")));
+    
     QByteArray baBitrate = m_pConfig->getValueString(ConfigKey(SHOUTCAST_PREF_KEY,"bitrate")).toLatin1();
     QByteArray baFormat = m_pConfig->getValueString(ConfigKey(SHOUTCAST_PREF_KEY,"format")).toLatin1();
     int len;
@@ -58,6 +67,15 @@
         return;
     }
     
+    if (!(m_pShoutMetaData = shout_metadata_new())) {
+        qDebug() << "Cound not allocate shout_metadata_t";
+        return;
+    }
+    
+    // set to a high number to automatically update the metadata
+    // on the first change
+    m_pMetaDataLife = 31337;
+    
     //Initialize the m_pShout structure with the info from Mixxx's shoutcast preferences.
     updateFromPreferences();
 
@@ -67,7 +85,8 @@
     }
     
     qDebug("********START SERVERCONNECT*******");
-    serverConnect();
+    if ( !serverConnect())
+        return;
     
     if (( len = baBitrate.indexOf(' ')) != -1) {
         baBitrate.resize(len);
@@ -75,10 +94,20 @@
     
     // Initialize encoder
     if ( ! qstrcmp(baFormat, "MP3")) {
+#ifdef __SHOUTCAST_LAME__
         encoder = new EncoderMp3(m_pConfig, this);
+#else
+        qDebug() << "*** Missing MP3 Encoder Support";
+        return;
+#endif // __SHOUTCAST_LAME__
     }
     else if ( ! qstrcmp(baFormat, "Ogg Vorbis")) {
+#ifdef __SHOUTCAST_VORBIS__
         encoder = new EncoderVorbis(m_pConfig, this);
+#else
+        qDebug() << "*** Missing OGG Vorbis Encoder Support";
+        return;
+#endif // __SHOUTCAST_VORBIS__
     }
     else {
         qDebug() << "**** Unknown Encoder Format";
@@ -95,7 +124,12 @@
 {
     delete encoder;
     delete m_pUpdateShoutcastFromPrefs;
+    delete m_pCrossfader;
+    delete m_pVolume1;
+    delete m_pVolume2;
     
+    if (m_pShoutMetaData)
+        shout_metadata_free(m_pShoutMetaData);
     if (m_pShout)
         shout_close(m_pShout);
     shout_shutdown();
@@ -205,7 +239,7 @@
     
 }
 
-void EngineShoutcast::serverConnect()
+bool EngineShoutcast::serverConnect()
 {
     qDebug("in serverConnect();");
     if (m_pShout)
@@ -223,7 +257,10 @@
     }
     if (m_iShoutStatus == SHOUTERR_CONNECTED) {
         qDebug() << "***********Connected to Shoutcast server...";
+        return true;
     }
+    
+    return false;
 }
 
 void EngineShoutcast::writePage(unsigned char *header, unsigned char *body,
@@ -259,15 +296,128 @@
     }
 }
 
-/*void EngineShoutcast::wrapper2writePage(void *pObj, unsigned char *header, unsigned char *body,
-                                        int headerLen, int bodyLen)
-{
-    EngineShoutcast* mySelf = (EngineShoutcast*)pObj;
-    pObj->writePage(header, body, headerLen, bodyLen);
-}*/
-
-void EngineShoutcast::process(const CSAMPLE *pIn, const CSAMPLE *pOut, const int iBufferSize)
-{
-//    encoder->encodeBuffer((void*) &objA, EngineShoutcast::wrapper2writePage, pOut, iBufferSize);
+void EngineShoutcast::process(const CSAMPLE *, const CSAMPLE *pOut, const int iBufferSize)
+{
+    if (m_iShoutStatus != SHOUTERR_CONNECTED)
+        return;
+    
     if (iBufferSize > 0) encoder->encodeBuffer(pOut, iBufferSize);
+    
+    if ( metaDataHasChanged())
+        updateMetaData();
+}
+
+/* Algorithm which simply flips the lowest and/or second lowest bits,
+ * bits 1 and 2, to represent which track is active and returns the result.
+ */
+
+int EngineShoutcast::getActiveTracks()
+{
+    int tracks = 0;
+    
+    
+    if (ControlObject::getControl(ConfigKey("[Channel1]","play"))->get()==1.) tracks |= 1;
+    if (ControlObject::getControl(ConfigKey("[Channel2]","play"))->get()==1.) tracks |= 2;
+    
+    if (tracks ==  0)
+        return 0;
+    
+    // Detect the dominant track by checking the crossfader and volume levels
+    if ((tracks & 1) && (tracks & 2)) {
+        
+        if ((m_pVolume1->get() == 0) && (m_pVolume2->get() == 0))
+            return 0;
+        
+        if (m_pVolume2->get() == 0) {
+            tracks = 1;
+        }
+        else if ( m_pVolume1->get() == 0) {
+            tracks = 2;
+        }
+        // allow a bit of leeway with the crossfader
+        else if ((m_pCrossfader->get() < 0.05) && (m_pCrossfader->get() > -0.05)) {
+            
+            if (m_pVolume1->get() > m_pVolume2->get()) {
+                tracks = 1;
+            }
+            else if (m_pVolume1->get() < m_pVolume2->get()) {
+                tracks = 2;
+            }
+            
+        }
+        else if ( m_pCrossfader->get() < -0.05 ) {
+            tracks = 1;
+        }
+        else if ( m_pCrossfader->get() > 0.05 ) {
+            tracks = 2;
+        }
+        
+    }
+    
+    return tracks;
+}
+
+bool EngineShoutcast::metaDataHasChanged()
+{
+    int tracks;
+    TrackInfoObject *newMetaData;
+    bool changed = false;
+    
+    
+    if ( m_pMetaDataLife < 1 ) {
+        m_pMetaDataLife++;
+        return false;
+    }
+    
+    m_pMetaDataLife = 0;
+    
+    
+    tracks = getActiveTracks();
+    
+    
+    switch (tracks)
+    {
+    case 0:
+        // no tracks are playing
+        // we should set the metadata to nothing
+        break;
+    case 1:
+        // track 1 is active
+        
+        newMetaData = PlayerInfo::Instance().getTrackInfo(1);
+        if (newMetaData != m_pMetaData)
+        {
+            m_pMetaData = newMetaData;
+            changed = true;
+        }
+        break;
+    case 2:
+        // track 2 is active
+        newMetaData = PlayerInfo::Instance().getTrackInfo(2);
+        if (newMetaData != m_pMetaData)
+        {
+            m_pMetaData = newMetaData;
+            changed = true;
+        }
+        break;
+    case 3:
+        // both tracks are active, just stick with it for now
+        break;
+    }
+    
+    qDebug() << "tracks = " << tracks << " changed = " << changed;
+    
+    
+    return changed;
+}
+
+void EngineShoutcast::updateMetaData()
+{
+    // convert QStrings to char*s
+    QByteArray baArtist = m_pMetaData->getArtist().toLatin1();
+    QByteArray baTitle = m_pMetaData->getTitle().toLatin1();
+    QByteArray baSong = baArtist + " - " + baTitle;
+    
+    shout_metadata_add(m_pShoutMetaData, "song",  baSong.data());
+    shout_set_metadata(m_pShout, m_pShoutMetaData);
 }

=== modified file 'mixxx/src/engine/engineshoutcast.h'
--- mixxx/src/engine/engineshoutcast.h	2009-02-18 04:28:21 +0000
+++ mixxx/src/engine/engineshoutcast.h	2009-07-08 03:01:35 +0000
@@ -51,14 +51,23 @@
 //    void writePage(unsigned char *header, unsigned char *body,
 //                   int headerLen, int bodyLen, int count);
 private:
-    void serverConnect();
+    bool serverConnect();
+    int getActiveTracks();
+    bool metaDataHasChanged();
+    void updateMetaData();
+    TrackInfoObject *m_pMetaData;
     shout_t *m_pShout;
+    shout_metadata_t *m_pShoutMetaData;
+    int m_pMetaDataLife;
     long m_iShoutStatus;
     ConfigObject<ConfigValue> *m_pConfig;
     ControlObject* recReady;
     Encoder *encoder;
     ControlObjectThreadMain* m_pUpdateShoutcastFromPrefs;
 //    void (*writeFn)(unsigned char *, unsigned char *, int, int);
+    ControlObjectThread* m_pCrossfader;
+    ControlObjectThread* m_pVolume1;
+    ControlObjectThread* m_pVolume2;
 };
 
 #endif