summaryrefslogtreecommitdiffstats
path: root/mediaplayer
diff options
context:
space:
mode:
authorScott Murray <scott.murray@konsulko.com>2023-02-19 17:39:28 -0500
committerScott Murray <scott.murray@konsulko.com>2023-02-19 17:48:16 -0500
commit2cbbf914242d0766ff4e97ee3c0d02505d13e422 (patch)
tree8cb78ef266900f17e27a90d844af143a59887d7e /mediaplayer
parent9a7e2c5420cae0669c149d5dddbcd99a8fac0038 (diff)
mediaplayer: Improve MPD failure handling
Changes: - Avoid crashing in the MPD event handler code when our connection times out because it has hung. Additionally, attempt to update the front end UI to a sane state with respect to metadata when this happens. - Lower the MPD connection timeout and keepalive timeouts to try to catch MPD failures faster. There are still problems in this area with respect to the front end UI getting stuck if it does an operation that would attempt to use the MPD API while it is hung. Fixing that completely would require a significant refactoring to move all the MPD API code to a separate thread so that the QML rendering thread does not get stuck. It's unclear at present if such a refactoring is justified, as the MPD hang in this case is fixed by changes in MPD 0.23.8. Bug-AGL: SPEC-4661 Signed-off-by: Scott Murray <scott.murray@konsulko.com> Change-Id: Ia5e74d00acdc6b3b28cdf87b26486dbddfa02a98
Diffstat (limited to 'mediaplayer')
-rw-r--r--mediaplayer/MediaplayerMpdBackend.cpp82
-rw-r--r--mediaplayer/MediaplayerMpdBackend.h1
-rw-r--r--mediaplayer/MpdEventHandler.cpp13
3 files changed, 66 insertions, 30 deletions
diff --git a/mediaplayer/MediaplayerMpdBackend.cpp b/mediaplayer/MediaplayerMpdBackend.cpp
index 5cc9dce..8ffb346 100644
--- a/mediaplayer/MediaplayerMpdBackend.cpp
+++ b/mediaplayer/MediaplayerMpdBackend.cpp
@@ -19,14 +19,15 @@
#include "MpdEventHandler.h"
#include "mediaplayer.h"
-// Use a 60s timeout on our MPD connection
+// Use a 30s timeout on our MPD connection
// NOTE: The connection is actively poked at a higher frequency than
// this to ensure we don't hit the timeout. The alternatives
// are to either open and close a connection for every command,
// or try to keep the connection in idle mode when not using it.
// The latter is deemed too complicated for our purposes for now,
// due to it likely requiring another thread.
-#define MPD_CONNECTION_TIMEOUT 60000
+#define MPD_CONNECTION_TIMEOUT 30000
+#define MPD_KEEPALIVE_TIMEOUT 5000
MediaplayerMpdBackend::MediaplayerMpdBackend(Mediaplayer *player, QQmlContext *context, QObject *parent) :
MediaplayerBackend(player, parent)
@@ -43,7 +44,7 @@ MediaplayerMpdBackend::MediaplayerMpdBackend(Mediaplayer *player, QQmlContext *c
// Set up connection keepalive timer
m_mpd_conn_timer = new QTimer(this);
connect(m_mpd_conn_timer, &QTimer::timeout, this, &MediaplayerMpdBackend::connectionKeepaliveTimeout);
- m_mpd_conn_timer->start(MPD_CONNECTION_TIMEOUT / 2);
+ m_mpd_conn_timer->start(MPD_KEEPALIVE_TIMEOUT);
m_song_pos_timer = new QTimer(this);
connect(m_song_pos_timer, &QTimer::timeout, this, &MediaplayerMpdBackend::songPositionTimeout);
@@ -57,6 +58,7 @@ MediaplayerMpdBackend::MediaplayerMpdBackend(Mediaplayer *player, QQmlContext *c
// Create thread to handle polling for MPD events
MpdEventHandler *handler = new MpdEventHandler();
handler->moveToThread(&m_handlerThread);
+ connect(&m_handlerThread, &QThread::finished, this, &MediaplayerMpdBackend::handleHandlerFinish);
connect(&m_handlerThread, &QThread::finished, handler, &QObject::deleteLater);
connect(this,
&MediaplayerMpdBackend::startHandler,
@@ -169,25 +171,37 @@ void MediaplayerMpdBackend::connectionKeepaliveTimeout(void)
m_mpd_conn_mutex.lock();
// Clear any lingering non-fatal errors
- if (!mpd_connection_clear_error(m_mpd_conn)) {
+ if (m_mpd_conn && !mpd_connection_clear_error(m_mpd_conn)) {
// NOTE: There should likely be an attempt to reconnect here,
// but it definitely would complicate things for all the
// other users.
qWarning() << "MPD connection in error state!";
- m_mpd_conn_mutex.unlock();
- return;
+ mpd_connection_free(m_mpd_conn);
+ m_mpd_conn = NULL;
+ m_mpd_conn_timer->stop();
}
+ m_mpd_conn_mutex.unlock();
+}
- struct mpd_status *status = mpd_run_status(m_mpd_conn);
- if (!status) {
- qWarning() << "MPD connection status check failed";
- } else {
- mpd_status_free(status);
- }
+void MediaplayerMpdBackend::handleHandlerFinish(void)
+{
+ m_mpd_conn_mutex.lock();
+
+ // If the event thread finished, our connection is likely dead.
+ // Given the behavior seen when mpd is hung is most client API
+ // calls hang, trying to determine mpd liveliness here seems
+ // problematic. Attempting to reconnect would probably be the
+ // only robust solution.
+ mpd_connection_free(m_mpd_conn);
+ m_mpd_conn = NULL;
m_mpd_conn_mutex.unlock();
+
+ m_mpd_conn_timer->stop();
+ delete m_mpd_conn_timer;
}
+
void MediaplayerMpdBackend::songPositionTimeout(void)
{
m_state_mutex.lock();
@@ -271,7 +285,7 @@ void MediaplayerMpdBackend::play()
m_mpd_conn_mutex.lock();
m_state_mutex.lock();
- if (!m_playing) {
+ if (m_mpd_conn && !m_playing) {
mpd_run_play(m_mpd_conn);
}
m_state_mutex.unlock();
@@ -284,7 +298,7 @@ void MediaplayerMpdBackend::pause()
m_mpd_conn_mutex.lock();
m_state_mutex.lock();
- if (m_playing) {
+ if (m_mpd_conn && m_playing) {
mpd_run_pause(m_mpd_conn, true);
}
m_state_mutex.unlock();
@@ -298,7 +312,7 @@ void MediaplayerMpdBackend::previous()
// MPD only allows next/previous if playing
m_state_mutex.lock();
- if (m_playing) {
+ if (m_mpd_conn && m_playing) {
mpd_run_previous(m_mpd_conn);
}
m_state_mutex.unlock();
@@ -312,7 +326,7 @@ void MediaplayerMpdBackend::next()
// MPD only allows next/previous if playing
m_state_mutex.lock();
- if (m_playing) {
+ if (m_mpd_conn && m_playing) {
mpd_run_next(m_mpd_conn);
}
m_state_mutex.unlock();
@@ -324,9 +338,11 @@ void MediaplayerMpdBackend::seek(int milliseconds)
{
m_mpd_conn_mutex.lock();
- float t = milliseconds;
- t /= 1000.0;
- mpd_run_seek_current(m_mpd_conn, t, false);
+ if (m_mpd_conn) {
+ float t = milliseconds;
+ t /= 1000.0;
+ mpd_run_seek_current(m_mpd_conn, t, false);
+ }
m_mpd_conn_mutex.unlock();
}
@@ -336,9 +352,11 @@ void MediaplayerMpdBackend::fastforward(int milliseconds)
{
m_mpd_conn_mutex.lock();
- float t = milliseconds;
- t /= 1000.0;
- mpd_run_seek_current(m_mpd_conn, t, true);
+ if (m_mpd_conn) {
+ float t = milliseconds;
+ t /= 1000.0;
+ mpd_run_seek_current(m_mpd_conn, t, true);
+ }
m_mpd_conn_mutex.unlock();
}
@@ -348,9 +366,11 @@ void MediaplayerMpdBackend::rewind(int milliseconds)
{
m_mpd_conn_mutex.lock();
- float t = -milliseconds;
- t /= 1000.0;
- mpd_run_seek_current(m_mpd_conn, t, true);
+ if (m_mpd_conn) {
+ float t = -milliseconds;
+ t /= 1000.0;
+ mpd_run_seek_current(m_mpd_conn, t, true);
+ }
m_mpd_conn_mutex.unlock();
}
@@ -359,7 +379,7 @@ void MediaplayerMpdBackend::picktrack(int track)
{
m_mpd_conn_mutex.lock();
- if (track >= 0) {
+ if (m_mpd_conn && track >= 0) {
mpd_run_play_pos(m_mpd_conn, track);
}
@@ -383,10 +403,12 @@ void MediaplayerMpdBackend::loop(QString state)
// mpd_run_single_state(m_mpd_conn, MPD_SINGLE_OFF) (default)
// mpd_run_repeat(m_mpd_conn, true) to loop
- if (state == "playlist") {
- mpd_run_repeat(m_mpd_conn, true);
- } else {
- mpd_run_repeat(m_mpd_conn, false);
+ if (m_mpd_conn) {
+ if (state == "playlist") {
+ mpd_run_repeat(m_mpd_conn, true);
+ } else {
+ mpd_run_repeat(m_mpd_conn, false);
+ }
}
m_mpd_conn_mutex.unlock();
diff --git a/mediaplayer/MediaplayerMpdBackend.h b/mediaplayer/MediaplayerMpdBackend.h
index 4b8d74c..2a44655 100644
--- a/mediaplayer/MediaplayerMpdBackend.h
+++ b/mediaplayer/MediaplayerMpdBackend.h
@@ -55,6 +55,7 @@ signals:
private slots:
void connectionKeepaliveTimeout(void);
+ void handleHandlerFinish(void);
void songPositionTimeout(void);
void updatePlaybackState(int queue_pos, int song_pos_ms, bool state);
void updateMetadata(QVariantMap metadata);
diff --git a/mediaplayer/MpdEventHandler.cpp b/mediaplayer/MpdEventHandler.cpp
index 56a4612..977f96c 100644
--- a/mediaplayer/MpdEventHandler.cpp
+++ b/mediaplayer/MpdEventHandler.cpp
@@ -16,6 +16,7 @@
#include <QDebug>
#include <QFileInfo>
+#include <QThread>
#include "MpdEventHandler.h"
MpdEventHandler::MpdEventHandler(QObject *parent) :
@@ -62,9 +63,13 @@ void MpdEventHandler::handleEvents(void)
else if (events & MPD_IDLE_PLAYER) {
handlePlayerEvent();
}
+ if (mpd_connection_get_error(m_mpd_conn) != MPD_ERROR_SUCCESS) {
+ done = true;
+ }
}
qDebug() << "MpdEventHandler::handleEvents: exit";
+ QThread::currentThread()->quit();
}
void MpdEventHandler::handleDatabaseEvent(void)
@@ -176,6 +181,14 @@ void MpdEventHandler::handlePlayerEvent(void)
// corking in WirePlumber...
struct mpd_status *status = mpd_run_status(m_mpd_conn);
+ if (!status) {
+ // mpd has gone away, attempt to get the UI into a good state
+ metadata["position"] = 0;
+ metadata["status"] = QString("stopped");
+ emit metadataUpdate(metadata);
+ emit playbackStateUpdate(pos, 0, false);
+ return;
+ }
int elapsed_ms = mpd_status_get_elapsed_ms(status);
metadata["position"] = elapsed_ms;