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

Ignore position of _on_position_changed callback. Fixes #1462 #1496

Merged
merged 3 commits into from Jun 13, 2016

Conversation

2 participants
@dublok
Contributor

dublok commented Apr 4, 2016

@adamcik Is there a reason why you check for position match in _on_position_changed? It seems there is only one _on_position_changed for each seek. (I tried to, but didn't get two simultaneous seeks)

Fixes #1462.

@adamcik

This comment has been minimized.

Member

adamcik commented Apr 4, 2016

@dublok

This comment has been minimized.

Contributor

dublok commented Apr 4, 2016

Shall I close this because ist the wrong way to ignore position?
Or shall I rework the test (as I have forgotten to do a tox before)

@dublok

This comment has been minimized.

Contributor

dublok commented Apr 4, 2016

@adamcik I still have one failing test. Please check if this PR has to be rejected or if the test does not simulate the real behaviour.

test_seek_race_condition_emits_events fails. It expects a single seeked event and get seeked + playback ended

With a real running mopidy I only get the single seeked event (as expected)

INFO     _on_about_to_finish_callback
INFO     _on_position_changed 85000 0
WARNING  Triggering seeked event
@dublok

This comment has been minimized.

Contributor

dublok commented Apr 9, 2016

Changed the test. It now records only events triggered after seek.

@dublok dublok changed the title from Ignore position of _on_position_changed callback to Ignore position of _on_position_changed callback. Fixes #1462 Apr 10, 2016

@adamcik adamcik added this to the v2.1 - The rest of v2.0 milestone Jun 13, 2016

@adamcik adamcik added the A-audio label Jun 13, 2016

@adamcik adamcik merged commit 431984d into mopidy:develop Jun 13, 2016

1 check passed

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

This comment has been minimized.

Member

adamcik commented Jun 13, 2016

Thanks, now I just have to remember to add a changelog entry...

adamcik added a commit that referenced this pull request Jul 25, 2016

Merge pull request #1496 from dublok/fix/1462-flac-seek-freeze
audio: Ignore position of _on_position_changed callback (fixes #1462)

@dublok dublok deleted the dublok:fix/1462-flac-seek-freeze branch Sep 14, 2016

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