From 757f514202edd9b05a327b7c49a43409574d1b23 Mon Sep 17 00:00:00 2001 From: Martin Lehmann Date: Wed, 21 Apr 2021 08:48:10 +0200 Subject: [PATCH 1/6] Handle some Exceptions caused eg. by missing network during shutdown --- .../xyz/gianlu/librespot/core/Session.java | 36 ++++++++++++------- .../player/state/DeviceStateHandler.java | 7 +++- 2 files changed, 29 insertions(+), 14 deletions(-) diff --git a/lib/src/main/java/xyz/gianlu/librespot/core/Session.java b/lib/src/main/java/xyz/gianlu/librespot/core/Session.java index 1df0b9c9..ff04cea7 100644 --- a/lib/src/main/java/xyz/gianlu/librespot/core/Session.java +++ b/lib/src/main/java/xyz/gianlu/librespot/core/Session.java @@ -452,6 +452,8 @@ private void authenticatePartial(@NotNull Authentication.LoginCredentials creden public void close() throws IOException { LOGGER.info("Closing session. {deviceId: {}}", inner.deviceId); + if (scheduledReconnect != null) scheduledReconnect.cancel(true); + closing = true; scheduler.shutdownNow(); @@ -513,7 +515,11 @@ public void close() throws IOException { } private void sendUnchecked(Packet.Type cmd, byte[] payload) throws IOException { - cipherPair.sendEncoded(conn.out, cmd.val, payload); + try { + cipherPair.sendEncoded(conn.out, cmd.val, payload); + } catch (NullPointerException e) { + throw new IOException(e); + } } private void waitAuthLock() { @@ -692,6 +698,7 @@ public Configuration configuration() { } private void reconnect() { + if (!this.closing) { synchronized (reconnectionListeners) { reconnectionListeners.forEach(ReconnectionListener::onConnectionDropped); } @@ -715,15 +722,18 @@ private void reconnect() { synchronized (reconnectionListeners) { reconnectionListeners.forEach(ReconnectionListener::onConnectionEstablished); } - } catch (IOException | GeneralSecurityException | SpotifyAuthenticationException ex) { - conn = null; - LOGGER.error("Failed reconnecting, retrying in 10 seconds...", ex); - - try { - scheduler.schedule(this::reconnect, 10, TimeUnit.SECONDS); - } catch (RejectedExecutionException exx) { - LOGGER.info("Scheduler already shutdown, stopping reconnection", exx); - } + } catch (NullPointerException | IOException | GeneralSecurityException | SpotifyAuthenticationException ex) { + if (!this.closing) { + conn = null; + LOGGER.error("Failed reconnecting, retrying in 10 seconds...", ex); + + try { + scheduler.schedule(this::reconnect, 10, TimeUnit.SECONDS); + } catch (RejectedExecutionException exx) { + LOGGER.info("Scheduler already shutdown, stopping reconnection", exx); + } + } + } } } @@ -1309,8 +1319,8 @@ public void run() { LOGGER.info("Skipping unknown command {cmd: 0x{}, payload: {}}", Integer.toHexString(packet.cmd), Utils.bytesToHex(packet.payload)); continue; } - } catch (IOException | GeneralSecurityException ex) { - if (running) { + } catch (IOException | GeneralSecurityException | NullPointerException ex) { + if (running && !closing) { LOGGER.error("Failed reading packet!", ex); reconnect(); } @@ -1318,7 +1328,7 @@ public void run() { break; } - if (!running) break; + if (!running || closing) break; switch (cmd) { case Ping: diff --git a/player/src/main/java/xyz/gianlu/librespot/player/state/DeviceStateHandler.java b/player/src/main/java/xyz/gianlu/librespot/player/state/DeviceStateHandler.java index ee9adcd2..373ac240 100644 --- a/player/src/main/java/xyz/gianlu/librespot/player/state/DeviceStateHandler.java +++ b/player/src/main/java/xyz/gianlu/librespot/player/state/DeviceStateHandler.java @@ -44,6 +44,7 @@ import java.io.UnsupportedEncodingException; import java.net.URLDecoder; import java.util.*; +import java.util.concurrent.RejectedExecutionException; /** * @author Gianlu @@ -226,7 +227,11 @@ public synchronized void updateState(@NotNull Connect.PutStateReason reason, int .setClientSideTimestamp(TimeProvider.currentTimeMillis()) .getDeviceBuilder().setDeviceInfo(deviceInfo).setPlayerState(state); - putStateWorker.submit(putState.build()); + try { + putStateWorker.submit(putState.build()); + } catch (RejectedExecutionException e){ + LOGGER.debug("Failed to update state, ignoring.", e); + } } public synchronized int getVolume() { From 039c6e9d9e28994d84c82457d0831888beab321d Mon Sep 17 00:00:00 2001 From: Gianlu Date: Wed, 21 Apr 2021 12:30:20 +0200 Subject: [PATCH 2/6] Do not report RejectedExecutionException when closing DeviceStateHandler --- .../librespot/player/state/DeviceStateHandler.java | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/player/src/main/java/xyz/gianlu/librespot/player/state/DeviceStateHandler.java b/player/src/main/java/xyz/gianlu/librespot/player/state/DeviceStateHandler.java index 373ac240..3f62e938 100644 --- a/player/src/main/java/xyz/gianlu/librespot/player/state/DeviceStateHandler.java +++ b/player/src/main/java/xyz/gianlu/librespot/player/state/DeviceStateHandler.java @@ -66,6 +66,7 @@ public final class DeviceStateHandler implements Closeable, DealerClient.Message private final Connect.PutStateRequest.Builder putState; private final AsyncWorker putStateWorker; private volatile String connectionId = null; + private volatile boolean closing = false; public DeviceStateHandler(@NotNull Session session, @NotNull PlayerConfiguration conf) { this.session = session; @@ -228,9 +229,9 @@ public synchronized void updateState(@NotNull Connect.PutStateReason reason, int .getDeviceBuilder().setDeviceInfo(deviceInfo).setPlayerState(state); try { - putStateWorker.submit(putState.build()); - } catch (RejectedExecutionException e){ - LOGGER.debug("Failed to update state, ignoring.", e); + putStateWorker.submit(putState.build()); + } catch (RejectedExecutionException ex) { + if (!closing) LOGGER.error("Failed to submit update state task.", ex); } } @@ -249,6 +250,8 @@ public void setVolume(int val) { @Override public void close() { + closing = true; + session.dealer().removeMessageListener(this); session.dealer().removeRequestListener(this); From 6c5177c84471a34827df3dbc52b17c0b3d8c1027 Mon Sep 17 00:00:00 2001 From: Gianlu Date: Wed, 21 Apr 2021 12:34:42 +0200 Subject: [PATCH 3/6] Minor refactoring --- .../xyz/gianlu/librespot/core/Session.java | 34 ++++++++++--------- 1 file changed, 18 insertions(+), 16 deletions(-) diff --git a/lib/src/main/java/xyz/gianlu/librespot/core/Session.java b/lib/src/main/java/xyz/gianlu/librespot/core/Session.java index ff04cea7..053794c8 100644 --- a/lib/src/main/java/xyz/gianlu/librespot/core/Session.java +++ b/lib/src/main/java/xyz/gianlu/librespot/core/Session.java @@ -516,7 +516,7 @@ public void close() throws IOException { private void sendUnchecked(Packet.Type cmd, byte[] payload) throws IOException { try { - cipherPair.sendEncoded(conn.out, cmd.val, payload); + cipherPair.sendEncoded(conn.out, cmd.val, payload); } catch (NullPointerException e) { throw new IOException(e); } @@ -698,7 +698,9 @@ public Configuration configuration() { } private void reconnect() { - if (!this.closing) { + if (closing) + return; + synchronized (reconnectionListeners) { reconnectionListeners.forEach(ReconnectionListener::onConnectionDropped); } @@ -722,18 +724,18 @@ private void reconnect() { synchronized (reconnectionListeners) { reconnectionListeners.forEach(ReconnectionListener::onConnectionEstablished); } - } catch (NullPointerException | IOException | GeneralSecurityException | SpotifyAuthenticationException ex) { - if (!this.closing) { - conn = null; - LOGGER.error("Failed reconnecting, retrying in 10 seconds...", ex); - - try { - scheduler.schedule(this::reconnect, 10, TimeUnit.SECONDS); - } catch (RejectedExecutionException exx) { - LOGGER.info("Scheduler already shutdown, stopping reconnection", exx); - } - } - } + } catch (NullPointerException | IOException | GeneralSecurityException | SpotifyAuthenticationException ex) { + if (closing) + return; + + conn = null; + LOGGER.error("Failed reconnecting, retrying in 10 seconds...", ex); + + try { + scheduler.schedule(this::reconnect, 10, TimeUnit.SECONDS); + } catch (RejectedExecutionException exx) { + LOGGER.info("Scheduler already shutdown, stopping reconnection", exx); + } } } @@ -1320,7 +1322,7 @@ public void run() { continue; } } catch (IOException | GeneralSecurityException | NullPointerException ex) { - if (running && !closing) { + if (running) { LOGGER.error("Failed reading packet!", ex); reconnect(); } @@ -1328,7 +1330,7 @@ public void run() { break; } - if (!running || closing) break; + if (!running) break; switch (cmd) { case Ping: From 183a030b5512eec7bcf1f4ff5df6c1c5025d9cdf Mon Sep 17 00:00:00 2001 From: Gianlu Date: Wed, 21 Apr 2021 17:28:30 +0200 Subject: [PATCH 4/6] Removed NPE catches + added null checks on connection --- .../xyz/gianlu/librespot/core/Session.java | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/lib/src/main/java/xyz/gianlu/librespot/core/Session.java b/lib/src/main/java/xyz/gianlu/librespot/core/Session.java index 053794c8..0b1609b3 100644 --- a/lib/src/main/java/xyz/gianlu/librespot/core/Session.java +++ b/lib/src/main/java/xyz/gianlu/librespot/core/Session.java @@ -388,7 +388,7 @@ private void authenticate(@NotNull Authentication.LoginCredentials credentials) * {@code true} for {@link Session#reconnect()}. */ private void authenticatePartial(@NotNull Authentication.LoginCredentials credentials, boolean removeLock) throws IOException, GeneralSecurityException, SpotifyAuthenticationException { - if (cipherPair == null) throw new IllegalStateException("Connection not established!"); + if (conn == null || cipherPair == null) throw new IllegalStateException("Connection not established!"); Authentication.ClientResponseEncrypted clientResponseEncrypted = Authentication.ClientResponseEncrypted.newBuilder() .setLoginCredentials(credentials) @@ -409,7 +409,6 @@ private void authenticatePartial(@NotNull Authentication.LoginCredentials creden receiver = new Receiver(); - byte[] bytes0x0f = new byte[20]; random().nextBytes(bytes0x0f); sendUnchecked(Packet.Type.Unknown_0x0f, bytes0x0f); @@ -515,11 +514,10 @@ public void close() throws IOException { } private void sendUnchecked(Packet.Type cmd, byte[] payload) throws IOException { - try { - cipherPair.sendEncoded(conn.out, cmd.val, payload); - } catch (NullPointerException e) { - throw new IOException(e); - } + if (conn == null) + throw new IOException("Cannot write to missing connection."); + + cipherPair.sendEncoded(conn.out, cmd.val, payload); } private void waitAuthLock() { @@ -724,7 +722,7 @@ private void reconnect() { synchronized (reconnectionListeners) { reconnectionListeners.forEach(ReconnectionListener::onConnectionEstablished); } - } catch (NullPointerException | IOException | GeneralSecurityException | SpotifyAuthenticationException ex) { + } catch (IOException | GeneralSecurityException | SpotifyAuthenticationException ex) { if (closing) return; @@ -1321,8 +1319,8 @@ public void run() { LOGGER.info("Skipping unknown command {cmd: 0x{}, payload: {}}", Integer.toHexString(packet.cmd), Utils.bytesToHex(packet.payload)); continue; } - } catch (IOException | GeneralSecurityException | NullPointerException ex) { - if (running) { + } catch (IOException | GeneralSecurityException ex) { + if (running && !closing) { LOGGER.error("Failed reading packet!", ex); reconnect(); } From 5d1b06d318c14fafff3cc6e63a6caf724395226c Mon Sep 17 00:00:00 2001 From: Gianlu Date: Wed, 21 Apr 2021 18:44:15 +0200 Subject: [PATCH 5/6] Allowing programmatic registration of new decoders (#343) --- .../librespot/player/codecs/Codecs.java | 76 +++++++++++++++++++ .../player/playback/PlayerQueueEntry.java | 21 +---- 2 files changed, 80 insertions(+), 17 deletions(-) create mode 100644 player/src/main/java/xyz/gianlu/librespot/player/codecs/Codecs.java diff --git a/player/src/main/java/xyz/gianlu/librespot/player/codecs/Codecs.java b/player/src/main/java/xyz/gianlu/librespot/player/codecs/Codecs.java new file mode 100644 index 00000000..af655042 --- /dev/null +++ b/player/src/main/java/xyz/gianlu/librespot/player/codecs/Codecs.java @@ -0,0 +1,76 @@ +/* + * Copyright 2021 devgianlu + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package xyz.gianlu.librespot.player.codecs; + +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import xyz.gianlu.librespot.audio.GeneralAudioStream; +import xyz.gianlu.librespot.audio.NormalizationData; +import xyz.gianlu.librespot.audio.format.SuperAudioFormat; +import xyz.gianlu.librespot.player.PlayerConfiguration; + +import java.util.*; + +/** + * @author devgianlu + */ +public final class Codecs { + private static final Map>> codecs = new EnumMap<>(SuperAudioFormat.class); + private static final Logger LOGGER = LoggerFactory.getLogger(Codecs.class); + + static { + registerCodec(SuperAudioFormat.VORBIS, VorbisCodec.class); + registerCodec(SuperAudioFormat.MP3, Mp3Codec.class); + } + + private Codecs() { + } + + @Nullable + public static Codec initCodec(@NotNull SuperAudioFormat format, @NotNull GeneralAudioStream audioFile, @Nullable NormalizationData normalizationData, @NotNull PlayerConfiguration conf, int duration) { + Set> set = codecs.get(format); + if (set == null) return null; + + Optional> opt = set.stream().findFirst(); + if (!opt.isPresent()) return null; + + try { + Class clazz = opt.get(); + return clazz.getConstructor(GeneralAudioStream.class, NormalizationData.class, PlayerConfiguration.class, int.class).newInstance(audioFile, normalizationData, conf, duration); + } catch (ReflectiveOperationException ex) { + LOGGER.error("Failed initializing Codec instance for {}", format, ex); + return null; + } + } + + public static void registerCodec(@NotNull SuperAudioFormat format, @NotNull Class clazz) { + codecs.computeIfAbsent(format, (key) -> new HashSet<>(5)).add(clazz); + } + + public static void replaceCodecs(@NotNull SuperAudioFormat format, @NotNull Class clazz) { + Set> set = codecs.get(format); + if (set != null) set.clear(); + registerCodec(format, clazz); + } + + public static void unregisterCodec(@NotNull Class clazz) { + for (Set> set : codecs.values()) + set.remove(clazz); + } +} diff --git a/player/src/main/java/xyz/gianlu/librespot/player/playback/PlayerQueueEntry.java b/player/src/main/java/xyz/gianlu/librespot/player/playback/PlayerQueueEntry.java index fffbad6a..2dc2e9fe 100644 --- a/player/src/main/java/xyz/gianlu/librespot/player/playback/PlayerQueueEntry.java +++ b/player/src/main/java/xyz/gianlu/librespot/player/playback/PlayerQueueEntry.java @@ -16,7 +16,6 @@ package xyz.gianlu.librespot.player.playback; -import javazoom.jl.decoder.BitstreamException; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import org.slf4j.Logger; @@ -33,8 +32,7 @@ import xyz.gianlu.librespot.player.PlayerConfiguration; import xyz.gianlu.librespot.player.StateWrapper; import xyz.gianlu.librespot.player.codecs.Codec; -import xyz.gianlu.librespot.player.codecs.Mp3Codec; -import xyz.gianlu.librespot.player.codecs.VorbisCodec; +import xyz.gianlu.librespot.player.codecs.Codecs; import xyz.gianlu.librespot.player.codecs.VorbisOnlyAudioQuality; import xyz.gianlu.librespot.player.crossfade.CrossfadeController; import xyz.gianlu.librespot.player.metrics.PlaybackMetrics; @@ -132,20 +130,9 @@ private void load(boolean preload) throws IOException, Codec.CodecException, Mer if (crossfade.hasAnyFadeOut() || conf.preloadEnabled) notifyInstant(INSTANT_PRELOAD, (int) (crossfade.fadeOutStartTimeMin() - TimeUnit.SECONDS.toMillis(20))); - switch (stream.in.codec()) { - case VORBIS: - codec = new VorbisCodec(stream.in, stream.normalizationData, conf, metadata.duration()); - break; - case MP3: - try { - codec = new Mp3Codec(stream.in, stream.normalizationData, conf, metadata.duration()); - } catch (BitstreamException ex) { - throw new IOException(ex); - } - break; - default: - throw new UnsupportedEncodingException(stream.in.codec().toString()); - } + codec = Codecs.initCodec(stream.in.codec(), stream.in, stream.normalizationData, conf, metadata.duration()); + if (codec == null) + throw new UnsupportedEncodingException(stream.in.codec().toString()); LOGGER.trace("Loaded {} codec. {of: {}, format: {}, playbackId: {}}", stream.in.codec(), stream.in.describe(), codec.getAudioFormat(), playbackId); } From 0291caa485272e873c9b96d5d3bc07d11016b8b2 Mon Sep 17 00:00:00 2001 From: Gianlu Date: Thu, 22 Apr 2021 08:15:33 +0200 Subject: [PATCH 6/6] Make Codec constructor public (#343) --- .../src/main/java/xyz/gianlu/librespot/player/codecs/Codec.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/player/src/main/java/xyz/gianlu/librespot/player/codecs/Codec.java b/player/src/main/java/xyz/gianlu/librespot/player/codecs/Codec.java index 1c680ad2..10c76773 100644 --- a/player/src/main/java/xyz/gianlu/librespot/player/codecs/Codec.java +++ b/player/src/main/java/xyz/gianlu/librespot/player/codecs/Codec.java @@ -44,7 +44,7 @@ public abstract class Codec implements Closeable { protected int seekZero = 0; private OutputAudioFormat format; - Codec(@NotNull GeneralAudioStream audioFile, @Nullable NormalizationData normalizationData, @NotNull PlayerConfiguration conf, int duration) { + public Codec(@NotNull GeneralAudioStream audioFile, @Nullable NormalizationData normalizationData, @NotNull PlayerConfiguration conf, int duration) { this.audioIn = audioFile.stream(); this.audioFile = audioFile; this.duration = duration;