Reinhard Nissl wrote:
> Hi,
>
> Klaus Schmidinger wrote:
>
>>>>> The attached patch should fix a hang (livelock) which can occur when
>>>>> moving between editing marks. Blame Timothy Baldwin if it doesn't
>>>>> work -
>>>>> he suggested it ;-)
>>>
>>>
>>>> Under which conditions do these lockups happen?
>>>> Which output device is in use (FF DVB card or some software player)?
>>>
>>>
>>> It's happened here: budget card, xine-lib.
>>
>>
>> Ah, as I suspected ;-)
>>
>> Could it be that the xine player doesn't implement
>> cDevice::Poll() the way it is supposed to?
>> My guess would be that in case of a still picture the xine player's
>> Poll() function returns immediately instead of waiting for the
>> given timeout. That would explain why everything appears to lock up.
>
>
> We've already discussed vdr-xine's Poll() implementation in this thread
> (~ 26.11.2004):
>
> vdr-1.3.15: high CPU load when showing still picture while editing cut
> marks
>
> At that time, you've decided to put Sleep() in the loop back again. But
> you've put it at a different location than it was in the previous versions.
>
> This can still lead to high cpu load when someone pauses a recording
> almost at the end. At that time, VDR has already read and sent the last
> packet to vdr-xine and is now just waiting for vdr-xine to report (via
> DeviceFlush()) that xine has played the last frame of the given data.
>
> The attached patch fixes this issue by exchanging two "if" blocks, which
> also moves the Sleep() almost to the position where is was located in
> earlier VDR versions.
>
> This patch is also part of my other dvbplayer-2/3 patch, but I've
> extracted it to get it included earlier than the other things of the
> original patches.
>
> But maybe it would be even better to move this Sleep() out of the locked
> area.
>
>> Could you please verify this?
>
>
> Well, as written in the above mentioned thread, after sending the still
> image data to xine, the fifo gets empty and therefore Poll() returns
> immediately, as data can be transfered to xine.
>
> But I do not see any special behaviour for FF cards in this code:
>
> bool cDvbDevice::Poll(cPoller &Poller, int TimeoutMs)
> {
> Poller.Add((playMode == pmAudioOnly
> || playMode == pmAudioOnlyBlack) ? fd_audio : fd_video, true);
> return Poller.Poll(TimeoutMs);
> }
>
> Is it that FF cards behave like that when a still image is shown?
> But as a Clear() is happening before sending the still image, I'd expect
> even a FF card to immediately return here in Poll().
Well, actually I thought that the FF cards would wait in the Poll()
function if they are in still of pause mode, but apparently they don't
(I checked).
>>>> I don't really see what difference this sched_yield() would make, so
>>>> I'd
>>>> like to understand this before simply throwing it in...
>>>
>>>
>>> Quoting from the mail which described the patch:
>>>
>>> | It's livelock!
>>>
>>> | The thread which executes cDvbPlayer::Action(void) (in dvbplayer.c)
>>> locks
>>> | the cDvbPlayer object most of the time when an editing mark is
>>> first jumped
>>> | to. The symptoms are cured by adding a call to sched_yield() before
>>> | LOCK_THREAD in cDvbPlayer::Action(void).
>
>
> Well, I do not have this sched_yield() in my code version, but maybe I
> do not see any effects of missig it as my development PC has an
> HyperThreading processor. When the unlock happens, the other waiting
> thread might immediately be able to enter the critical section.
It works here on my system without the sched_yield(), too. But maybe that's
because I'm still using kernel 2.4.20 and the old DVB driver.
Anyway, I don't see anything wrong with your patch and as far as I have seen
everything still runs ok on my system. So if others (especially those who
wrote that the sched_yield() line helped) could please test this patch
(note: _without_ the sched_yield()!) and let me know whether this also
works for them, I could release VDR 1.3.24 shortly...
Klaus
> ------------------------------------------------------------------------
>
> --- ../vdr-1.3.23-orig/dvbplayer.c 2005-01-14 15:00:56.000000000 +0100
> +++ dvbplayer.c 2005-04-03 10:51:15.000000000 +0200
> @@ -380,8 +468,8 @@ void cDvbPlayer::Action(void)
>
> // Read the next frame from the file:
>
> - if (!readFrame && (replayFile >= 0 || readIndex >= 0)) {
> - if (playMode != pmStill) {
> + if (playMode != pmStill && playMode != pmPause) {
> + if (!readFrame && (replayFile >= 0 || readIndex >= 0)) {
> if (!nonBlockingFileReader->Reading()) {
> if (playMode == pmFast || (playMode == pmSlow && playDir == pdBackward)) {
> uchar FileNumber;
> @@ -438,16 +535,16 @@ void cDvbPlayer::Action(void)
> break;
> }
> }
> - else
> - cCondWait::SleepMs(3); // this keeps the CPU load low
> - }
>
> - // Store the frame in the buffer:
> + // Store the frame in the buffer:
>
> - if (readFrame) {
> - if (ringBuffer->Put(readFrame))
> - readFrame = NULL;
> + if (readFrame) {
> + if (ringBuffer->Put(readFrame))
> + readFrame = NULL;
> + }
> }
> + else
> + cCondWait::SleepMs(3); // this keeps the CPU load low
>
> // Get the next frame from the buffer:
