Re: [PATCH 0/9] Misc fixes related to rewinds

["Alexander E. Patrakov" <patrakov@gmail.com>]

14.09.2014 17:09, Alexander E. Patrakov wrote:
> 14.09.2014 16:11, Alexander E. Patrakov wrote:
>> 14.09.2014 14:53, Raymond Yau wrote:
>>>
>>>  >
>>>  > Note: this series touches pcm_dmix.c, but does not make it
>>> rewindable. In
>>>  > other words, a variant of the test in [2] now produces a tone
>>> instead of
>>>  > failing due to snd_pcm_rewind() returning 0. But it should ideally
>>> produce
>>>  > silence. Obviously, there is some bug left that I have not pinpointed
>>> yet.
>>>  >
>>>
>>> It is better to let dmix not support rewind and force your rewind
>>> program to fail
>>>
>>> You rewind program should not affect the other application playing
>>> through dmix
>>
>> Sorry, you are not an author of a single line in dmix (although you do
>> understand how it works), so I cannot just blindly follow this request.
>> OTOH, I was about to ask the same "should it work" question.
>
> Well, after looking a bit more at this, I think that I know what's wrong
> with dmix. I can probably fix it, but I have tasks with higher priority
> right now.
>
> The real problem is that it tries to do some remixing work in its
> .rewind callback. It shouldn't do this - in fact, the whole callback
> should just move the application pointer. Look - on a hardware device,
> if you disable xrun detection, write something, rewind it, and then let
> it underrun, it will still be played in full. On dmix, it is actively
> attempted to be unmixed.
>
> The real rewind-support work should be done in the mmap_commit callback.
> It should:
>
> 0. Keep a shadow copy of the mmap areas, so that it can look at old
> samples.
>
> 1. Zero out the part of the sum buffer crossed by the hardware pointer
> since the last call (possibly in another client). Zero out the
> corresponding part of the mmap areas (both real and shadow). This should
> probably be done in snd_pcm_dmix_sync_ptr(), but I am not sure. There is
> already a hack based on ARCH_CMPXCHG that purports to do this in
> mix_areas_16().
>
> 2. Unmix the old contents of the mmap areas in mmap_commit callback
> (instead of the rewind callback). Most of the time, it will unmix the
> sequence of zeros.
>
> 3. Mix the new contents of mmap areas.
>
> 4. Update the shadow copy.

Well, scratch that. It obviously does not apply to dshare, where the 
problem also exists, although no real work is done in the rewind 
callback (correctly), and I still don't know why.

Sorry for the spam.

-- 
Alexander E. Patrakov