[pulseaudio-discuss] [patch] avoid fake rewind when setting cork state

[Colin Guthrie]

'Twas brillig, and Wang Xingchao at 13/08/11 02:38 did gyre and gimble:
> Hi Col,
> 
> 2011/8/13 Colin Guthrie <gmane at colin.guthr.ie>
>>
>> 'Twas brillig, and Wang Xingchao at 12/08/11 07:37 did gyre and gimble:
>>> Hi Col,
>>>
>>> Pls find attached updated patch, it works fine on my Meego platform.
>>> Feel free to let me know if there's any risk.
>>
>> This just doesn't feel right.
>>
>> Also, it seems that the current state must already be
>> PA_SINK_INPUT_RUNNING due to the line:
>>
>>    corking = state == PA_SINK_INPUT_CORKED && i->thread_info.state ==
>> PA_SINK_INPUT_RUNNING;
>>
>> (assuming that the thread_info.state is not updated in the
>>> state_changed() callbacks)
>>
>> I've checked already when trying to debug this (by putting in an assert
>> before calling rewind and it was already in the running state, so I
>> don't think this patch will actually help things (tho' I have not tested
>> it personally).
>>
> 
> state had been changed to "CORKED" by :
>  i->thread_info.state = state;


Yeah, but meant after your last patch... but, indeed, the underlying
problem was that the thread_info.state was not updated when neither
corking nor uncorking was true.

> So, need reset it to "RUNNING" before calling pa_sink_input_request_rewind(),
> otherwise it does not really rewind, that's the bug.
> 
> Though it's a bit ugly of the change. What about below change?
> 
> if(!corking)
>     i->thread_info.state = state;
> 
>  if (corking) {
> 
> @@ -1764,9 +1763,11 @@ void
> pa_sink_input_set_state_within_thread(pa_sink_input *i,
> pa_sink_input_state
>          /* This will tell the implementing sink input driver to rewind
>           * so that the unplayed already mixed data is not lost */
>          pa_sink_input_request_rewind(i, 0, TRUE, TRUE, FALSE);
> +    	i->thread_info.state = state;
> 
>      } else if (uncorking) {
> 
>          i->thread_info.underrun_for = (uint64_t) -1;
>          i->thread_info.playing_for = 0;

I took a very slightly different approach but very similar in that I
added a final else which just set the state.

I'll push this shortly after some more testing!

Col

-- 

Colin Guthrie
gmane(at)colin.guthr.ie
http://colin.guthr.ie/

Day Job:
  Tribalogic Limited [http://www.tribalogic.net/]
Open Source:
  Mageia Contributor [http://www.mageia.org/]
  PulseAudio Hacker [http://www.pulseaudio.org/]
  Trac Hacker [http://trac.edgewall.org/]