Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Minimizing audio pop with owntone + metadata pipe #389

Closed
yepitschunked opened this issue Aug 17, 2021 · 2 comments
Closed

Minimizing audio pop with owntone + metadata pipe #389

yepitschunked opened this issue Aug 17, 2021 · 2 comments

Comments

@yepitschunked
Copy link

As mentioned in owntone/owntone-server#295 (comment), the metadata pipe support makes owntone much more responsive. However, there's a distracting ~0.2s of audio pop at the start of every user-initiated track change. My guess of what's happening is this:

  1. owntone has its own internal buffer of 2s of audio
  2. upon receiving the flush message, owntone clears its buffer and continues to read from pipe
  3. however, unless we got lucky with perfect alignment between the flush event and start of the new track audio arriving, the pipe will contain [end of last song----|----start of new song-------]. owntone blindly plays this data (since it's just a dumb pipe) and we get the pop.

I think this is fundamentally hard to solve because because the metadata change and the contents of the audio pipe are not in sync, and owntone has no idea when the next song is starting (it's just playing the contents of the pipe). However, I think silence/some additional delay would be preferable to the noise.

I couldn't find an event in the player code that corresponded to the new stream actually starting (onTrackChanged is fired immediately), so onMetadataAvailable seemed like the next best thing. Moving the dacpPipe.sendPipeFlush(); call here minimized the sound to a brief click, which makes me feel like this hypothesis is on the right track. However, moving it here kinda sucks because it really ought to be a track change callback, and it affects non user-initiated track changes too.

Wondering if you have some ideas. I think the ideal time to send the flush message would be right before the new track audio is about to be written to the pipe.

@devgianlu
Copy link
Member

(sorry for the late reply)

I have added an onFinishedLoading listener here. That should be the closest thing to the first bytes being written to the output. Let me know how it works so that I can change where the flush happens with another commit.

@foxy82
Copy link

foxy82 commented Oct 16, 2021

Hi - I just tried this out and can confirm that removing:
if (userInitiated) dacpPipe.sendPipeFlush();
from onTrackChanged

And adding:
dacpPipe.sendPipeFlush();
to onFinishedLoading

Changes it from an obvious 2 seconds of the previous track to a tiny/v. short crackle sound.

my git diff:

diff --git a/player/src/main/java/xyz/gianlu/librespot/player/Player.java b/player/src/main/java/xyz/gianlu/librespot/player/Player.java
index 7f049b8..bddf2db 100644
--- a/player/src/main/java/xyz/gianlu/librespot/player/Player.java
+++ b/player/src/main/java/xyz/gianlu/librespot/player/Player.java
@@ -981,7 +981,7 @@ public class Player implements Closeable {
 
                     @Override
                     public void onTrackChanged(@NotNull Player player, @NotNull PlayableId id, @Nullable MetadataWrapper metadata, boolean userInitiated) {
-                        if (userInitiated) dacpPipe.sendPipeFlush();
+//                        if (userInitiated) dacpPipe.sendPipeFlush();
                     }
 
                     @Override
@@ -1052,6 +1052,7 @@ public class Player implements Closeable {
 
                     @Override
                     public void onFinishedLoading(@NotNull Player player) {
+                        dacpPipe.sendPipeFlush();
                     }
                 });
             }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants