D29312: Change microphoneindicator for reporting audio monitors

[Giusy Margarita]

kurmikon added a comment.


  Pulseaudio is an abstraction layer over alsa.
  
  You, as a developer, can record sources in many ways, but since it's an 
abstraction layer, you can't be really sure whether the source is a microphone 
or not. There's a discussion on github where a user was asking on differences 
between speakers attached to the system and network output: PulseEffects funder 
said there's no difference, since Pulseaudio exposes them the same way.
  
  Maybe there should be something to discover it in some complicated ways, but 
that's also too difficult to implement in a simple desktop environment applet. 
No offense, but who designed this indicator had in mind one only use case. 
Unfortunately there are also other cases and Plasma gives a wrong information 
in those events.
  
  Besides, it's also not coherent. I have two laptops. On one I don't have the 
microphone and no message is shown.
  On the other the microphone is integrated, always connected, and the message 
is shown: "PulseEffects is using the microphone". That's not true, I'm using 
PulseEffects to apply volume normalization on output sources.
  
  What can we do? Maybe we can exclude PulseEffects with a string comparison or 
a reg exp from the app list? Nope, because it can record from the mic in other 
use case, so that would be in conflict with the applet primary purpose.
  
  So, please, don't mock the users and let's show the truth. If you don't like 
"monitoring", we can say "recording", "using", "handling". But not microphone. 
An audio stream.

REPOSITORY
  R115 Plasma Audio Volume Applet

REVISION DETAIL
  https://phabricator.kde.org/D29312

To: kurmikon, #vdg, #plasma, drosca, broulik
Cc: mart, nicolasfella, ngraham, plasma-devel, Orage, LeGast00n, 
The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, ragreen, ZrenBot, 
himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, 
ahiemstra

D29312: Change microphoneindicator for reporting audio monitors

[Giusy Margarita]

kurmikon added a comment.


  In D29312#662773 , @mart wrote:
  
  > i think i would prefer a somewhat inaccurate message, rather than an 
