From 9921c2e5e166c83de2c87ccb140c167cbb3d123e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Sch=C3=BCrmann?= Date: Thu, 14 Mar 2019 07:04:37 +0100 Subject: [PATCH 1/3] Improve error handling when sending broadcasting metadata --- src/engine/sidechain/shoutconnection.cpp | 35 ++++++++++++++++-------- src/engine/sidechain/shoutconnection.h | 1 + 2 files changed, 25 insertions(+), 11 deletions(-) diff --git a/src/engine/sidechain/shoutconnection.cpp b/src/engine/sidechain/shoutconnection.cpp index b72b9dda6da..9c272c69128 100644 --- a/src/engine/sidechain/shoutconnection.cpp +++ b/src/engine/sidechain/shoutconnection.cpp @@ -147,6 +147,13 @@ QByteArray ShoutConnection::encodeString(const QString& string) { return string.toLatin1(); } +void ShoutConnection::insertMetaData(const char *pName, const char *pValue) { + int ret = shout_metadata_add(m_pShoutMetaData, pName, pValue); + if (ret != SHOUTERR_SUCCESS) { + kLogger.warning() << "shout_metadata_add" << pName << "fails with error code" << ret; + } +} + void ShoutConnection::updateFromPreferences() { kLogger.debug() << m_pProfile->getProfileName() << ": updating from preferences"; @@ -705,8 +712,14 @@ bool ShoutConnection::metaDataHasChanged() { void ShoutConnection::updateMetaData() { setFunctionCode(5); - if (!m_pShout || !m_pShoutMetaData) + if (!m_pShout) { + kLogger.debug() << "updateMetaData failed, invalid m_pShout"; + return; + } + if (!m_pShoutMetaData) { + kLogger.debug() << "updateMetaData failed, invalid m_pShoutMetaData"; return; + } /** * If track has changed and static metadata is disabled @@ -742,8 +755,8 @@ void ShoutConnection::updateMetaData() { // old way for those use cases. if (!m_format_is_mp3 && m_protocol_is_icecast2) { setFunctionCode(9); - shout_metadata_add(m_pShoutMetaData, "artist", encodeString(artist).constData()); - shout_metadata_add(m_pShoutMetaData, "title", encodeString(title).constData()); + insertMetaData("artist", encodeString(artist).constData()); + insertMetaData("title", encodeString(title).constData()); } else { // we are going to take the metadata format and replace all // the references to $title and $artist by doing a single @@ -775,10 +788,13 @@ void ShoutConnection::updateMetaData() { QByteArray baSong = encodeString(metadataFinal); setFunctionCode(10); - shout_metadata_add(m_pShoutMetaData, "song", baSong.constData()); + insertMetaData("song", baSong.constData()); } setFunctionCode(11); - shout_set_metadata(m_pShout, m_pShoutMetaData); + int ret = shout_set_metadata(m_pShout, m_pShoutMetaData); + if (ret != SHOUTERR_SUCCESS) { + kLogger.warning() << "shout_set_metadata fails with error code" << ret; + } } } else { // Otherwise we might use static metadata @@ -788,14 +804,11 @@ void ShoutConnection::updateMetaData() { // see comment above... if (!m_format_is_mp3 && m_protocol_is_icecast2) { setFunctionCode(12); - shout_metadata_add( - m_pShoutMetaData,"artist",encodeString(m_customArtist).constData()); - - shout_metadata_add( - m_pShoutMetaData,"title",encodeString(m_customTitle).constData()); + insertMetaData("artist", encodeString(m_customArtist).constData()); + insertMetaData("title", encodeString(m_customTitle).constData()); } else { QByteArray baCustomSong = encodeString(m_customArtist.isEmpty() ? m_customTitle : m_customArtist + " - " + m_customTitle); - shout_metadata_add(m_pShoutMetaData, "song", baCustomSong.constData()); + insertMetaData("song", baCustomSong.constData()); } setFunctionCode(13); diff --git a/src/engine/sidechain/shoutconnection.h b/src/engine/sidechain/shoutconnection.h index 225fdef9b8e..d771a0871d8 100644 --- a/src/engine/sidechain/shoutconnection.h +++ b/src/engine/sidechain/shoutconnection.h @@ -116,6 +116,7 @@ class ShoutConnection bool waitForRetry(); void tryReconnect(); + void insertMetaData(const char *name, const char *value); QTextCodec* m_pTextCodec; TrackPointer m_pMetaData; From 0a9e68b2895fb53164ac55be844d365018ec0444 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Sch=C3=BCrmann?= Date: Thu, 14 Mar 2019 07:09:45 +0100 Subject: [PATCH 2/3] Don't send an empty codec field in case of default ISO-8859-1, remove the related warning --- src/engine/sidechain/shoutconnection.cpp | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/src/engine/sidechain/shoutconnection.cpp b/src/engine/sidechain/shoutconnection.cpp index 9c272c69128..dd2ab9ca2b0 100644 --- a/src/engine/sidechain/shoutconnection.cpp +++ b/src/engine/sidechain/shoutconnection.cpp @@ -186,17 +186,19 @@ void ShoutConnection::updateFromPreferences() { // strings to pass to libshout. QString codec = m_pProfile->getMetadataCharset(); - QByteArray baCodec = codec.toLatin1(); - m_pTextCodec = QTextCodec::codecForName(baCodec); - if (!m_pTextCodec) { - kLogger.warning() - << "Couldn't find broadcast metadata codec for codec:" << codec - << " defaulting to ISO-8859-1."; + if (!codec.isEmpty()) { + QByteArray baCodec = codec.toLatin1(); + m_pTextCodec = QTextCodec::codecForName(baCodec); + if (!m_pTextCodec) { + kLogger.warning() + << "Couldn't find broadcast metadata codec for codec:" << codec + << " defaulting to ISO-8859-1."; + } else { + // Indicates our metadata is in the provided charset. + insertMetaData("charset", baCodec.constData()); + } } - // Indicates our metadata is in the provided charset. - shout_metadata_add(m_pShoutMetaData, "charset", baCodec.constData()); - QString serverType = m_pProfile->getServertype(); QString host = m_pProfile->getHost(); From e2c0fae826781946f88e74ffc8879ce7c4ad229d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Sch=C3=BCrmann?= Date: Thu, 14 Mar 2019 07:35:14 +0100 Subject: [PATCH 3/3] Disable TLS, fixes lp:1817395 --- src/engine/sidechain/shoutconnection.cpp | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/engine/sidechain/shoutconnection.cpp b/src/engine/sidechain/shoutconnection.cpp index dd2ab9ca2b0..0267fafbad3 100644 --- a/src/engine/sidechain/shoutconnection.cpp +++ b/src/engine/sidechain/shoutconnection.cpp @@ -89,6 +89,16 @@ ShoutConnection::ShoutConnection(BroadcastProfilePtr profile, errorDialog(tr("Error setting non-blocking mode:"), shout_get_error(m_pShout)); } + +#ifdef SHOUT_TLS + // Libshout defaults to SHOUT_TLS_AUTO if build with SHOUT_TLS + // Sometimes autodetection fails, resulting into no metadata send + // https://bugs.launchpad.net/mixxx/+bug/1817395 + if (shout_set_tls(m_pShout, SHOUT_TLS_DISABLED) != SHOUTERR_SUCCESS) { + errorDialog(tr("Error setting tls mode:"), + shout_get_error(m_pShout)); + } +#endif } ShoutConnection::~ShoutConnection() {