On Wednesday, March 27, 2013 10:35:18 AM you wrote: > On Tue, 2013-03-26 at 19:34 -0700, Justin Chudgar wrote: > > https://github.com/justinzane/pulseaudio/blob/master/src/modules/module-lf > > e-lp.c > > ... > > My understanding is that since I've got a causal filter, I need to do one > > of the two things when a rewind request appears: > > 1 - buffer the biquad samples so that I can restart filtering the rewound > > input memblock_q immediately > > - or - > > 2 - buffer the input samples for at least sample_rate/20 and when a rewind > > come in, set the biquad samples to 0.0 and filter the buffered input. > > I think option 1 is better. Rewinding means that you jump back in time. > Let T be the point in time to which you jump. Option 1 ensures that you > restore exactly the same state that you had when you reached T the first > time. > > I have no idea how close the resulting state would be to the optimal > with this limited input history approach, maybe it would be virtually > perfect. But to avoid this uncertainty, I prefer option 1. According to my quick calculations, 176 samples history is sufficent for 48dB difference, 414 for 90dB difference. > > > Also, is it possible? likely? for a rewind to occur during the processing > > of a sink_input_pop_cb? Since this is where the filtering occurs, how > > does one manage locking of the shared userdata? > > sink_input_pop_cb() and sink_input_process_rewind_cb() are both called > from the "IO thread" of the master sink, so they can't be called > concurrently. There's no need for locking. Excellent! > > > The more focused on my little bit of code, the better, and thanks again. > > In sink_input_pop_cb(), I think you should push to the biquad history > buffer the amount that you return to the caller in the "chunk" function > argument, that is, the amount that you processed. If the history buffer, > as a result, contains more data than max_rewind, you should drop the > excessive history data. My initial implementation pushed into the history buffer after each calculation -- which occurs in the sink_input_pop_cb processing loop. > > sink_input_process_rewind_cb() is a bit more tricky, I have some trouble > wrapping my head around what is happening and what should be done... > nbytes is the amount of our old output that has been discarded and which > we should therefore regenerate. It sounds logical that this would be the > amount that we should jump back in the biquad sample history. But the > comment about resetting the filter is in a branch that is only executed > when we seek in u->memblockq, which would suggest that we should care > about the amount variable instead of (or in addition to) the nbytes > variable. What does nbytes refer to? Do I divide like: rewind_frames = nbytes / (sizeof(u->sample_spec.format) * u- >sample_spec.channels); <me>grumble, doc comments</me> > > If u->memblockq contains data, it is unprocessed input, so I don't think > we should care about the amount of seeking we do in that buffer. > Unprocessed input hasn't yet had effect on our filter state. It seems to > me that the right thing to do is to care only about nbytes and jump back > in the biquad history buffer by that amount (and restore the filter > state accordingly). I think I've understood, with two and a half caveats. 1 - When does u->sink->thread_info.max_rewind get populated with a useful value? pa__init? Should I just sprinkle fprintf's around to see? 1.5 - Is the proper way to handle changes in max_rewind size just to realloc the history buffer and adjust the index? 2 - max_rewind and max_request are sizes by 'size_t nbytes'. How does that relate to the number of frames? In other words, should I store (num_frames = nbytes / (sizeof(sample_spec.format) * sample_spec.channels)? PS: I spent a few hours just writing doc comments for all the functions. That really helped me get a clearer picture of what is going on. > -- > Tanu
