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

[Wang Xingchao]

2011/8/15 Colin Guthrie <gmane at colin.guthr.ie>:
> 'Twas brillig, and Colin Guthrie at 13/08/11 12:54 did gyre and gimble:
>> '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!
>
> For reference:
>
> http://cgit.freedesktop.org/pulseaudio/pulseaudio/commit/?id=2f55da5baa0323af76510fd07c540f6dafcf1ac0
>
> Hope you agree this is the best way to document the issue, even if there
> are less LoC that could achieve the same result.
>
> Col
>

Agree! Thanks Col! :-)

--xingchao
>
> --
>
> 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/]
>
> _______________________________________________
> pulseaudio-discuss mailing list
> pulseaudio-discuss at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
>