Add pop sound on volume change with libcanberra

[dooblem]

Hi fernando,

I just implemented a new feature in pa-applet : plays pop sounds when changing volume with keypresses (ubuntu like).
pa-applet is now included by default in Manjaro (Archlinux based) distribution.

What do you think ?

Thanks in advance,
Marc

[fernandotcl]

Thanks, that's a very interesting idea. At the moment, however, I don't have time to integrate your patch. I'll leave this ticket open, and hopefully come back to it later.

[dooblem]

Hi Fernando,

That's one click in github ;)

But I understand you want to test it and review it. I'm awaiting your
feedback.

Thanks in advance,
Marc

Thanks, that's a very interesting idea. At the moment, however, I
don't have time to integrate your patch. I'll leave this ticket open,
and hopefully come back to it later.

—
Reply to this email directly or view it on GitHub
#8 (comment).

[dooblem]

for info, Manjaro folks included my patch in the stable repos.

[dooblem]

Another commit with a bugfix.

When in dual screen with the following config:

Having tray icon at the top of the left screen results to the following volume scale position:

In the screenshot you see that the volume scale is placed above the tray icon. On the physical screens, the volume scale is hidden and unusable.

My patch fixes that : it checks if the volume scale can be displayed on a monitor, otherwise the volume scale position is inverted (below tray icon).

Note: this will happen also when having the secondary screen above the primary screen (with tray icon on top of primary screen).

[fernandotcl]

Thanks for the contribution. About this latest contribution, some notes:

1. Next time you contribute to a project using GitHub, please have one branch per change. As you can see, everything is grouped in the same issue here, and this is a mess.
2. You're not following the very basic style of the rest of the source (8-stop tabs?).
3. Why check if Y > monitor_rect? If that's the case, flipping the scale orientation won't help at all.

I pushed an amended fix as 33b413b. If you agree with 3, I'll merge it. Let me know.

[dooblem]

Thanks for the tips. I'll keep that in mind.
You're right, the Y > monitor_rect test seems useless.
I tested it like that. Works like a charm. I agree.

Let me know if you want me to make another branch for the pop sound patch.

Le 05/03/2014 22:16, Fernando Lemos a écrit :

Thanks for the contribution. About this latest contribution, some notes:

1. Next time you contribute to a project using GitHub, please have
   one branch per change. As you can see, everything is grouped in
   the same issue here, and this is a mess.
2. You're not following the very basic style of the rest of the
   source (8-stop tabs?).
3. Why check if Y > monitor_rect? If that's the case, flipping the
   scale orientation won't help at all.

I pushed an amended fix as 33b413b
33b413b.
If you agree with 3, I'll merge it. Let me know.

—
Reply to this email directly or view it on GitHub
#8 (comment).

[fernandotcl]

Merged, thanks.

I'll keep this issue open until I find time to look into the libcanberra integration patch. If you think you can improve the libcanberra patch (e.g. by reviewing the style), I would recommend resetting your master to match my master first, then branching off and making a new commit that references this issue in the commit message.

Also, could you please make the dependence on libcanberra optional? Thanks.

[dooblem]

Hi Fernando,
How would you do to make the dependency optional? by using dynamic
library loading with dlopen()?

Le 06/03/2014 21:23, Fernando Lemos a écrit :

Merged, thanks.

I'll keep this issue open until I find time to look into the
libcanberra integration patch. If you think you can improve the
libcanberra patch (e.g. by reviewing the style), I would recommend
resetting your master to match my master first, then branching off and
making a new commit that references this issue in the commit message.

Also, could you please make the dependence on libcanberra optional?
Thanks.

—
Reply to this email directly or view it on GitHub
#8 (comment).

[fernandotcl]

I mean making it optional to build with libcanberra. If libcanberra is not installed, pa-applet should build just fine automatically (except without libcanberra support, of course).

[dooblem]

ok you mean at compile time.
I spent huge time this morning trying to make it work. By default ./configure detects if canberra is installed or not, but you can still use --without-canberra or --with-canberra to force it.

I've put my little work on a new branch https://github.com/dooblem/pa-applet/tree/canberra-option but I'm not proud of it and it's still incomplete.

I'm really bad in autotools. And I'm missing some Makefile code to avoid building with libcanberra in case it's not here.

Any help appreciated.

Sidenote: I rebased this branch with your master, and I fixed the 2 tabs.