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

Make sure GStreamer callbacks to audio actors are race free #1222

Open
DavisNT opened this issue Jul 17, 2015 · 3 comments
Open

Make sure GStreamer callbacks to audio actors are race free #1222

DavisNT opened this issue Jul 17, 2015 · 3 comments
Assignees
Labels
A-audio Area: Audio layer C-bug Category: This is a bug

Comments

@DavisNT
Copy link
Contributor

DavisNT commented Jul 17, 2015

Sometimes the following is written to stdout/stderr of Mopidy:

Traceback (most recent call last):
  File "/usr/local/lib/python2.7/dist-packages/mopidy/audio/actor.py", line 234, in on_message
    self.on_playbin_state_changed(*msg.parse_state_changed())
  File "/usr/local/lib/python2.7/dist-packages/mopidy/audio/actor.py", line 283, in on_playbin_state_changed
    target_state = _GST_STATE_MAPPING[self._audio._target_state]
KeyError: <enum GST_STATE_READY of type GstState>

Probably this has no noticeable effect on playback (at least I have not correlated it to any issues). May be this error makes sense to @jodal or somebody else who is maintaining Mopidy.

@jodal jodal added the C-bug Category: This is a bug label Jul 20, 2015
@jodal jodal modified the milestone: v1.0.8 Jul 20, 2015
@jodal jodal added the A-audio Area: Audio layer label Jul 20, 2015
@adamcik
Copy link
Member

adamcik commented Jul 22, 2015

Could you try running with mopidy -o loglevels/mopidy.audio.gst=DEBUG to get some more logs. I have a fairly good idea of what is happening, but can't seem to reproduce it.

@adamcik
Copy link
Member

adamcik commented Jul 22, 2015

Oh and what type of URI was this, web stream, local file, spotify, ...?

@adamcik
Copy link
Member

adamcik commented Jul 22, 2015

Never mind, I think I've understood what is happening here. Short version is that I think we have a race condition, but as you noted it should be mostly harmless.

  1. Target state is for instance PLAYING
  2. on_playbin_state_changed(PAUSED, PLAYING, None) gets called indicating that we are done with the switch.
  3. Something else comes along and changes the track, which triggers prepare_change() which in turn causes _target_state to be set to READY
  4. We get a traceback when line 283 gets excuted

Reason this happens is that the on_playbin_state_changed is running from a GStreamer thread so it can be interleaved with the audio actor. So we need to look at all the callbacks we get from GStreamer and make sure that we use instance attributes safely.

@adamcik adamcik modified the milestones: v1.1 - Robust core, v1.0.8 Jul 22, 2015
@adamcik adamcik changed the title KeyError in on_playbin_state_changed() Make sure GStreamer callbacks to audio actors are race free Jul 22, 2015
@adamcik adamcik modified the milestones: v1.2 - Gapless, v1.1 - Robust core Jul 22, 2015
@adamcik adamcik modified the milestones: v1.2.1, v1.2 - Gapless and GStreamer 1.x Feb 5, 2016
jodal added a commit to jodal/mopidy that referenced this issue Feb 10, 2016
Fixes mopidy#1430. See mopidy#1222 for explanation and proper fix.
jodal added a commit to jodal/mopidy that referenced this issue Feb 10, 2016
Fixes mopidy#1430. See mopidy#1222 for explanation and proper fix.
@jodal jodal modified the milestones: v2.0.2 - Bug fixes, v2.0.1 - Bug fixes Aug 7, 2016
@jodal jodal modified the milestones: v2.1.1 - Bug fixes, v2.2 Mar 30, 2018
@jodal jodal removed this from the v2.2 milestone Apr 9, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-audio Area: Audio layer C-bug Category: This is a bug
Projects
None yet
Development

No branches or pull requests

3 participants