Giter Site home page Giter Site logo

Comments (25)

cathugger avatar cathugger commented on May 20, 2024 2

https://github.com/cathugger/MPD/tree/opustags it's incomplete but somehow works on vlc, I'm pretty sure I'm doing something wrong still

from mpd.

cathugger avatar cathugger commented on May 20, 2024 1

@JustArchi I'm actually quite confident with my current implementation, but poor client support is quite disappointing.
It's not going to get better unless clients are fixed.
I could rebase my stuff on top of current master (files affected didn't change much) but I wanted to hear opinion from @MaxKellermann first.

from mpd.

MaxKellermann avatar MaxKellermann commented on May 20, 2024

Unfortunately, Opus does not support Vorbis comments, therefore adding tag support requires implementing it from scratch. But what MPD falls back to is Icy tags, which it enables if your streaming client asks for it. Please confirm that MPD did not send Icy tags, even though your client requested it!

from mpd.

JustArchi avatar JustArchi commented on May 20, 2024

I have no clue how to confirm that.

Verbose MPD log:

Jul 21 00:44 : zeroconf: No global port, disabling zeroconf
Jul 21 00:44 : state_file: Loading state file /var/lib/mpd/state
Jul 21 00:44 : playlist: queue song 93:"IRON ATTACK!/2008.05.25 [MIA-007] Blade Of Ancient Temple [例大祭5]/WALTZ OF PUPPET.ogg"
Jul 21 00:44 : decoder_thread: probing plugin vorbis
Jul 21 00:44 : decoder_thread: probing plugin oggflac
Jul 21 00:44 : decoder_thread: probing plugin opus
Jul 21 00:44 : decoder: audio_format=48000:16:2, seekable=true
Jul 21 00:44 : inotify: initializing inotify
Jul 21 00:44 : output: opened plugin=shout name="Archi's Touhou Radio [HQ 128K Opus]" audio_format=48000:16:2
Jul 21 00:44 : inotify: watching music directory
Jul 21 00:44 : state_file: Saving state file /var/lib/mpd/state

But I noticed something that can be a clue why icy tags are not working - when I try to update metadata from Icecast admin panel, I get:

Message: Mountpoint will not accept URL updates

Return Code: 1

Probably Icecast for some reason doesn't have icy metadata on for opus mountpoint, and since MPD doesn't write opus metadata either, fallback doesn't work by default.

I'm wondering if we can't enable metadata write in opus ogg container until we can get proper opus metadata support. Last time I checked, VLC nicely displayed ogg metadata that were included in opus container, even though it's not the "default" way of doing that. What I mean is that VLC can handle both opus metadata, as well as ogg metadata, and since we're using ogg container for opus already, maybe we can use it as a workaround until we can implement proper support for opus metadata.

Anyway, I'll try to reach icecast folks over not accepting URL updates, as that's part of the issue I'm having. However, having proper support for opus metadata in MPD can be nice as well, so I'm leaving the issue open, unless you want to close it of course.

In any case, thanks a lot for your response and entire work on MPD! 💓.

from mpd.

JustArchi avatar JustArchi commented on May 20, 2024

If it helps, I compiled latest icecast from source (version 2.4.99.1) and I took a look into the logs.

[2017-08-28  18:43:33] DBUG connection/_handle_authentication_mount_normal Trying <mount type="normal"> specific authenticators for client 0x55ed3d97a400.
[2017-08-28  18:43:33] DBUG connection/_handle_authentication_mount_default Trying <mount type="default"> specific authenticators for client 0x55ed3d97a400.
[2017-08-28  18:43:33] DBUG auth/auth_addref ...refcount on auth_t (null) is now 3
[2017-08-28  18:43:33] DBUG auth/auth_add_client Trying to add client 0x55ed3d97a400 to auth 0x55ed3d8e4c90's (role legacy-global-source) queue.
[2017-08-28  18:43:33] DBUG auth/auth_addref ...refcount on auth_t (null) is now 4
[2017-08-28  18:43:33] INFO auth/auth_add_client adding client 0x55ed3d97a400 for authentication on 0x55ed3d8e4c90
[2017-08-28  18:43:33] DBUG auth/queue_auth_client ...refcount on auth_t (null) is now 4
[2017-08-28  18:43:33] DBUG auth/__handle_auth_client client 0x55ed3d97a400 on auth 0x55ed3d8e4c90 role legacy-global-source processed: ok
[2017-08-28  18:43:33] DBUG connection/_handle_get_request Got client 0x55ed3d97a400 with URI /admin/metadata
[2017-08-28  18:43:33] DBUG connection/_handle_get_request Client 0x55ed3d97a400 requesting admin interface.
[2017-08-28  18:43:33] DBUG admin/admin_handle_request Admin request (/admin/metadata)
[2017-08-28  18:43:33] DBUG admin/admin_handle_request Got command (metadata)
[2017-08-28  18:43:33] INFO admin/admin_handle_request Received admin command metadata on mount "/touhou.opus"
[2017-08-28  18:43:33] DBUG admin/command_metadata Got metadata update request
[2017-08-28  18:43:33] DBUG fserve/fserve_add_client Adding client 0x55ed3d97a400 to file serving engine
[2017-08-28  18:43:33] DBUG fserve/fserve_add_pending fserve handler waking up
[2017-08-28  18:43:33] DBUG auth/auth_release ...refcount on auth_t (null) is now 3
[2017-08-28  18:43:33] DBUG client/client_destroy Called to destory client 0x55ed3d97a400
[2017-08-28  18:43:33] DBUG auth/auth_release ...refcount on auth_t (null) is now 2
[2017-08-28  18:43:33] DBUG fserve/fserv_thread_function fserve handler exit

Most important part is right here:

[2017-08-28  18:43:33] DBUG admin/admin_handle_request Admin request (/admin/metadata)
[2017-08-28  18:43:33] DBUG admin/admin_handle_request Got command (metadata)
[2017-08-28  18:43:33] INFO admin/admin_handle_request Received admin command metadata on mount "/touhou.opus"
[2017-08-28  18:43:33] DBUG admin/command_metadata Got metadata update request

This means that MPD properly tries to use icy tags, however, icecast doesn't support them for Opus streams.

Message: Mountpoint will not accept URL updates

But according to this, there is some support for Opus tags inside icecast, probably from the stream itself, via OpusTags header. I understand that MPD does not transmit that header to icecast just yet, as it's not implemented and it tries to update metadata based on URL metadata instead. Perhaps it could be considered as useful enhancement in the future?

from mpd.

1985a avatar 1985a commented on May 20, 2024

Thank you for posting this. I have the same issues since a few months but my English is not very good at this time, due to my mother language is Spanish and I felt that maybe MaxKellermann wouldn't understand me.

And thanks Max for making MPD a great program to handle music.

from mpd.

cathugger avatar cathugger commented on May 20, 2024

I've been trying to modify opus encoder's code to add tags support. So far I got it working initially (it sends proper metadata when I connect via http) but it's still incorrect for changing track.
I think I didn't properly terminate old stream, mpv throws errors about currupted packets on track change and doesn't show metadata.

from mpd.

JustArchi avatar JustArchi commented on May 20, 2024

That's great to hear @cathugger! Sadly I'm not able to help since I have no clue myself how it should work, but I'm very happy to hear that somebody decided to pick it up. Please don't hesitate to send PR once you get it fully working, and all the best! 👍

from mpd.

cathugger avatar cathugger commented on May 20, 2024

@JustArchi I don't have too much clue myself tbh, I barely got it work initially but because of not knowing all details I wasn't able to make it work properly no matter how much random tweaks I threw at it.
I'd need to properly dig into it to get some clue how it's supposed to work (track switching is hinted in standard itself), but I'm not sure if I have time for that atm...

from mpd.

cathugger avatar cathugger commented on May 20, 2024

@JustArchi offtopic, but don't use "quality" parameter for opus codec, as it's ignored. use "bitrate" instead.

from mpd.

JustArchi avatar JustArchi commented on May 20, 2024

I've compiled latest MPD from source with your changes and I'm very positively shocked - it does indeed seem to work just fine with my VLC! 💓

You've done incredible, I'm keeping this build around for a while and looking for future improvements. Please send @MaxKellermann a pull request once you feel that you're satisfied with it so we can have it reviewed and merged into the upstream. I'll also be happy to test any changes you do in the meantime.

In short, huge kudos to you! 💓

from mpd.

cathugger avatar cathugger commented on May 20, 2024

after looking around for a bit, I stumbled upon https://trac.videolan.org/vlc/ticket/18401 and https://jmvalin.ca/misc_stuff/chain_works.opus which is supposed to be well-encoded but mpv throws exactly same errors as my for code..
I know that my code is currently horrible but it looks like not all players are currently supporting it either.

from mpd.

cathugger avatar cathugger commented on May 20, 2024

at this point it seems that having a flag to opt-in opus stream chaining support would be desirable

from mpd.

cathugger avatar cathugger commented on May 20, 2024

should I name option "chaining" or "opustags"? I'm currently thinking about "chaining", as it's more technical and could be applied for flac-in-ogg too (incase that would get implemented in future).

from mpd.

cathugger avatar cathugger commented on May 20, 2024

it seems to kinda work for vlc but it still complains about some mismatches, and I'm unsure about clients which doesn't support chained opus yet (like mplayer and mpv), they probably will actually play pre-skip data as silence as they don't parse headers properly and don't take pre-skip value in them into account...
we should also consider using https://github.com/xiph/libopusenc as it could do pretty much all the chaining work for us, but it's not yet in packages of most distros, so maybe do separate encoder using that?

from mpd.

cathugger avatar cathugger commented on May 20, 2024

Anyways, I'll wait for any of mpd devs to review current code before making PR.

from mpd.

cathugger avatar cathugger commented on May 20, 2024

I did some tests with various players and tracks which are meant to be played in gapless way.
Here's my results:

  • mplayer and mpv doesn't recognize headers and produce small crack sound regardless if I include pre-skip or not. maybe with pre-skip it's smaller but I'm not sure
  • vlc tries to recognize headers and successfully updates metadata, but still produces small crack regardless. not sure about pre-skip effect there either
  • firefox works flawlessly regardless if I include pre-skip or not
  • gst-play works flawlessly regardless of pre-skip
  • chromium produces noticeable gap with pre-skip, without pre-skip it produces noticeable crack. I think it produces gap in either way, just with pre-skip enabled it's bigger.

So https://wiki.xiph.org/OggOpusImplementation seems to be accurate.
What do you think about any of this, @MaxKellermann ?

from mpd.

cathugger avatar cathugger commented on May 20, 2024
  • foobar2000 plays fine with or without pre-skip, updates tags properly, I couldn't notice gaps

from mpd.

cathugger avatar cathugger commented on May 20, 2024

I changed my mind about using "chaining" to indicate feature, as support of clients will depend on particular codec.
Vorbis chaining got good support at this point, and mpd encodes ogg vorbis streams in this way already.
It seems that ogg/flac support would be different too.
Therefore, I'll change option to enable this to "opustags".

from mpd.

JustArchi avatar JustArchi commented on May 20, 2024

@MaxKellermann Could you help us with any of this? I'd love to have this sorted out, and it seems @cathugger already managed to cover majority of what has to be fixed, we're only after that stutter now, that I also reproduced quite reliably. I love how we have half-baked implementation ready, proving that what we're doing here is in fact possible, we just need somebody with enough knowledge to glue this all together and fix once for good 🙂

from mpd.

cathugger avatar cathugger commented on May 20, 2024

this ticket seems to be forgotten at this point.
I guess I'll just make PR.

from mpd.

MaxKellermann avatar MaxKellermann commented on May 20, 2024

Oh, yes, please. Always PR if you have code, even if you're not sure if it's good enough yet.

from mpd.

cathugger avatar cathugger commented on May 20, 2024

#205

from mpd.

cathugger avatar cathugger commented on May 20, 2024

@JustArchi it got merged to master

from mpd.

JustArchi avatar JustArchi commented on May 20, 2024

Since the feature eventually made it in, for which I'm very grateful, I'll close the issue for now since there is no ongoing work required for this.

Thanks!

from mpd.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.