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

[Georg Chini]

On 05.05.19 18:41, Alexander E. Patrakov wrote:
> вс, 5 мая 2019 г. в 01:41, Georg Chini <georg at chini.tk>:
>> On 04.05.19 20:54, Alexander E. Patrakov wrote:
>>> сб, 4 мая 2019 г. в 20:25, Georg Chini <georg at chini.tk>:
>>>> On 04.05.19 16:42, Alexander E. Patrakov wrote:
>>>>> сб, 4 мая 2019 г. в 16:17, Georg Chini <georg at chini.tk>:
>>>>>> Here is the new version of the header file, based on your feedback.
>>>>>> The main changes are:
>>>>>>
>>>>>> - The create_filter() function now receives the channel maps for input
>>>>>> and output.
>>>>>> - The create_filter() function receives a kill_filter() function and a
>>>>>> module pointer
>>>>>>        which makes it possible for the filter to initiate unloading of the
>>>>>> module if it
>>>>>>        detects that it is no longer applicable.
>>>>>> - An output_changed() function was added which communicates current sink
>>>>>>       and port name to the filter, so that it can detect if the output has
>>>>>> changed.
>>>>>>
>>>>>> Also I did a bit of cleanup and added a few more comments. Hope it looks
>>>>>> better now.
>>>>> It definitely looks better.
>>>>>
>>>>> I am still confused about disable_rewind and max_latency. Let's
>>>>> suppose that someone wants to implement a rewindable filter. In this
>>>>> case, they need to keep history, because PulseAudio can ask the filter
>>>>> to rewind some samples. And, as it is not allowed to say "no", they
>>>>> must keep enough history to satisfy any possible rewind request. But
>>>>> some upper bound must exist. Do I understand correctly that
>>>>> max_latency serves as such upper bound?
>>>>>
>>>>> Regarding the non-rewindable filters, we do need to limit the latency,
>>>>> but I believe it is wrong for each individual filter to specify its
>>>>> own value for such limit. It should be a global policy (the same value
>>>>> for all non-rewindable sinks), and I don't see any reason for the
>>>>> filter to be able to influence it.
>>>>>
>>>>> Therefore, I believe these two fields can be replaced by one,
>>>>> max_rewind, which is the size of history, in samples, that the filter
>>>>> is willing to keep. Zero means a non-rewindable filter.
>>>>>
>>>> That sounds like a good suggestion. I would however think
>>>> that it is better if 0 means that the filter will rewind as far as
>>>> PA wants it to. There may be filters that are stateless (like the
>>>> trivial amplifier example). We could use -1 to disable rewinding.
>>> OK.
>>>
>>>> That would also mean to limit the latency to whatever the filter
>>>> can rewind, correct? I would use the maximum of max_rewind
>>>> and the default latency for non-rewindable filters as the
>>>> max_latency value then, because I don't think it makes sense
>>>> to set the maximum latency even smaller than for non-rewindable
>>>> filters.
>>> Makes sense.
>>>
>>>> What do you think is reasonable for non-rewindable filters?
>>>> 50 ms?
>>> There were different opinions on that matter. 50 ms is indeed in a
>>> range that I would agree to.
>>>
>> Just finished the next version. Does this look OK to you now?
> I think that the only significant difference is the addition of error
> codes. I cannot comment on them with any authority, but the majority
> do not seem applicable to filters. Probably we need some mechanism
> that allows arbitrary error strings to be logged?
>
The significant difference was the removal of max_latency and
the change of bool disable_rewind to int max_rewind. I need the
error codes for handling message replies, that's why I added
them. We should not need any other error reporting I think.