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

Seeking to a non-integer position hangs clients #1756

Closed
roosemberth opened this issue Mar 28, 2019 · 2 comments
Closed

Seeking to a non-integer position hangs clients #1756

roosemberth opened this issue Mar 28, 2019 · 2 comments
Assignees
Labels
good first issue Call for participation
Milestone

Comments

@roosemberth
Copy link

Description: as indicated by the title
Expected behaviour: Seeking to a non-integer position should have the same behavior as seeking to integer seconds.

Steps to reproduce: Using python-mpd2, while playing any song using mopidy:

#! /usr/bin/env python3

from mpd import MPDClient

c = MPDClient()
c.connect("localhost", 6600)
c.seekcur('0.1')

The above code exits cleanly when using mpd but hangs when using mopidy.

@jodal
Copy link
Member

jodal commented Mar 28, 2019

From the Mopidy log when reproducing this:

ERROR    Unhandled exception in MpdSession (urn:uuid:7f76ab06-22c4-4e35-8230-80bd7e6176cc):
Traceback (most recent call last):
  File "/home/jodal/dev/virtualenvs/mopidy/local/lib/python2.7/site-packages/pykka/_actor.py", line 193, in _actor_loop
    response = self._handle_receive(envelope.message)
  File "/home/jodal/dev/virtualenvs/mopidy/local/lib/python2.7/site-packages/pykka/_actor.py", line 307, in _handle_receive
    return self.on_receive(message)
  File "/home/jodal/mopidy-dev/mopidy/mopidy/internal/network.py", line 423, in on_receive
    self.on_line_received(line)
  File "/home/jodal/mopidy-dev/mopidy/mopidy/mpd/session.py", line 34, in on_line_received
    response = self.dispatcher.handle_request(line)
  File "/home/jodal/mopidy-dev/mopidy/mopidy/mpd/dispatcher.py", line 47, in handle_request
    return self._call_next_filter(request, response, filter_chain)
  File "/home/jodal/mopidy-dev/mopidy/mopidy/mpd/dispatcher.py", line 69, in _call_next_filter
    return next_filter(request, response, filter_chain)
  File "/home/jodal/mopidy-dev/mopidy/mopidy/mpd/dispatcher.py", line 77, in _catch_mpd_ack_errors_filter
    return self._call_next_filter(request, response, filter_chain)
  File "/home/jodal/mopidy-dev/mopidy/mopidy/mpd/dispatcher.py", line 69, in _call_next_filter
    return next_filter(request, response, filter_chain)
  File "/home/jodal/mopidy-dev/mopidy/mopidy/mpd/dispatcher.py", line 87, in _authenticate_filter
    return self._call_next_filter(request, response, filter_chain)
  File "/home/jodal/mopidy-dev/mopidy/mopidy/mpd/dispatcher.py", line 69, in _call_next_filter
    return next_filter(request, response, filter_chain)
  File "/home/jodal/mopidy-dev/mopidy/mopidy/mpd/dispatcher.py", line 106, in _command_list_filter
    response = self._call_next_filter(request, response, filter_chain)
  File "/home/jodal/mopidy-dev/mopidy/mopidy/mpd/dispatcher.py", line 69, in _call_next_filter
    return next_filter(request, response, filter_chain)
  File "/home/jodal/mopidy-dev/mopidy/mopidy/mpd/dispatcher.py", line 135, in _idle_filter
    response = self._call_next_filter(request, response, filter_chain)
  File "/home/jodal/mopidy-dev/mopidy/mopidy/mpd/dispatcher.py", line 69, in _call_next_filter
    return next_filter(request, response, filter_chain)
  File "/home/jodal/mopidy-dev/mopidy/mopidy/mpd/dispatcher.py", line 148, in _add_ok_filter
    response = self._call_next_filter(request, response, filter_chain)
  File "/home/jodal/mopidy-dev/mopidy/mopidy/mpd/dispatcher.py", line 69, in _call_next_filter
    return next_filter(request, response, filter_chain)
  File "/home/jodal/mopidy-dev/mopidy/mopidy/mpd/dispatcher.py", line 160, in _call_handler_filter
    response = self._format_response(self._call_handler(request))
  File "/home/jodal/mopidy-dev/mopidy/mopidy/mpd/dispatcher.py", line 175, in _call_handler
    return protocol.commands.call(tokens, context=self.context)
  File "/home/jodal/mopidy-dev/mopidy/mopidy/mpd/protocol/__init__.py", line 180, in call
    return self.handlers[tokens[0]](context, *tokens[1:])
  File "/home/jodal/mopidy-dev/mopidy/mopidy/mpd/protocol/__init__.py", line 158, in validate
    return func(**callargs)
  File "/home/jodal/mopidy-dev/mopidy/mopidy/mpd/protocol/playback.py", line 381, in seekcur
    position = protocol.UINT(time) * 1000
  File "/home/jodal/mopidy-dev/mopidy/mopidy/mpd/protocol/__init__.py", line 53, in UINT
    raise ValueError('Only positive numbers are allowed')
ValueError: Only positive numbers are allowed

Translated to English, Mopidy probably correctly validates that the received value should be an unsigned int, but isn't.

The bug is that it from the client's perspective, Mopidy doesn't handle the problem the same way as the MPD server does.

Next steps here would be to reproduce with MPD and note down details like:

  • Does MPD respond differently to the commands seekcur 10 and seekcur 10.1, or does it respond with "OK" for both? Run e.g. telnet localhost 6600 and write the commands to test.
  • Does MPD actually seek to the position 10.1, or is the seek command silently ignored when the new position is not an integer?

Then, update Mopidy's test suite accordingly and update the implementation at https://github.com/mopidy/mopidy/blob/develop/mopidy/mpd/protocol/playback.py#L366-L382 to match MPD's behavior.

@jodal jodal added this to the v2.2.3 milestone Mar 28, 2019
@jodal jodal added A-mpd good first issue Call for participation labels Mar 28, 2019
@gmkey
Copy link

gmkey commented May 9, 2019

First finding: I can confirm MPD actually seeks to the position 10.1

@jodal jodal modified the milestones: v2.2.3, v2.2.4 Jun 19, 2019
@kingosticks kingosticks modified the milestones: v2.2.4, v2.3.0 Sep 26, 2019
@jodal jodal self-assigned this Sep 30, 2019
jodal added a commit to jodal/mopidy that referenced this issue Sep 30, 2019
@jodal jodal closed this as completed in ede267c Oct 1, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Call for participation
Projects
None yet
Development

No branches or pull requests

4 participants