obscuretecnicism which is technically accurate (
  
  
  I don't agree, but what do you suggest considering that now Plasma is showing 
a wrong message?

REPOSITORY
  R115 Plasma Audio Volume Applet

REVISION DETAIL
  https://phabricator.kde.org/D29312

To: kurmikon, #vdg, #plasma, drosca, broulik
Cc: mart, nicolasfella, ngraham, plasma-devel, Orage, LeGast00n, 
The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, ragreen, ZrenBot, 
himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, 
ahiemstra

D29312: Change microphoneindicator for reporting audio monitors

[Marco Martin]

mart added a comment.


  i think i would prefer a somewhat inaccurate message, rather than an 
obscuretecnicism which is technically accurate (

REPOSITORY
  R115 Plasma Audio Volume Applet

REVISION DETAIL
  https://phabricator.kde.org/D29312

To: kurmikon, #vdg, #plasma, drosca, broulik
Cc: mart, nicolasfella, ngraham, plasma-devel, Orage, LeGast00n, 
The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, ragreen, ZrenBot, 
himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, 
ahiemstra

D29312: Change microphoneindicator for reporting audio monitors

[Giusy Margarita]

kurmikon added a comment.


  @nicolasfella if an application is recording directly from an input device (a 
simple microphone), your hint would be the solution: we should check and 
exclude sink source monitors.
  
  This way PulseEffects would be correctly excluded. But I was thinking a 
special case which would lead to another issue. If in the future there will be 
(or maybe it already exists) an application that records from multiple input 
devices (something to mix from more than one microphone), this app should 
create a monitor sink to redirect every source, then it has to record from the 
monitor.
  
  In the case, if we exclude applications recoding from a sink monitor, we will 
exclude this app that really is using microphones, which is not the primary 
intended purpose.
  
  So, the best approach is to cover all cases and report "an app is monitoring 
something" when it is recoding from whichever source.

REPOSITORY
  R115 Plasma Audio Volume Applet

REVISION DETAIL
  https://phabricator.kde.org/D29312

To: kurmikon, #vdg, #plasma, drosca, broulik
Cc: nicolasfella, ngraham, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, 
cblack, jraleigh, zachus, fbampaloukas, ragreen, ZrenBot, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D29312: Change microphoneindicator for reporting audio monitors

[Giusy Margarita]

kurmikon added a comment.


  In D29312#660704 , @nicolasfella 
wrote:
  
  > In D29312#660702 , @kurmikon 
wrote:
  >
  > > In D29312#660670 , 
@nicolasfella wrote:
  > >
  > > > > but due to a lack in qt libraries
  > > >
  > > > Can you elaborate on this? What is Qt lacking?
  > >
  > >
  > > I'm not an expert, so I don't really know. Reading the bug report, 
there's no way to discern input devices from monitor sinks. So if you want to 
report applications that are using a microphone, you end up showing 
applications like PuseEffects that create a monitor sink. Those applications 
can use a microphone but in most cases don't, because PulseEffects is mostly 
used to apply effects to output streams (but need to record those streams 
effectively).
  >
  >
  > Qt is not really involved in this. What matters is what libpulse offers, 
and it seems to me that we can check whether a sink is a monitor 
(https://freedesktop.org/software/pulseaudio/doxygen/structpa__source__info.html#a5e304b796ce71c7fa54e5a88f630).
 One would need to add a method isMonitor to Sink that reads this information 
and then we can not show the indicator when it's a monitor
  
  
  I'm afraid that application that really use a microphone, create a monitor 
sink.

REPOSITORY
  R115 Plasma Audio Volume Applet

REVISION DETAIL
  https://phabricator.kde.org/D29312

To: kurmikon, #vdg, #plasma, drosca, broulik
Cc: nicolasfella, ngraham, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, 
cblack, jraleigh, zachus, fbampaloukas, ragreen, ZrenBot, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D29312: Change microphoneindicator for reporting audio monitors

[Nicolas Fella]

nicolasfella added a comment.


  In D29312#660702 , @kurmikon wrote:
  
  > In D29312#660670 , @nicolasfella 
wrote:
  >
  > > > but due to a lack in qt libraries
  > >
  > > Can you elaborate on this? What is Qt lacking?
  >
  >
  > I'm not an expert, so I don't really know. Reading the bug report, there's 
no way to discern input devices from monitor sinks. So if you want to report 
applications that are using a microphone, you end up showing applications like 
PuseEffects that create a monitor sink. Those applications can use a microphone 
but in most cases don't, because PulseEffects is mostly used to apply effects 
to output streams (but need to record those streams effectively).
  
  
  Qt is not really involved in this. What matters is what libpulse offers, and 
it seems to me that we can check whether a sink is a monitor 
(https://freedesktop.org/software/pulseaudio/doxygen/structpa__source__info.html#a5e304b796ce71c7fa54e5a88f630).
 One would need to add a method isMonitor to Sink that reads this information 
and then we can not show the indicator when it's a monitor

REPOSITORY
  R115 Plasma Audio Volume Applet

REVISION DETAIL
  https://phabricator.kde.org/D29312

To: kurmikon, #vdg, #plasma, drosca, broulik
Cc: nicolasfella, ngraham, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, 
cblack, jraleigh, zachus, fbampaloukas, ragreen, ZrenBot, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D29312: Change microphoneindicator for reporting audio monitors

[Giusy Margarita]

kurmikon added a comment.


  In D29312#660670 , @nicolasfella 
wrote:
  
  > > but due to a lack in qt libraries
  >
  > Can you elaborate on this? What is Qt lacking?
  
  
  I'm not an expert, so I don't really know. Reading the bug report, there's no 
way to discern input devices from monitor sinks. So if you want to report 
applications that are using a microphone, you end up showing applications like 
PuseEffects that create a monitor sink. Those application can use a microphone 
but in most cases don't, because PulseEffects is mostly used to apply effects 
to output streams (but need to record those streams effectively).

INLINE COMMENTS

> ngraham wrote in microphoneindicator.cpp:264
> I don't think this needs to be removed. If an app is "monitoring audio" but 
> there 's no audio recording device, that it hardly matters, right?

First of all, I'm not a c++ developer, I just changed strings, I don't even 
know what m_sourceModel is. I assume it's a structure listing input sinks, so 
if you don't have microphones connected, you won't see the indicator, right?

If I assumed right, this is useless I think, because when you connect a 
microphone, the indicator will show PulseEffects even if nothing is recording 
from the mic. Plus, some systems have microphone integrated, always connected, 
so it's useless in this case.

The point is: you want to show applications using microphones? So, show only 
them, and not monitor sinks.

Since it seems that can't be achieved, show everything is monitoring an audio 
stream,  including applications using microphone and monitor sinks. So, if 
there's no audio recording device, show monitor sinks (if they are present).

REPOSITORY
  R115 Plasma Audio Volume Applet

REVISION DETAIL
  https://phabricator.kde.org/D29312

To: kurmikon, #vdg, #plasma, drosca, broulik
Cc: nicolasfella, ngraham, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, 
cblack, jraleigh, zachus, fbampaloukas, ragreen, ZrenBot, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D29312: Change microphoneindicator for reporting audio monitors

[Nicolas Fella]

nicolasfella added a comment.


  > but due to a lack in qt libraries
  
  Can you elaborate on this? What is Qt lacking?

REPOSITORY
  R115 Plasma Audio Volume Applet

REVISION DETAIL
  https://phabricator.kde.org/D29312

To: kurmikon, #vdg, #plasma, drosca, broulik
Cc: nicolasfella, ngraham, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, 
cblack, jraleigh, zachus, fbampaloukas, ragreen, ZrenBot, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D29312: Change microphoneindicator for reporting audio monitors

[Nathaniel Graham]

ngraham added inline comments.

INLINE COMMENTS

> microphoneindicator.cpp:264
>  {
> -// If there are no microphones present, there's nothing to record
> -if (m_sourceModel->rowCount() == 0) {

I don't think this needs to be removed. If an app is "monitoring audio" but 
there 's no audio recording device, that it hardly matters, right?

REPOSITORY
  R115 Plasma Audio Volume Applet

REVISION DETAIL
  https://phabricator.kde.org/D29312

To: kurmikon, #vdg, #plasma, drosca, broulik
Cc: ngraham, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, 
jraleigh, zachus, fbampaloukas, ragreen, ZrenBot, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D29312: Change microphoneindicator for reporting audio monitors

[Nathaniel Graham]

ngraham added reviewers: VDG, Plasma, drosca, broulik.
ngraham added a comment.


  This is more accurate given the technical constraints, yeah.

REPOSITORY
  R115 Plasma Audio Volume Applet

REVISION DETAIL
  https://phabricator.kde.org/D29312

To: kurmikon, #vdg, #plasma, drosca, broulik
Cc: ngraham, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, 
jraleigh, zachus, fbampaloukas, ragreen, ZrenBot, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D29312: Change microphoneindicator for reporting audio monitors

[Giusy Margarita]

kurmikon created this revision.
Herald added a project: Plasma.
Herald added a subscriber: plasma-devel.
kurmikon requested review of this revision.

REVISION SUMMARY
  Microphone indicator was introduced to report applications using a microphone.
  
  That wasn't a bad idea, but due to a lack in qt libraries, we can't discern 
microphone input devices from pulseaudio monitor sinks. The latter are used by 
applications such as PulseEffects. They record the stream from a source (i.e. 
the audio from Firefox playing a YouTube video, in order to add some effects). 
Since they record something, they act like a microphone, even if they **are 
not** and **do not** use a microphone.
  
  I filed this bug report  months 
ago, but nothing was resolved after months.
  
  An initial workaround was this revision , 
that simply did nothing since PulseEffects keeps showing and the tooltip 
reports it's using a microphone when that's not true.
  
  So a solution to this issue is renaming the tooltip reporting **Monitor 
audio**, informing the user that //something is monitoring an audio stream//. 
That includes both cases, applications that are really using a microphone and 
applications like PulseEffects that are using an audio stream to record it and 
redirect the stream inside its input pipeline to apply some changes.
  
  If you will accept the revision, I will also help to translate it in my 
language.

REPOSITORY
  R115 Plasma Audio Volume Applet

REVISION DETAIL
  https://phabricator.kde.org/D29312

AFFECTED FILES
  src/qml/microphoneindicator.cpp

To: kurmikon
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, 
fbampaloukas, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart