вс, 5 мая 2019 г. в 22:58, Georg Chini <georg@xxxxxxxx>: > > On 05.05.19 18:41, Alexander E. Patrakov wrote: > > вс, 5 мая 2019 г. в 01:41, Georg Chini <georg@xxxxxxxx>: > >> On 04.05.19 20:54, Alexander E. Patrakov wrote: > >>> сб, 4 мая 2019 г. в 20:25, Georg Chini <georg@xxxxxxxx>: > >>>> On 04.05.19 16:42, Alexander E. Patrakov wrote: > >>>>> сб, 4 мая 2019 г. в 16:17, Georg Chini <georg@xxxxxxxx>: > >>>>>> 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. Then OK on the "significant" difference, and no review on anything else. -- Alexander E. Patrakov _______________________________________________ pulseaudio-discuss mailing list pulseaudio-discuss@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
