From c96498758fd51119ccf332d82c5a63b33570d02a Mon Sep 17 00:00:00 2001 From: Jonas Kvinge Date: Sat, 22 Apr 2023 03:54:09 +0200 Subject: [PATCH] Fix and improve gapless playback If "about-to-finish" was emitted before the preload time was reached, we never set the next uri, so gapless playback was broken. Make sure to always set the next uri, and increase preload gap from 5 to 8 seconds. --- src/engine/enginebase.cpp | 2 +- src/engine/enginebase.h | 5 +---- src/engine/gstengine.cpp | 26 ++++++++++++++--------- src/engine/gstengine.h | 8 +++---- src/engine/gstenginepipeline.cpp | 36 +++++++++++++++++++++++++------- src/engine/gstenginepipeline.h | 7 ++++++- 6 files changed, 56 insertions(+), 28 deletions(-) diff --git a/src/engine/enginebase.cpp b/src/engine/enginebase.cpp index 48d55143e..793561e71 100644 --- a/src/engine/enginebase.cpp +++ b/src/engine/enginebase.cpp @@ -190,7 +190,7 @@ void Engine::Base::ReloadSettings() { } -void Engine::Base::EmitAboutToEnd() { +void Engine::Base::EmitAboutToFinish() { if (about_to_end_emitted_) { return; diff --git a/src/engine/enginebase.h b/src/engine/enginebase.h index b6640f65d..fa7bd54cc 100644 --- a/src/engine/enginebase.h +++ b/src/engine/enginebase.h @@ -102,9 +102,7 @@ class Base : public QObject { public slots: virtual void ReloadSettings(); void UpdateVolume(const uint volume); - - protected: - void EmitAboutToEnd(); + void EmitAboutToFinish(); public: @@ -217,7 +215,6 @@ class Base : public QObject { bool http2_enabled_; bool strict_ssl_enabled_; - private: bool about_to_end_emitted_; Q_DISABLE_COPY(Base) diff --git a/src/engine/gstengine.cpp b/src/engine/gstengine.cpp index 2d609ff93..dda795785 100644 --- a/src/engine/gstengine.cpp +++ b/src/engine/gstengine.cpp @@ -70,6 +70,9 @@ const char *GstEngine::InterAudiosink = "interaudiosink"; const char *GstEngine::kDirectSoundSink = "directsoundsink"; const char *GstEngine::kOSXAudioSink = "osxaudiosink"; const int GstEngine::kDiscoveryTimeoutS = 10; +const qint64 GstEngine::kTimerIntervalNanosec = 1000 * kNsecPerMsec; // 1s +const qint64 GstEngine::kPreloadGapNanosec = 8000 * kNsecPerMsec; // 8s +const qint64 GstEngine::kSeekDelayNanosec = 100 * kNsecPerMsec; // 100msec GstEngine::GstEngine(TaskManager *task_manager, QObject *parent) : Engine::Base(Engine::EngineType::GStreamer, parent), @@ -162,7 +165,7 @@ void GstEngine::StartPreloading(const QUrl &media_url, const QUrl &stream_url, c // No crossfading, so we can just queue the new URL in the existing pipeline and get gapless playback (hopefully) if (current_pipeline_) { - current_pipeline_->SetNextUrl(media_url, stream_url, gst_url, beginning_nanosec, force_stop_at_end ? end_nanosec : 0); + current_pipeline_->PrepareNextUrl(media_url, stream_url, gst_url, beginning_nanosec, force_stop_at_end ? end_nanosec : 0); // Add request to discover the stream if (discoverer_) { if (!gst_discoverer_discover_uri_async(discoverer_, gst_url.constData())) { @@ -502,20 +505,18 @@ void GstEngine::timerEvent(QTimerEvent *e) { if (e->timerId() != timer_id_) return; - if (current_pipeline_) { - const qint64 current_position = position_nanosec(); + if (current_pipeline_ && !about_to_end_emitted_) { const qint64 current_length = length_nanosec(); - - const qint64 remaining = current_length - current_position; - - const qint64 fudge = kTimerIntervalNanosec + 100 * kNsecPerMsec; // Mmm fudge - const qint64 gap = static_cast(buffer_duration_nanosec_) + (autocrossfade_enabled_ ? fadeout_duration_nanosec_ : kPreloadGapNanosec); - // Only if we know the length of the current stream... if (current_length > 0) { + const qint64 current_position = position_nanosec(); + const qint64 remaining = current_length - current_position; + const qint64 fudge = kTimerIntervalNanosec + 100 * kNsecPerMsec; // Mmm fudge + const qint64 gap = static_cast(buffer_duration_nanosec_) + (autocrossfade_enabled_ ? fadeout_duration_nanosec_ : kPreloadGapNanosec); // Emit TrackAboutToEnd when we're a few seconds away from finishing if (remaining < gap + fudge) { - EmitAboutToEnd(); + qLog(Debug) << "Stream from URL" << media_url_.toString() << "about to end in" << remaining / kNsecPerSec << "seconds. Fuge:" << fudge / kNsecPerMsec << "+" << "Gap:" << gap / kNsecPerMsec; + EmitAboutToFinish(); } } } @@ -782,15 +783,19 @@ void GstEngine::StartFadeoutPause() { } void GstEngine::StartTimers() { + StopTimers(); timer_id_ = startTimer(kTimerIntervalNanosec / kNsecPerMsec); + } void GstEngine::StopTimers() { + if (timer_id_ != -1) { killTimer(timer_id_); timer_id_ = -1; } + } std::shared_ptr GstEngine::CreatePipeline() { @@ -824,6 +829,7 @@ std::shared_ptr GstEngine::CreatePipeline() { QObject::connect(ret.get(), &GstEnginePipeline::BufferingProgress, this, &GstEngine::BufferingProgress); QObject::connect(ret.get(), &GstEnginePipeline::BufferingFinished, this, &GstEngine::BufferingFinished); QObject::connect(ret.get(), &GstEnginePipeline::VolumeChanged, this, &EngineBase::UpdateVolume); + QObject::connect(ret.get(), &GstEnginePipeline::AboutToFinish, this, &EngineBase::EmitAboutToFinish); return ret; diff --git a/src/engine/gstengine.h b/src/engine/gstengine.h index 1e0876c60..5561960de 100644 --- a/src/engine/gstengine.h +++ b/src/engine/gstengine.h @@ -159,9 +159,9 @@ class GstEngine : public Engine::Base, public GstBufferConsumer { static const char *kDirectSoundSink; static const char *kOSXAudioSink; static const int kDiscoveryTimeoutS; - static const qint64 kTimerIntervalNanosec = 1000 * kNsecPerMsec; // 1s - static const qint64 kPreloadGapNanosec = 5000 * kNsecPerMsec; // 5s - static const qint64 kSeekDelayNanosec = 100 * kNsecPerMsec; // 100msec + static const qint64 kTimerIntervalNanosec; + static const qint64 kPreloadGapNanosec; + static const qint64 kSeekDelayNanosec; TaskManager *task_manager_; GstStartup *gst_startup_; @@ -172,7 +172,6 @@ class GstEngine : public Engine::Base, public GstBufferConsumer { std::shared_ptr current_pipeline_; std::shared_ptr fadeout_pipeline_; std::shared_ptr fadeout_pause_pipeline_; - QUrl preloaded_url_; QList buffer_consumers_; @@ -202,7 +201,6 @@ class GstEngine : public Engine::Base, public GstBufferConsumer { int discovery_finished_cb_id_; int discovery_discovered_cb_id_; - }; #endif // GSTENGINE_H diff --git a/src/engine/gstenginepipeline.cpp b/src/engine/gstenginepipeline.cpp index 1772b1b57..3d825a78b 100644 --- a/src/engine/gstenginepipeline.cpp +++ b/src/engine/gstenginepipeline.cpp @@ -123,7 +123,8 @@ GstEnginePipeline::GstEnginePipeline(QObject *parent) notify_source_cb_id_(-1), about_to_finish_cb_id_(-1), notify_volume_cb_id_(-1), - logged_unsupported_analyzer_format_(false) { + logged_unsupported_analyzer_format_(false), + about_to_finish_(false) { eq_band_gains_.reserve(kEqBandCount); for (int i = 0; i < kEqBandCount; ++i) eq_band_gains_ << 0; @@ -1102,13 +1103,16 @@ void GstEnginePipeline::AboutToFinishCallback(GstPlayBin *playbin, gpointer self GstEnginePipeline *instance = reinterpret_cast(self); + qLog(Debug) << "Stream from URL" << instance->gst_url_ << "about to finish."; + + instance->about_to_finish_ = true; + if (instance->has_next_valid_url() && !instance->next_uri_set_) { - // Set the next uri. When the current song ends it will be played automatically and a STREAM_START message is send to the bus. - // When the next uri is not playable an error message is send when the pipeline goes to PLAY (or PAUSE) state or immediately if it is currently in PLAY state. - instance->next_uri_set_ = true; - g_object_set(G_OBJECT(instance->pipeline_), "uri", instance->next_gst_url_.constData(), nullptr); + instance->SetNextUrl(); } + emit instance->AboutToFinish(); + } GstBusSyncReply GstEnginePipeline::BusSyncCallback(GstBus *bus, GstMessage *msg, gpointer self) { @@ -1204,8 +1208,9 @@ void GstEnginePipeline::StreamStatusMessageReceived(GstMessage *msg) { void GstEnginePipeline::StreamStartMessageReceived() { if (next_uri_set_) { + qLog(Debug) << "Stream changed from URL" << gst_url_ << "to" << next_gst_url_; next_uri_set_ = false; - + about_to_finish_ = false; media_url_ = next_media_url_; stream_url_ = next_stream_url_; gst_url_ = next_gst_url_; @@ -1654,7 +1659,7 @@ void GstEnginePipeline::RemoveAllBufferConsumers() { buffer_consumers_.clear(); } -void GstEnginePipeline::SetNextUrl(const QUrl &media_url, const QUrl &stream_url, const QByteArray &gst_url, const qint64 beginning_nanosec, const qint64 end_nanosec) { +void GstEnginePipeline::PrepareNextUrl(const QUrl &media_url, const QUrl &stream_url, const QByteArray &gst_url, const qint64 beginning_nanosec, const qint64 end_nanosec) { next_media_url_ = media_url; next_stream_url_ = stream_url; @@ -1662,4 +1667,21 @@ void GstEnginePipeline::SetNextUrl(const QUrl &media_url, const QUrl &stream_url next_beginning_offset_nanosec_ = beginning_nanosec; next_end_offset_nanosec_ = end_nanosec; + if (about_to_finish_) { + SetNextUrl(); + } + +} + +void GstEnginePipeline::SetNextUrl() { + + if (about_to_finish_ && has_next_valid_url() && !next_uri_set_) { + // Set the next uri. When the current song ends it will be played automatically and a STREAM_START message is send to the bus. + // When the next uri is not playable an error message is send when the pipeline goes to PLAY (or PAUSE) state or immediately if it is currently in PLAY state. + next_uri_set_ = true; + qLog(Debug) << "Setting next URL to" << next_gst_url_; + g_object_set(G_OBJECT(pipeline_), "uri", next_gst_url_.constData(), nullptr); + about_to_finish_ = false; + } + } diff --git a/src/engine/gstenginepipeline.h b/src/engine/gstenginepipeline.h index 9be8d5727..634a3d215 100644 --- a/src/engine/gstenginepipeline.h +++ b/src/engine/gstenginepipeline.h @@ -95,7 +95,8 @@ class GstEnginePipeline : public QObject { void StartFader(const qint64 duration_nanosec, const QTimeLine::Direction direction = QTimeLine::Forward, const QEasingCurve::Type shape = QEasingCurve::Linear, const bool use_fudge_timer = true); // If this is set then it will be loaded automatically when playback finishes for gapless playback - void SetNextUrl(const QUrl &media_url, const QUrl &stream_url, const QByteArray &gst_url, const qint64 beginning_nanosec, const qint64 end_nanosec); + void PrepareNextUrl(const QUrl &media_url, const QUrl &stream_url, const QByteArray &gst_url, const qint64 beginning_nanosec, const qint64 end_nanosec); + void SetNextUrl(); bool has_next_valid_url() const { return next_stream_url_.isValid(); } void SetSourceDevice(const QString &device) { source_device_ = device; } @@ -140,6 +141,8 @@ class GstEnginePipeline : public QObject { void BufferingProgress(const int percent); void BufferingFinished(); + void AboutToFinish(); + protected: void timerEvent(QTimerEvent*) override; @@ -317,6 +320,8 @@ class GstEnginePipeline : public QObject { bool logged_unsupported_analyzer_format_; + bool about_to_finish_; + }; #endif // GSTENGINEPIPELINE_H