вт, 30 апр. 2019 г. в 16:31, Georg Chini <georg@xxxxxxxx>: > uint64_t min_latency; /* Minimum latency > allowed for the sink, 0 if unused */ > uint64_t max_latency; /* Maximum latency > allowed for the sink, 0 if unused */ Why would these fields be needed? > bool disable_rewind; /* True when rewinding is > disabled */ I'd say get rid of it either. It's too hard to implement rewinds correctly. Even in the simple cross-over filter that PulseAudio uses for LFE remixing, it took four tries to reach at something that doesn't produce clicks when the user e.g. changes the volume or otherwise triggers a rewind. Also, let's recollect the original purpose of the rewinds. They are, in the end, a way to save power. That is, to use a very long interval between wakeups if something non-interactive (like a music player) is doing its job, without the side effect of slow reaction to events like "new sink input wants to play something urgent". Please see my Linux Audio Conference presentation and video for more details: Paper: http://lac.linuxaudio.org/2015/papers/10.pdf Slides: http://lac.linuxaudio.org/2015/download/rewind-slides.pdf Video: http://lac.linuxaudio.org/2015/video.php?id=8 The main consideration here is that almost all filters are CPU-hungry enough to mask (i.e. make irrelevant) the savings obtained by dynamic latency and rewinds. So why give the filter implementer even an option to shoot themselves in the foot? > > /* Callback to reset the filter after rewind. May be NULL */ > void (*rewind_filter)(void *filter_handle, size_t amount); Don't say "reset" here. Reset is not a proper way to handle a rewind, users do notice the resulting clicks, see https://bugs.freedesktop.org/show_bug.cgi?id=50113 (note: this is NOT reported by me, there are real users who hear and care about this imperfection!) Please make it absolutely clear that the filter state must not be reset, but seamlessly restored to the specified point in the past (which is the filter's responsibility to keep). If one can't do that, they shouldn't say that they support rewinds. And yes, I understand that we have no filters (other than the trivial one) which handle this correctly. That's actually one of the reasons why I am so opposed to supporting rewindable filters. One more comment - the "void" return type means that the filter must be able to rewind itself no matter what, i.e. that failure is not possible and not allowed. Assuming that my hint to stop pretending to support rewinds gets ignored, I'd say that's a good thing. > > /* Callback to process a chunk of data by the filter. May be NULL */ > void (*process_chunk)(float *src, float *dst, unsigned count, void > *filter_handle); > > /* Callback to retrieve additional latency caused by the filter. > May be NULL */ > uint64_t (*get_extra_latency)(void *filter_handle); > > /* Initializes a new filter instance and returns a handle to it. */ > void *(*init_filter)(unsigned input_channels, unsigned > output_channels, unsigned sample_rate); > > /* Deletes filter instance. */ > void (*exit_filter)(void *filter_handle); > > /* If set, this function is called in thread context when an update > of the > * filter parameters is requested, may be NULL. The function must > replace > * the currently used parameter structure by the new structure and > return > * a pointer to the old structure so that it can be freed in the > main thread > * using parameter_free(). If the old structure can be re-used, the > function > * may return NULL. */ > void *(*update_filter_parameters)(void *parameters, void > *filter_handle); I don't think this is implementable. How are the parameters supposed to be initially allocated? -- Alexander E. Patrakov _______________________________________________ pulseaudio-discuss mailing list pulseaudio-discuss@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
