[pulseaudio-discuss] Resampler rewinding

[Georg Chini]

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.