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

(A proposal) Mute radio stream on ads #1572

Closed
wants to merge 1 commit into from
Closed

(A proposal) Mute radio stream on ads #1572

wants to merge 1 commit into from

Conversation

apiraino
Copy link

@apiraino apiraino commented Nov 10, 2018

Hello!

I've started using i3 since a couple of days and I've already fallen in love with py3status!! ❤️

So I've hacked together a patch for the mpris module adding an automated volume muter to be used in radio streaming; the volume cut-off avoids the annoying advertisements and restore the previous volume level when the ad is finished: this is all managed by reading the "song title" that is being received from DBus. Ads are recognized by a user provided "blacklist" of keywords.

As it is, this "works for me" (:copyright: 2018), but for a public use this is still embryonal (I've borrowed some code from the module volume_plugin), just to name a few things I see missing:

  • multiple audio backends (like in volume_plugin)
  • manage edge cases related to audio volume
  • perhaps a refactoring of the filtering code (maybe a separate class?)

So, if there is interest in this idea, I can keep on working to make it fit for a wider audience - else I will just use it myself.

Any suggestion and opinion is absolutely welcome!

Thanks

EDIT: just stumbled upon issue #1566 and I completely agree with that 👍

@maximbaz
Copy link
Contributor

Hey there, I'm not using mpris myself, but the idea definitely sounds fun 😄

I can imagine that making the code "production-ready" can be quite a bit of work, so if I were you I would start by explicitly pinging previous contributors to this module and asking if this is a feature they would like to be using as well — ping "authors" section of the module: @ritze, @tobes, @valdur55.

But in general, the idea is valid and it will be beneficial to at least you personally, so if you want to go proceed, in my mind — go ahead! (This is also in light of #1566 which you stumbled upon 🙂).

@apiraino
Copy link
Author

Thank you @maximbaz for the feedback. Like you mentioned, a feedback from the authors would be also great; f.e. a suggestion to develop a completely separated module (doing one thing and trying to do it well) would make perfectly sense either.

@ritze
Copy link
Contributor

ritze commented Nov 12, 2018

Hey apiraino,

I like your idea and already thought about such a tool too. I think writing a own tool independent to py3status makes here more sense. Like the usual Unix philosophy: One tool for one job.

Maybe you can refactor some code from kdeconnect-notify? This tool also reads from dbus but instead of the mpris plugin kdeconnect-notify is a standalone tool.

Another way could an own py3status plugin, where your new plugin and the mpris plugin are using the same code base?

What do you think?

@maximbaz
Copy link
Contributor

maximbaz commented Nov 12, 2018

Funny that I was discussing this PR today with someone who is using this module and they were also suggesting to implement it as a standalone tool :)

@apiraino
Copy link
Author

@maximbaz @ritze awesome that we're on the same page: I agree that a standalone tool is a better idea. Also thanks for pointing me to the KDE DBus notifier, there are some bits I can borrow to make a cleaner implementation.

Now, let's see what kind of tool this could be; I'd rather leave it as a py3status module for two reasons:

  1. keep the scope of the tool limited and well-defined by the use-case
  2. an external tool would end up as an undefined yet another thing running in background. If there was a general API to write a plugin for multiple music players, that would be a great use-case for an application-agnostic tool.

I was wondering, too, whether the code borrowed from mpris and volume_plugin (possibly others) could be a call for a refactor of some stuff straight into the core of py3status.
However, after reading the roadmap on #1526 (f.e. a DBus code refactor was suggested at some point), I think it may be a little early for these thoughts, so I'd bite the bullet, do some copy-pasting and get out the module "soon-ish".

When py3status will expose new APIs, I'll gladly grant some attention to this module again.

Deal? :-)

@maximbaz
Copy link
Contributor

maximbaz commented Dec 2, 2018

Hey sorry for a late reply.

I'd vote for a standalone tool and not for extension to py3status for the following two reasons:

  1. In your own tool you are free to use any programming languages and libraries, so you don't have to wait for py3status to expose some APIs.
  2. Contrary to all py3status modules, this tool wouldn't really need an interface, it has no output to show, it would just sit in background and do its job.

external tool would end up as an undefined yet another thing running in background. If there was a general API to write a plugin for multiple music players, that would be a great use-case for an application-agnostic tool.

Yet another thing is good, this follows Unix philosophy, many small tools with one purpose each. Also, I think DBus is already this exact "generic API" you wanted to communicate with music players, I mean if you can receive song title via DBus, what else do you need really 🙂

@apiraino
Copy link
Author

apiraino commented Dec 3, 2018

@maximbaz your observations make great sense. I'll close this PR and refactor the concept in an external tool. As you pointed out, this could be also the perfect case for a toy project on a new programming language (e.g. I'm into Rust, lately).

Thanks to everyone for your time and for sharing your ideas! 👍

@apiraino apiraino closed this Dec 3, 2018
@maximbaz
Copy link
Contributor

maximbaz commented Dec 3, 2018

Good luck! Remember to share your app once it's ready, it does seem you'll get some happy users here 😉

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