[pulseaudio-discuss] Resampler rewinding
Hi Tanu, Here a new picture: DIAGRAM FOR HARDWARE SINK 1 BEFORE STARTING THE MOVE (OR BEFORE STARTING A REWIND) +-+- | client stream memblockq | +-+- ^ read index +--+ | data in the client resampler | +--+ +--+ | client render_memblockq | queue length | +--+ ^ read index ^ write index +---+-+---+- | client history_queue | length equal to resampler data | queue length | +---+-+---+- ^ read index ^write index + | dma buffer| + ^ hardware playback position, can't rewind beyond this point What you can see in this picture is, that the difference between the render memblockq read index and the history queue read index will always equal the amount of data in the resampler, even if that amount is varying. So rewinding the history queue by the same amount as the render memblockq plus the resampler delay ensures, that after the rewind the read index of both queues is equal. The render memblockq plus resampler data and the history queue data are completely equivalent, just in different sample specs. Read and write index of the history queue are both ahead of the render queue by the amount of data in the resampler. The equivalence ensures that an operation done on one queue can be mirrored to the other queue. Meanwhile I changed the SOXR resampler to use variable rate. This makes it behave like all the other resamplers do. Probably I could try your approach again, but I still think my solution is more elegant than trying to force the queue into the audio flow somehow. ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] Resampler rewinding
On 16.06.19 12:28, Georg Chini wrote: I sent the patch that introduces the history queue to the list. Please take a look. I think it is rather simple. Hi Tanu, I tried your proposal to synchronize read and write index of the history queue immediately after the push and then to add the render memblockq length when the history queue is rewound. It does not work for the soxr resampler, possibly because the soxr resampler has a non-constant delay. That means, that if the resampler had for example 1000 frames delay before the reset, it will already produce output data if you feed it 1000 frames after the reset. (The soxr resampler still produces some glitches after my changes anyway, probably because the state cannot be restored perfectly, but you have to listen hard to hear them.) I did not try the second option, that is synchronizing the read and write index in pa_sink_input_drop(), because that would mean that I do not only have to take the render memblockq length into account, but also the length of the history queue. Some data may have been pushed into both queues after read and write index were synchronized. So I would ask you to reconsider the picture you have in mind. In my opinion it is inappropriate anyway, because the history queue is not part of the audio flow and therefore trying to make it fit in somehow is the wrong approach. Instead it should be considered as a mirror of the render memblockq. Using it as a mirror definitely produces simpler code. ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] Resampler rewinding
I sent the patch that introduces the history queue to the list. Please take a look. I think it is rather simple. ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] Resampler rewinding
On 16.06.19 10:39, Tanu Kaskinen wrote: On Sat, 2019-06-15 at 11:31 +0200, Georg Chini wrote: On 15.06.19 10:01, Tanu Kaskinen wrote: On Tue, 2019-06-11 at 22:08 +0200, Georg Chini wrote: Hi Tanu, the first diagram should look like this: DIAGRAM FOR HARDWARE SINK 1 BEFORE STARTING THE MOVE +-+- client stream memblockq | +-+- ^ read index +++- client history_queue |queue length | +++- ^ read index ^write index +--+ | data in the client resampler | +--+ +--+ client render_memblockq | queue length | +--+ ^ read index ^ write index + | dma buffer| + ^ hardware playback position, can't rewind beyond this point This is why I do not need to take the render memblockq length into account when rewinding the history queue. (When rewinding during a volume change, the history queue is also rewound by the same amount as the render memblockq plus the data in the resampler. Then the resampler bit is fed back into the resampler.) The third diagram is correct again, before finishing the move, the read pointers are out of sync and will be re-synchronized when pa_sink_input_drop() is called the next time during FINISH_MOVE. I don't see why history_queue length should be in sync with the render_memblockq length. I find your scheme harder to understand, because the usual rule of the write position of a downstream buffer matching the read position of the next buffer upstream doesn't hold. In your scheme the read index of history_queue doesn't point to the location from which the resampler reads data. Could you change this bit in your code? I do not read from the history queue during normal operation, the input data for the resampler comes from the client. I just push the data that the client provides into the history. Ok, so the history_queue isn't part of the audio flow, it's just a separate buffer with its own special rules. To make it part of the audio flow, you could read a chunk from the client, push it to the history_queue, then immediately read the chunk back from the history_queue and push it to the resampler. Yes, I started like that but it is much simpler to keep it out of the flow. Data is pushed into the queue when it is received and dropped in pa_sink_input_drop() to match the length of the render memblockq. It is much simpler to keep the indices in sync than to have to care about the read index difference between the render memblockq and the history queue. What is there to care about? The read index difference is the render_memblockq length plus the resampler delay. Those will keep up to date by themselves, no manual intervention needed. Yes, but as said below, you have to add the render memblockq length in multiple places. This basically means that I can do the same operations on the history queue that I do on the render memblockq. The history queue is only read in two situations: 1) During a volume change. In this case I can now simply rewind the history queue by the same amount that I rewind the render memblockq plus the bit that I have to feed into the resampler. Otherwise you'd simply rewind the history queue by the new render_memblockq length (after rewinding it first) plus the resampler delay. Doesn't seem any more complicated to me. It is only a bit more complicated. But you will have to do the same when changing volume or doing anything that involves feeding data from the history queue into the resampler or the memblockq. 2) During a move. Here again it helps that the indices are in sync because I do not need to save the render memblockq length during the START_MOVE message. I can simply ignore the bit in the render queue that was not yet read because it will be taken into account automatically. So instead of having a variable "old_render_memblockq_length" you use the history_memblockq length as a substitute, which makes it
Re: [pulseaudio-discuss] Resampler rewinding
On Sat, 2019-06-15 at 11:31 +0200, Georg Chini wrote: > On 15.06.19 10:01, Tanu Kaskinen wrote: > > On Tue, 2019-06-11 at 22:08 +0200, Georg Chini wrote: > > > Hi Tanu, > > > > > > the first diagram should look like this: > > > > > > DIAGRAM FOR HARDWARE SINK 1 BEFORE STARTING THE MOVE > > > > > > +-+- > > > >client stream memblockq > > > > | > > > +-+- > > > > > > ^ read index > > > > > > +++- > > > > client history_queue |queue > > > > length| > > > +++- > > >^ read > > > index ^write index > > > > > > > > > > > > > > > +--+ > > > | data in the client > > > resampler | > > > > > > +--+ > > > > > > +--+ > > > > client render_memblockq | queue length | > > > +--+ > > > ^ read index ^ write index > > > > > > + > > > | dma buffer| > > > + > > > ^ hardware playback position, > > >can't rewind beyond this point > > > > > > This is why I do not need to take the render memblockq length into > > > account when > > > rewinding the history queue. (When rewinding during a volume change, the > > > history > > > queue is also rewound by the same amount as the render memblockq plus the > > > data in > > > the resampler. Then the resampler bit is fed back into the resampler.) > > > > > > The third diagram is correct again, before finishing the move, the read > > > pointers are > > > out of sync and will be re-synchronized when pa_sink_input_drop() is > > > called the next > > > time during FINISH_MOVE. > > I don't see why history_queue length should be in sync with the > > render_memblockq length. I find your scheme harder to understand, > > because the usual rule of the write position of a downstream buffer > > matching the read position of the next buffer upstream doesn't hold. In > > your scheme the read index of history_queue doesn't point to the > > location from which the resampler reads data. Could you change this bit > > in your code? > > > I do not read from the history queue during normal operation, the input > data for the resampler comes from the client. I just push the data that > the client provides into the history. Ok, so the history_queue isn't part of the audio flow, it's just a separate buffer with its own special rules. To make it part of the audio flow, you could read a chunk from the client, push it to the history_queue, then immediately read the chunk back from the history_queue and push it to the resampler. > It is much simpler to keep the indices in sync than to have to care about > the read index difference between the render memblockq and the history > queue. What is there to care about? The read index difference is the render_memblockq length plus the resampler delay. Those will keep up to date by themselves, no manual intervention needed. > This basically means that I can do the same operations on the history > queue that I do on the render memblockq. > The history queue is only read in two situations: > > 1) During a volume change. In this case I can now simply rewind the history > queue by the same amount that I rewind the render memblockq plus the bit > that I have to feed into the resampler. Otherwise you'd simply rewind the history queue by the new render_memblockq length (after rewinding it first) plus the resampler delay. Doesn't seem any more complicated to me. > 2) During a move. Here again it helps that the indices are in sync because I > do not need to save the render memblockq length during the START_MOVE > message. I can simply ignore the bit in the render queue that was not yet > read because it will be taken into account automatically. So instead of having a variable "old_render_memblockq_length" you use the history_memblockq length as a substitute, which makes it necessary to fiddle with the read index to keep it sync with the render_memblockq. I don't see how that promotes clarity - the variable name is much more descriptive than
Re: [pulseaudio-discuss] Resampler rewinding
On 15.06.19 10:01, Tanu Kaskinen wrote: On Tue, 2019-06-11 at 22:08 +0200, Georg Chini wrote: Hi Tanu, the first diagram should look like this: DIAGRAM FOR HARDWARE SINK 1 BEFORE STARTING THE MOVE +-+- client stream memblockq | +-+- ^ read index +++- client history_queue |queue length | +++- ^ read index ^write index +--+ | data in the client resampler | +--+ +--+ client render_memblockq | queue length | +--+ ^ read index ^ write index + | dma buffer| + ^ hardware playback position, can't rewind beyond this point This is why I do not need to take the render memblockq length into account when rewinding the history queue. (When rewinding during a volume change, the history queue is also rewound by the same amount as the render memblockq plus the data in the resampler. Then the resampler bit is fed back into the resampler.) The third diagram is correct again, before finishing the move, the read pointers are out of sync and will be re-synchronized when pa_sink_input_drop() is called the next time during FINISH_MOVE. I don't see why history_queue length should be in sync with the render_memblockq length. I find your scheme harder to understand, because the usual rule of the write position of a downstream buffer matching the read position of the next buffer upstream doesn't hold. In your scheme the read index of history_queue doesn't point to the location from which the resampler reads data. Could you change this bit in your code? I do not read from the history queue during normal operation, the input data for the resampler comes from the client. I just push the data that the client provides into the history. It is much simpler to keep the indices in sync than to have to care about the read index difference between the render memblockq and the history queue. This basically means that I can do the same operations on the history queue that I do on the render memblockq. The history queue is only read in two situations: 1) During a volume change. In this case I can now simply rewind the history queue by the same amount that I rewind the render memblockq plus the bit that I have to feed into the resampler. 2) During a move. Here again it helps that the indices are in sync because I do not need to save the render memblockq length during the START_MOVE message. I can simply ignore the bit in the render queue that was not yet read because it will be taken into account automatically. Additionally, I have to do something with the read index of the history queue anyway, because it is never read during normal operation. So if I do not drop the data somewhere, the length will keep growing. So all in all I think it makes the code much better readable and understandable if the history queue is kept in sync with the render queue. It is a history queue and as such should reflect what happens to the render queue. I can still change this if you want, but for me it does not make sense to complicate the code unnecessary. ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] Resampler rewinding
On Tue, 2019-06-11 at 22:08 +0200, Georg Chini wrote: > Hi Tanu, > > the first diagram should look like this: > > DIAGRAM FOR HARDWARE SINK 1 BEFORE STARTING THE MOVE > > +-+- > > client stream memblockq > > | > +-+- > > ^ read index > > +++- > > client history_queue |queue > > length| > +++- > ^ read index > ^write index > > > > > +--+ > | data in the client > resampler | > > +--+ > > +--+ > > client render_memblockq | queue length | > +--+ > ^ read index ^ write index > > + > | dma buffer| > + > ^ hardware playback position, > can't rewind beyond this point > > This is why I do not need to take the render memblockq length into account > when > rewinding the history queue. (When rewinding during a volume change, the > history > queue is also rewound by the same amount as the render memblockq plus the > data in > the resampler. Then the resampler bit is fed back into the resampler.) > > The third diagram is correct again, before finishing the move, the read > pointers are > out of sync and will be re-synchronized when pa_sink_input_drop() is called > the next > time during FINISH_MOVE. I don't see why history_queue length should be in sync with the render_memblockq length. I find your scheme harder to understand, because the usual rule of the write position of a downstream buffer matching the read position of the next buffer upstream doesn't hold. In your scheme the read index of history_queue doesn't point to the location from which the resampler reads data. Could you change this bit in your code? -- Tanu https://www.patreon.com/tanuk https://liberapay.com/tanuk ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
[pulseaudio-discuss] Resampler rewinding
Hi Tanu, the first diagram should look like this: DIAGRAM FOR HARDWARE SINK 1 BEFORE STARTING THE MOVE +-+- | client stream memblockq | +-+- ^ read index +++- | client history_queue |queue length | +++- ^ read index ^write index +--+ | data in the client resampler | +--+ +--+ | client render_memblockq | queue length | +--+ ^ read index ^ write index + | dma buffer| + ^ hardware playback position, can't rewind beyond this point This is why I do not need to take the render memblockq length into account when rewinding the history queue. (When rewinding during a volume change, the history queue is also rewound by the same amount as the render memblockq plus the data in the resampler. Then the resampler bit is fed back into the resampler.) The third diagram is correct again, before finishing the move, the read pointers are out of sync and will be re-synchronized when pa_sink_input_drop() is called the next time during FINISH_MOVE. ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
