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

Port to GStreamer 1.x and PyGI #1419

Merged
merged 92 commits into from Feb 2, 2016
Merged

Port to GStreamer 1.x and PyGI #1419

merged 92 commits into from Feb 2, 2016

Conversation

jodal
Copy link
Member

@jodal jodal commented Jan 31, 2016

This pull request is identical to what we've had in the feature/gst1 branch the last few weeks, just rebased on top of develop.

It switches Mopidy from GStreamer 0.10 to 1.2+ and the GStreamer Python bindings from PyGST to PyGI. (Fixes #225)

On a related note, this is the last major blocker for running Mopidy on Python 3, ref. #779.

It was a workaround for icy:// support on GStreamer 0.10.
GStreamer no longer use sys.argv directly. If you want GStreamer to
handle command line arguments, you must pass them explicitly to
Gst.init().
And audio/x-raw-int and audio/x-raw-float with audio/x-raw
Audio
-----

- **Breaking:** The audio scanner now returns ISO-8601 formatted strings
Copy link
Member

Choose a reason for hiding this comment

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

@tkem this one is probably relevant for you with respect to mopidy-local-sqlite. Other uses of the scanner is mostly tunein and the likes, @kingosticks I presume this won't effect that code?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think I looked into Mopidy-Local-* at some point looking for issues with this without finding anything, but it would be great if @tkem checked himself.

Copy link
Member

Choose a reason for hiding this comment

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

I don't think Tunein is using the Scanner instance it creates any more. Looks like it's in a bit of a state after that last PRs I merged. So no, no issue there.

Copy link
Member

Choose a reason for hiding this comment

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

mopidy-local-sqlite doesn't use the audio scanner directly, but only uses the Track as passed by the local library interface.
mopidy-local-images does use the tags from the audio scanner, but only looks for image and preview-image tags.
So there shouldn't be any issues.

Copy link
Member Author

Choose a reason for hiding this comment

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

We should be all good then. Mopidy-Spotify is the only extension that needs a new release to work with Mopidy 1.2 (which in semver should be named 2.0, but, well, we control the only thing that is broken).

@adamcik
Copy link
Member

adamcik commented Feb 1, 2016

Think that is it for this round.

If GStreamer is too old, it fails like this:

  $ mopidy
  ERROR: Mopidy requires GStreamer >= 1.2, but found GStreamer 1.0.0.
@jodal
Copy link
Member Author

jodal commented Feb 1, 2016

I think that addressed all comments, if you add your _query_duration() comment and set-to-blank-data TODO.


audio/x-raw,format=S16LE,rate=44100,channels=2,layout=interleaved

If you Mopidy backend uses ``set_appsrc()``, please refer to GStreamer
Copy link
Member

Choose a reason for hiding this comment

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

s/you/your/

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks!


:param taglist: A GStreamer taglist to be converted.
:type taglist: :class:`Gst.TagList`
:rtype: dictionary of tag keys with a list of values.
Copy link
Member

Choose a reason for hiding this comment

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

Don't we normally express returning dictionaries as {tag: list of plain Python types} ?

Copy link
Member Author

Choose a reason for hiding this comment

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

As soon as we get to Python 3 only (Mopidy 3.0?) I want to replace all of these rtype declarations with optional typing, e.g. Dict[str, List]. For now, I'm not sure we have a clear convention.

jodal added a commit that referenced this pull request Feb 2, 2016
Port to GStreamer 1.x and PyGI
@jodal jodal merged commit 3429487 into develop Feb 2, 2016
@jodal jodal deleted the feature/gst1 branch February 2, 2016 21:24
@jodal jodal self-assigned this Feb 2, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-audio Area: Audio layer
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants