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

use dns-sd instead of mdns for discovery #13

Closed
wants to merge 1 commit into from

Conversation

awiouy
Copy link
Collaborator

@awiouy awiouy commented Jan 28, 2018

Congratulations for this repository and for the improvements it brings to Librespot.

I maintain the Librespot addon for LibreELEC. Librespot requires some modifications to adapt to LibreELEC. At the moment I use patches to modify Librespot, but patches are tedious to maintain. I would therefore be happy if these patches could be merged upstream. My pacthes are however too brutal to be merged as such. But my control of Rust is not good enough to propose something else.

I therefore need your help to adapt my patches.
Thank you in advance for your help.

This is the first of my three patches: use dns-sd instead of mdns.

@ComlOnline
Copy link
Contributor

Hey @awiouy,
I'll have a look and see what I can do, My biggest question is why do you require dns-sd over mDNS?

Cheers

@awiouy
Copy link
Collaborator Author

awiouy commented Jan 28, 2018

Because disallow-other-stacks=yes is set in avahi-deamon.conf of LibreELEC and cannot be modified, so using dns-sd is the only option to run Librespot on LibreELEC.

@sashahilton00
Copy link
Member

There has been a bit of debate over this in the past, I wonder, even though if it is a little clunky, whether we might not be better off having a build flag which allows one to opt for dns-sd instead if desired? There isn't too much code, so using a conditional block might not be a bad way to do it. Any thoughts?

@ComlOnline
Copy link
Contributor

I like the idea of a switch, here is an attempt to integrate it at compile time but with the idea to move to automated builds i prefer a --dns-sd option flag. It means that both could be implemented without a change for those who don't need it as it could potentially break some more complex network setups.

@sashahilton00
Copy link
Member

sashahilton00 commented Jan 28, 2018

That looks pretty similar to what I was thinking. I'd reduce it further though such that one would simply have a --feature "dns-sd" flag if one wanted the dns-sd code, otherwise it defaults to mdns, for simplicity sake, and so that people already using librespot don't need to think about this change. @awiouy let us know if this solution would work for you.

@awiouy
Copy link
Collaborator Author

awiouy commented Jan 29, 2018

Yes, --feature "dns-sd" would do.

@sashahilton00
Copy link
Member

I've written the feature flag code for this PR in #112, please take a look @awiouy

@ComlOnline
Copy link
Contributor

Closing this as its been superseded by the above PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants