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

Gapless aware seek handling #1346

Merged
merged 8 commits into from Dec 5, 2015

Conversation

2 participants
@adamcik
Member

adamcik commented Dec 4, 2015

Fixes: #312, #1304 (partially), #1305

Changelog to follow once I've reviewed the diff again.

@adamcik adamcik added the A-core label Dec 4, 2015

@adamcik adamcik added this to the v1.2 - Gapless milestone Dec 4, 2015

adamcik added some commits Dec 3, 2015

core: Switch back to correct track if seek happens before stream changed
Technically the seek still needs to be postponed for this to work right, but
it's a step closer.
core: Trigger position changed from audio events.
Makes sure to only fire when the position changed to our intended seek target.
Otherwise we would also be triggering this when playback starts.
core: Return pending position during active seek.
This covers over that audio will fail query position while a seek is in
progress. It also means that instead of returning zero we at least return
something which is much closer to the time that we will soon end up playing
from.

@adamcik adamcik force-pushed the adamcik:feature/eot-seek-handling branch from bbeea33 to 9f23757 Dec 4, 2015

@adamcik adamcik referenced this pull request Dec 4, 2015

Merged

MPD idle cleanup #1347

@@ -16,6 +16,11 @@ Core API
- Start ``tlid`` counting at 1 instead of 0 to keep in sync with MPD's
``songid``.

- `get_time_position` now returns the seek target while a seek is in progress.

This comment has been minimized.

@jodal

jodal Dec 4, 2015

Member

Either you need double backtick quotes or use :meth:~mopidy.core.PlaybackController.get_time_position.

- `get_time_position` now returns the seek target while a seek is in progress.
This gives better results than just failing the position query.

(Fixes: :issue:`312` PR: :pr:`1346`)

This comment has been minimized.

@jodal

jodal Dec 4, 2015

Member

Put this in the same paragraph as the rest.

We don't have support for :pr: yet, it was only suggested on IRC. Feel free to add it to docs/conf.py.

@@ -83,6 +88,11 @@ Gapless
- Tests have been updated to always use a core actor so async state changes
don't trip us up.

- Seek events are now triggered when the seek completes. Further changes have

This comment has been minimized.

@jodal

jodal Dec 4, 2015

Member

Sounds like we didn't trigger that event at all before. Need to explain what the old behavior was.

- Seek events are now triggered when the seek completes. Further changes have
been made to make seek work correctly for gapless related corner cases.

(Fixes: :issue:`1305` PR: :pr:`1346`)

This comment has been minimized.

@jodal

jodal Dec 4, 2015

Member

Same comments as above.

@jodal

This comment has been minimized.

Member

jodal commented Dec 4, 2015

Reviewed commit by commit. LGTM.

@jodal jodal self-assigned this Dec 4, 2015

@adamcik

This comment has been minimized.

Member

adamcik commented Dec 5, 2015

Done.

- Seek events are now triggered when the seek completes. Previously the event
was emitted when the seek was requested, not when it completed. Further
changes have been made to make seek work correctly for gapless related corner
cases. (Fixes: :issue:`1305` PR: :pr:`1346`)

This comment has been minimized.

@jodal

jodal Dec 5, 2015

Member

"Same comments as above" means that :pr: must be replaced with :issue: here as well. ;-)

@adamcik

This comment has been minimized.

Member

adamcik commented Dec 5, 2015

Done

jodal added a commit that referenced this pull request Dec 5, 2015

@jodal jodal merged commit 23cdeb2 into mopidy:develop Dec 5, 2015

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@adamcik adamcik deleted the adamcik:feature/eot-seek-handling branch Dec 5, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment