Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Determine how to handle stream vs track metadata. #270

Open
adamcik opened this Issue · 17 comments

4 participants

@adamcik
Owner

For cases such as last.fm radio or just plain streaming we will have one set of metadata, typically a name and uri for the stream, and the track metadata (possibly without it's own uri). Currently we have no good way of handling this in our core or our frontends/clients.

The goal of this issue is to determine a sensible solution for this that does not break existing special cases such as scrobbling, yet allows for simple implementation in clients via our frontends. Timeframe for this should most likely be linked to getting a streaming backend (#151) and/or a last.fm backend in (#38).

@adamcik adamcik referenced this issue from a commit
Commit has since been removed from the repository and is no longer available.
@adamcik adamcik referenced this issue from a commit in adamcik/mopidy
@adamcik adamcik stream backend: Add StreamBackend, fixes #151
Adds a basic streaming backend simply handles streaming audio and nothing else.
I.e. no metadata beyond the URI we where given. #270 still needs to be solved
for actual metadata to make sense in this backend.
341dea7
@adamcik adamcik referenced this issue from a commit
@adamcik adamcik stream backend: Add StreamBackend, fixes #151
Adds a basic streaming backend simply handles streaming audio and nothing else.
I.e. no metadata beyond the URI we where given. #270 still needs to be solved
for actual metadata to make sense in this backend.
795926c
@hechtus

I was playing around with the streaming backend and getting metadata out of the stream. This was quite simple by acting on the gstreamer message gst.MESSAGE_TAG. I parsed the taglist and got the 'title' and other useful tags. But how to proceed? The frontend needs to informed about new metadata. How could this be done On the other hand I see problems if the metadata conflicts with the existing track info. I think updating the metadata or not should be dependent on the backend playback provider. Any advises from your side?

@adamcik
Owner

I've elsewhere suggested that we switch to always using the pipeline data a the source for truth for currently playing metadata. So we would be using the default metadata from the track to seed the current metadata, and always overwrite it with anything gstreamer gives us. This change should allow us to cleanly handle "normal" tracks and streams in the same way and get a sane behavior without having to worry about distinct track vs stream metadata handling.

As for reading the tag data, see mopidy/scanner.py and look into pulling out the gst.MESSAGE_TAG to mopidy Track code perhaps?

@hechtus

I was simply extending the function Audio::_on_message() like this:

    elif message.type == gst.MESSAGE_TAG:
        taglist = message.parse_tag()
        for key in taglist.keys():
            self._tags[key] = taglist[key]
        print self._tags

This gives me the dictionary self._tags looking like this

{'has-crc': False, 'bitrate': 128000, 'location': u'http://www.ndr.de', 'genre': u'live', 'organization': u'NDR Kultur', 'audio-codec': u'MPEG 1 Audio, Layer 3 (MP3)', 'channel-mode': u'joint-stereo'}

This stream does not have the current track name, but this has:

{'has-crc': False, 'title': u'Auburn Lull - [untitled]', 'bitrate': 192000, 'location': u'http://radio.maschinengeist.org', 'genre': u'Ambient, Drone, Experimental, Downtempo, Space, DreamPop', 'organization': u'[1337 4M8I3N7] MASCHINENGEIST.ORG RADIO Deep Field Transmissions', 'audio-codec': u'MPEG 1 Audio, Layer 3 (MP3)', 'channel-mode': u'stereo'}

The tag dictionary self._tags must be cleared whenever the track is changed. I did this in the prepare_change() function.

@hechtus

... and yes, you are right. The scanner code looks useful. So, the question for me is now, how to "overwrite the default metadata". I found no way to notify the frontend (mpd) about new metadata.

@hechtus

To notify the MPD client about new metadata I sent the 'tracklist_changed' event to the CoreListener. So, the MPD backend sends the 'playlist' IDLE command to the MPD client. The client does (ncmcpp does, gmpc 0.20.0 does not) a 'currentsong' request which could implemented a bit smarter translating the gstreamer tags and merging them into the current track info. What do you think?

@adamcik
Owner

Looking at http://www.musicpd.org/doc/protocol/ch03.html the playlist idle seems to be the right hint to send. Wouldn't hurt to test and see what MPD itself when playing a radio stream that sends metadata in the stream. But it sounds like you are on the right track for fixing this :-) Other thing that comes to mind is that a metadata_changed event in core might make more sense for the core API, end result on the MPD side should still be the same, but makes it clearer what is going on internally.

@hechtus

You could have a look at hechtus/mopidy@059e5f7. I'm unsure where to translate and merge the metadata into the current track data. We could do this in the frontend on currentsong requests, but this would somehow result in duplicated code in the frontends ...

@adamcik
Owner

I was thinking something along the lines of a current_metadata or current_track in core's playback instance as the place to have this info. Then having things like currentsong use that. Where the current_* data is initialized to a track or metadata based on the track that is passed in and then updated with whatever gstreamer might give us.

@hechtus

I already tried something similar. Have a look at hechtus/mopidy@98b4a5a. I'm still messing around on to merge current_track and current_metadata. Should I use copy() to modify individual properties of a Track?

I think parsing the gstreamer metadata and converting it to a mopidy Track should be done in Audio, because it is gstreamer/audio specific. Merging could be done either in frontend or playback. I would also prefer playback somehow.

@adamcik
Owner

Using track.copy(**dict_with_data) should do what you want without changing the current code, unless I misunderstood you?

@adamcik
Owner

Hmm, keeping it in audio sounds like a good call. It would probably make sense to extend the audio listener interface we have to pass the metadata to core, and then have core emit the even metadata changed event as you've already done.

@jodal jodal added this to the v0.20 - Audio and mixer cleanup milestone
@adamcik adamcik removed this from the v0.20 - Audio cleanup milestone
@adamcik
Owner

First part of this has been merged for 0.20, we just forgot to link in this issue.

@adamcik adamcik referenced this issue in AlexandrePTJ/mopidy-somafm
Closed

Show real artist/song name in Now Playing #13

@adamcik
Owner

Current state is that audio now emits tags_changed(...) to AudioListeners. From there the following should happen in core:

  • We expect core to call audio.get_current_tags()` in response totags_changed``
  • Using this info core.playback.current_track should be updated.
  • When this is done a new CoreListener event for track_metadata_changed should be emited.

Frontends like the MPD frontend would need to be updated accordingly. An other option as mentioned earlier in this issue is a new separate current_metadata which might make more sense, but for the short term just using what we have is likely easier.

@AlexandrePTJ hope this is enough to point you in the right direction :-)

@AlexandrePTJ
@adamcik
Owner

Oh, and do note the TODO items for the get_current_tags - could be you don't have to address those for a PR covering this, but you should keep them in mind.

https://github.com/mopidy/mopidy/blob/develop/mopidy/audio/actor.py#L780-782

@jodal jodal added this to the v0.20 - Audio cleanup 1 milestone
@adamcik adamcik was assigned by jodal
@jodal
Owner

@adamcik I guess this issue can be closed as soon as the changelog is updated with the changes introduced with PR #938.

@adamcik
Owner

Only if we have an other issue to take over the model part of this as we still don't have a clean supported way to track both the station name and current "track".

@adamcik adamcik referenced this issue from a commit in adamcik/mopidy
@adamcik adamcik core: Switch to reference based stream info.
- Adds tests for new behaviors in core.
- Adds stream name to MPD format (fixes #944)
- Adds 'stream_changed' core event (needs a new name/event)
- Adds 'get_stream_reference' (which I'm also unsure about)

The bits I'm unsure about are mostly with respect to #270, but I'm going ahead
with this commit so we can discuss the details in PR with this code as an
example.
6fcd438
@adamcik adamcik removed their assignment
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.