[pulseaudio-discuss] new module module-plugin-sink

[Georg Chini]

On 03.05.19 06:56, Alexander E. Patrakov wrote:
> пт, 3 мая 2019 г. в 09:31, Georg Chini <georg at chini.tk>:
>> On 03.05.19 02:22, Alexander E. Patrakov wrote:
>>> чт, 2 мая 2019 г. в 23:45, Georg Chini <georg at chini.tk>:
>>>> Hi,
>>>>
>>>> I created a new module-plugin-sink and a small amplifier demo plugin
>>>> based on the attached header file. I did not (yet) drop max_latency,
>>>> disable_rewind and rewind_filter() because I am still waiting for more
>>>> feedback on the specification. I made it however more clear (using
>>>> Alexander's wording) that this should only be used if "real" rewinding
>>>> is possible.
>>> Thanks for that.
>>>
>>> However, the interface still thinks in terms of "number of channels",
>>> which is, in the general case, wrong. E.g., a "proper"
>>> module-virtual-surround-sink (not what we currently have) would sound
>>> differently if given the same samples in "5.1" and "5.1 (Side)"
>>> channel layouts.
>> I do not understand what you mean. How would you do it without
>> number of channels? You have to know how many input and
>> output channels are there, otherwise you can't process audio.
> I am saying that, if we are going to support filters with non-trivial
> interaction between channels, we would need to have a pa_channel_map
> somewhere. In your header, this structure is not mentioned at all.
>
> "5.1" and "5.1 (Side)" both have 6 channels. The hypothetical proper
> virtual surround plugin would need to know if its input is "5.1" or
> "5.1 (Side)", but, based on your interface, it can't.

I still don't see your point. The channel layout of input/output
and the playback device is known to module-plugin-sink, so if
a filter needs it, it can be passed as parameter. No need to
have it in the interface.

>
>>> I wouldn't mind the number of channels, if we limit the interface to
>>> "true" filters that have the same number of input and output channels
>>> and don't care about the channel map. I.e., explicitly declare the
>>> virtual surround sink as out-of-scope. But then, what useful things
>>> are left in scope except the equalizer and maybe a dynamic range
>>> compressor?
>> Why would there be any reason to limit it to filters with
>> equal number of channels?
> Because that follows from the premise that the filter does not mix
> channels. And without channel layout information (i.e. given only the
> total number of inputs, but not even their order), it is impossible to
> mix channels meaningfully.
See above.
>
>>> Also, please make sure that, in the callbacks, the filter handle
>>> always comes as the first parameter.
>> No problem, but usually in pulseaudio the userdata pointer
>> comes last in callbacks.
> Ok, ok, the real idea was "be consistent" :)

Well, things are still at a very early stage and surely I will do
some cleanup once the general approach is accepted.

>
>>> Regarding the virtual surround plugin, I actually have one more
>>> business argument why, if implemented properly (and not how it is done
>>> currently) it should be out of scope. The reason is that this plugin
>>> with publicly available HRIRs only applies to headphones. From a
>>> usability perspective, it should be switched off (or to a different
>>> filter, specifically crafted for each set of speakers and the
>>> listening room) when the audio is playing to something else than
>>> headphones. The proposed interface does not have (and likely should
>>> not have, because we want to keep it simple and stable) any place to
>>> express such policy.
>> Why would that be a problem? You can have a boolean bypass
>> parameter that switches off the filter when needed. Or even a
>> choice that switches between different HRIR files.
> Who would switch it (the virtual surround effect, I am not talking
> about other effects) off? The idea is that it is not the user (because
> with this particular effect it should really be automatic), and is not
> the filter itself (because it cannot know, based on your interface,
> that it is playing to speakers). So it has to be some third entity,
> with enough knowledge both about the filter and the PulseAudio notion
> of ports (to distinguish between speakers, headphones, and something
> unknown). And, given that entity, it does not really make sense to
> abstract the virtual surround implementation behind some common plugin
> interface, it has already leaked.
>
> As an example of prior art, please also consider the LFE channel
> remixer - it is not implemented as a virtual sink precisely for this
> reason.
As said above, there are parameters that can be passed to
the filter if needed and module-plugin-sink could propagate
changes in the playback device to the filter that way.
We could reserve special parameter names for this information,
because these controls would not be exposed to the user.

If you think channel layout and playback device should be
in the interface, I can integrate it, but using parameters looks
easier to me because this way we need no additional notification
functions.