From 88a63005096a3598701fcda1bbc7b926144895be Mon Sep 17 00:00:00 2001 From: David Sansome Date: Tue, 14 Oct 2014 19:25:39 +1100 Subject: [PATCH] Completely revert the Spotify seeking stuff - it's hacky and I don't like waiting 4 seconds to have my songs start. I'll reimplement it properly later. Reverts commits 96387803cde43c282c029b9524573a1474851340 and 160b151652c47ffcec53ea22720bb2517e502799. --- ext/clementine-spotifyblob/mediapipeline.cpp | 5 +-- ext/clementine-spotifyblob/spotifyclient.cpp | 16 +------- ext/clementine-spotifyblob/spotifyclient.h | 1 - .../spotifymessages.proto | 5 +-- src/engines/gstenginepipeline.cpp | 40 ------------------- src/engines/gstenginepipeline.h | 8 ---- src/internet/spotifyserver.cpp | 2 - src/internet/spotifyserver.h | 1 - src/internet/spotifyservice.cpp | 9 ++--- src/internet/spotifyservice.h | 2 +- 10 files changed, 10 insertions(+), 79 deletions(-) diff --git a/ext/clementine-spotifyblob/mediapipeline.cpp b/ext/clementine-spotifyblob/mediapipeline.cpp index d7c973a4f0..717571359e 100644 --- a/ext/clementine-spotifyblob/mediapipeline.cpp +++ b/ext/clementine-spotifyblob/mediapipeline.cpp @@ -81,9 +81,8 @@ bool MediaPipeline::Init(int sample_rate, int channels) { // Try to send 5 seconds of audio in advance to initially fill Clementine's // buffer. - // Commented for now as otherwise the seek will take too long. - //g_object_set(G_OBJECT(tcpsink_), "ts-offset", qint64(-5 * kNsecPerSec), - // nullptr); + g_object_set(G_OBJECT(tcpsink_), "ts-offset", qint64(-5 * kNsecPerSec), + nullptr); // We know the time of each buffer g_object_set(G_OBJECT(appsrc_), "format", GST_FORMAT_TIME, nullptr); diff --git a/ext/clementine-spotifyblob/spotifyclient.cpp b/ext/clementine-spotifyblob/spotifyclient.cpp index f34f1fb6d7..ac0ffd3fec 100644 --- a/ext/clementine-spotifyblob/spotifyclient.cpp +++ b/ext/clementine-spotifyblob/spotifyclient.cpp @@ -758,9 +758,6 @@ int SpotifyClient::MusicDeliveryCallback(sp_session* session, } if (num_frames == 0) { - // According to libspotify documentation, this occurs when a discontinuity - // has occurred (such as after a seek). Maybe should clear buffers here as - // well? (in addition of clearing buffers in gstenginepipeline.cpp) return 0; } @@ -919,17 +916,8 @@ void SpotifyClient::StartPlayback(const pb::spotify::PlaybackRequest& req) { } void SpotifyClient::Seek(qint64 offset_nsec) { - if (sp_session_player_seek(session_, offset_nsec / kNsecPerMsec) - != SP_ERROR_OK) { - qLog(Error) << "Seek error"; - return; - } - - pb::spotify::Message message; - - pb::spotify::SeekCompleted* response = message.mutable_seek_completed(); - Q_UNUSED(response); - SendMessage(message); + // TODO + qLog(Error) << "TODO seeking"; } void SpotifyClient::TryPlaybackAgain(const PendingPlaybackRequest& req) { diff --git a/ext/clementine-spotifyblob/spotifyclient.h b/ext/clementine-spotifyblob/spotifyclient.h index 1a8d698b19..ba676e5618 100644 --- a/ext/clementine-spotifyblob/spotifyclient.h +++ b/ext/clementine-spotifyblob/spotifyclient.h @@ -59,7 +59,6 @@ class SpotifyClient : public AbstractMessageHandler { pb::spotify::LoginResponse_Error error_code); void SendPlaybackError(const QString& error); void SendSearchResponse(sp_search* result); - void SendSeekCompleted(); // Spotify session callbacks. static void SP_CALLCONV LoggedInCallback(sp_session* session, sp_error error); diff --git a/ext/libclementine-spotifyblob/spotifymessages.proto b/ext/libclementine-spotifyblob/spotifymessages.proto index 64b2443b6f..063d4be7aa 100644 --- a/ext/libclementine-spotifyblob/spotifymessages.proto +++ b/ext/libclementine-spotifyblob/spotifymessages.proto @@ -176,9 +176,6 @@ message SeekRequest { optional int64 offset_nsec = 1; } -message SeekCompleted { -} - enum Bitrate { Bitrate96k = 1; Bitrate160k = 2; @@ -229,7 +226,7 @@ message Message { optional BrowseToplistRequest browse_toplist_request = 19; optional BrowseToplistResponse browse_toplist_response = 20; optional PauseRequest pause_request = 21; - optional SeekCompleted seek_completed = 22; + // ID 22 unused. optional AddTracksToPlaylistRequest add_tracks_to_playlist = 23; optional RemoveTracksFromPlaylistRequest remove_tracks_from_playlist = 24; } diff --git a/src/engines/gstenginepipeline.cpp b/src/engines/gstenginepipeline.cpp index 063b15f438..da89a816e6 100644 --- a/src/engines/gstenginepipeline.cpp +++ b/src/engines/gstenginepipeline.cpp @@ -68,7 +68,6 @@ GstEnginePipeline::GstEnginePipeline(GstEngine* engine) buffering_(false), mono_playback_(false), end_offset_nanosec_(-1), - spotify_offset_(0), next_beginning_offset_nanosec_(-1), next_end_offset_nanosec_(-1), ignore_next_seek_(false), @@ -96,19 +95,6 @@ GstEnginePipeline::GstEnginePipeline(GstEngine* engine) } for (int i = 0; i < kEqBandCount; ++i) eq_band_gains_ << 0; - - // FIXME Currently useless Spotify hack: we currently don't know what to do - // when the seek is completed. We should flush the current buffers, but see - // comments in SpotifySeekCompleted for why that doesn't work. - // If we fix and reactivate this code, we will have another problem though: - // calling server() try to login the user. If the user doesn't use Spotify, it - // will receive an error message. We should have a lightweight version of - // server() that just return it (or NULL) without trying to create it IMO to - // avoid this issue. - //if (InternetModel::Service()->IsBlobInstalled()) { - // connect(InternetModel::Service()->server(), SIGNAL(SeekCompleted()), - // SLOT(SpotifySeekCompleted())); - //} } void GstEnginePipeline::set_output_device(const QString& sink, @@ -184,7 +170,6 @@ bool GstEnginePipeline::ReplaceDecodeBin(const QUrl& url) { // Tell spotify to start sending data to us. InternetModel::Service()->server()->StartPlaybackLater( url.toString(), port); - spotify_offset_ = 0; } else { new_bin = engine_->CreateElement("uridecodebin"); g_object_set(G_OBJECT(new_bin), "uri", url.toEncoded().constData(), @@ -979,10 +964,6 @@ qint64 GstEnginePipeline::position() const { gint64 value = 0; gst_element_query_position(pipeline_, GST_FORMAT_TIME, &value); - if (url_.scheme() == "spotify") { - value += spotify_offset_; - } - return value; } @@ -1030,17 +1011,6 @@ bool GstEnginePipeline::Seek(qint64 nanosec) { return true; } - if (url_.scheme() == "spotify" && !buffering_) { - SpotifyService* spotify = InternetModel::Service(); - // Need to schedule this in the spotify service's thread - QMetaObject::invokeMethod(spotify, "Seek", Qt::QueuedConnection, - Q_ARG(qint64, nanosec)); - // Need to reset spotify_offset_ to get the real pipeline position, as it is - // used in position() - spotify_offset_ = nanosec - position(); - return true; - } - if (!pipeline_is_connected_ || !pipeline_is_initialised_) { pending_seek_nanosec_ = nanosec; return true; @@ -1051,16 +1021,6 @@ bool GstEnginePipeline::Seek(qint64 nanosec) { GST_SEEK_FLAG_FLUSH, nanosec); } -void GstEnginePipeline::SpotifySeekCompleted() { - qLog(Debug) << "Spotify Seek completed"; - // FIXME: we should clear buffers to start playing data from seek point right - // now (currently there is small delay) but I didn't managed to tell gstreamer - // to do this without breaking the streaming completely... - // Funny thing to notice: for me the delay varies when changing buffer size, - // but a larger buffer doesn't necessary increase the delay. - // FIXME: also, this method is never called currently (see constructor) -} - void GstEnginePipeline::SetEqualizerEnabled(bool enabled) { eq_enabled_ = enabled; UpdateEqualizer(); diff --git a/src/engines/gstenginepipeline.h b/src/engines/gstenginepipeline.h index a3a297108e..e36a926f99 100644 --- a/src/engines/gstenginepipeline.h +++ b/src/engines/gstenginepipeline.h @@ -164,7 +164,6 @@ class GstEnginePipeline : public QObject { private slots: void FaderTimelineFinished(); - void SpotifySeekCompleted(); private: static const int kGstStateTimeoutNanosecs; @@ -230,13 +229,6 @@ class GstEnginePipeline : public QObject { // past this position. qint64 end_offset_nanosec_; - // Another Spotify hack... - // Used in position(). We need this because when seeking Spotify tracks, we - // don't actually seek the pipeline, but ask libspotify to send us data with - // a seek offset instead. So querying the pipeline to get track's position - // wouldn't make sense. - qint64 spotify_offset_; - // We store the beginning and end for the preloading song too, so we can just // carry on without reloading the file if the sections carry on from each // other. diff --git a/src/internet/spotifyserver.cpp b/src/internet/spotifyserver.cpp index 3e5409ca93..6e8a54cf03 100644 --- a/src/internet/spotifyserver.cpp +++ b/src/internet/spotifyserver.cpp @@ -155,8 +155,6 @@ void SpotifyServer::MessageArrived(const pb::spotify::Message& message) { emit AlbumBrowseResults(message.browse_album_response()); } else if (message.has_browse_toplist_response()) { emit ToplistBrowseResults(message.browse_toplist_response()); - } else if (message.has_seek_completed()) { - emit SeekCompleted(); } } diff --git a/src/internet/spotifyserver.h b/src/internet/spotifyserver.h index 89e43fdad6..f360304cd6 100644 --- a/src/internet/spotifyserver.h +++ b/src/internet/spotifyserver.h @@ -75,7 +75,6 @@ class SpotifyServer : public AbstractMessageHandler { void SyncPlaylistProgress(const pb::spotify::SyncPlaylistProgress& progress); void AlbumBrowseResults(const pb::spotify::BrowseAlbumResponse& response); void ToplistBrowseResults(const pb::spotify::BrowseToplistResponse& response); - void SeekCompleted(); protected: void MessageArrived(const pb::spotify::Message& message); diff --git a/src/internet/spotifyservice.cpp b/src/internet/spotifyservice.cpp index 745101174e..376536f591 100644 --- a/src/internet/spotifyservice.cpp +++ b/src/internet/spotifyservice.cpp @@ -585,6 +585,10 @@ QList SpotifyService::playlistitem_actions(const Song& song) { return playlistitem_actions_; } +PlaylistItem::Options SpotifyService::playlistitem_options() const { + return PlaylistItem::SeekDisabled; +} + QWidget* SpotifyService::HeaderWidget() const { if (IsLoggedIn()) return search_box_; return nullptr; @@ -767,11 +771,6 @@ void SpotifyService::SetPaused(bool paused) { server_->SetPaused(paused); } -void SpotifyService::Seek(qint64 offset_nsec) { - EnsureServerCreated(); - server_->Seek(offset_nsec); -} - void SpotifyService::SyncPlaylistProgress( const pb::spotify::SyncPlaylistProgress& progress) { qLog(Debug) << "Sync progress:" << progress.sync_progress(); diff --git a/src/internet/spotifyservice.h b/src/internet/spotifyservice.h index b157dc3119..35d34dfa96 100644 --- a/src/internet/spotifyservice.h +++ b/src/internet/spotifyservice.h @@ -56,13 +56,13 @@ class SpotifyService : public InternetService { void ItemDoubleClicked(QStandardItem* item); void DropMimeData(const QMimeData* data, const QModelIndex& index); QList playlistitem_actions(const Song& song) override; + PlaylistItem::Options playlistitem_options() const; QWidget* HeaderWidget() const; void Logout(); void Login(const QString& username, const QString& password); Q_INVOKABLE void LoadImage(const QString& id); Q_INVOKABLE void SetPaused(bool paused); - Q_INVOKABLE void Seek(qint64 offset_nsec); SpotifyServer* server() const;