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

Soaku gapless playback #277

Merged
merged 5 commits into from
Nov 24, 2020
Merged

Conversation

kingosticks
Copy link
Member

This is #269 with the remaining CI fixes. And also with the held buffer changed from a global to an instance variable as the music_delivery_callback tests were no longer doing the right thing.

@codecov
Copy link

codecov bot commented Sep 29, 2020

Codecov Report

Merging #277 (0ac120d) into master (d77a73b) will increase coverage by 0.03%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #277      +/-   ##
==========================================
+ Coverage   96.64%   96.67%   +0.03%     
==========================================
  Files          13       13              
  Lines        1191     1203      +12     
==========================================
+ Hits         1151     1163      +12     
  Misses         40       40              
Impacted Files Coverage Δ
mopidy_spotify/playback.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d77a73b...0ac120d. Read the comment docs.

@kingosticks
Copy link
Member Author

There is still https://github.com/mopidy/mopidy-spotify/pull/277/files#diff-d09e8ce8f0a316832307028fe46a8319R202 to think about. I think it's actually OK and the None condition is handled by our appsrc.

@kingosticks kingosticks self-assigned this Sep 29, 2020
@kingosticks kingosticks added the C-bug Category: This is a bug label Sep 29, 2020
@ArthaTi
Copy link

ArthaTi commented Oct 31, 2020

Haven't noticed your PR before. I'm glad you fixed it and I think your solution should be okay.

@kingosticks kingosticks mentioned this pull request Nov 23, 2020
Copy link
Member

@jodal jodal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎉

Comment on lines 18 to 21
# Last audio data sent to the buffer, currently on hold. This data is held
# because libspotify sends a single empty buffer before ending the track. It is
# discarded the moment a new track starts so a smooth transition between songs
# can be made.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stray comment? Move into __init__()?

@jodal
Copy link
Member

jodal commented Nov 24, 2020

After an hour or so on video with @kingosticks live-coding now, we've:

  • Made it thread-safe, in case someone changes track in the middle of doing an operation on the held buffer.
  • Made sure to hold on to the right buffer if the appsrc applies backpressure.

Should be ready to merge as soon as Nick is a bit more awake and confident enough :-)

@kingosticks kingosticks force-pushed the Soaku-gapless-playback branch from a6af848 to 0ac120d Compare November 24, 2020 17:38
@jodal jodal merged commit 4009771 into mopidy:master Nov 24, 2020
@ArthaTi
Copy link

ArthaTi commented Nov 25, 2020

This is great news! Thank you!

@kingosticks kingosticks deleted the Soaku-gapless-playback branch January 6, 2021 01:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants