From patchwork Sat May 7 18:40:01 2005 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Reinhard Nissl X-Patchwork-Id: 11868 Received: from imap.gmx.net ([213.165.64.20] helo=mail.gmx.net) by www.linuxtv.org with smtp (Exim 4.34) id 1DUUEA-00010f-2s for vdr@linuxtv.org; Sat, 07 May 2005 20:40:38 +0200 Received: (qmail invoked by alias); 07 May 2005 18:40:03 -0000 Received: from dialin-145-254-243-243.arcor-ip.net (EHLO [192.168.101.15]) [145.254.243.243] by mail.gmx.net (mp020) with SMTP; 07 May 2005 20:40:03 +0200 X-Authenticated: #527675 Message-ID: <427D0B81.7080605@gmx.de> Date: Sat, 07 May 2005 20:40:01 +0200 From: Reinhard Nissl User-Agent: Mozilla Thunderbird 1.0 (X11/20041206) X-Accept-Language: en-us, en MIME-Version: 1.0 To: Klaus Schmidinger's VDR Subject: Re: [vdr] [PATCH] Hang when moving between editing marks References: <4D516C3CF4%linux@youmustbejoking.demon.co.uk> <427CCC99.7080409@cadsoft.de> <4D675E7CFF%linux@youmustbejoking.demon.co.uk> <427CE6EF.3070108@cadsoft.de> In-Reply-To: <427CE6EF.3070108@cadsoft.de> X-Y-GMX-Trusted: 0 X-BeenThere: vdr@linuxtv.org X-Mailman-Version: 2.1.5 Precedence: list Reply-To: Klaus Schmidinger's VDR List-Id: Klaus Schmidinger's VDR List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 07 May 2005 18:40:38 -0000 Status: O X-Status: X-Keywords: X-UID: 1997 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(). >>> 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. Bye. --- ../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: