Message ID | 42C632CD.5090203@gmx.net |
---|---|
State | New |
Headers |
Received: from pop.gmx.de ([213.165.64.20] helo=mail.gmx.net) by www.linuxtv.org with smtp (Exim 4.34) id 1DobPJ-0005Ho-Dx for vdr@linuxtv.org; Sat, 02 Jul 2005 08:23:17 +0200 Received: (qmail invoked by alias); 02 Jul 2005 06:22:45 -0000 Received: from dialin123.zdv.Uni-Mainz.DE (EHLO [192.168.11.200]) [134.93.174.123] by mail.gmx.net (mp022) with SMTP; 02 Jul 2005 08:22:45 +0200 X-Authenticated: #404998 Message-ID: <42C632CD.5090203@gmx.net> Date: Sat, 02 Jul 2005 08:23:09 +0200 From: Martin Wache <M.Wache@gmx.net> User-Agent: Mozilla Thunderbird 1.0.2 (Macintosh/20050317) X-Accept-Language: de-DE, de, en-us, en MIME-Version: 1.0 To: Klaus Schmidinger's VDR <vdr@linuxtv.org> X-Enigmail-Version: 0.90.2.0 X-Enigmail-Supports: pgp-inline, pgp-mime Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: 7bit X-Y-GMX-Trusted: 0 Subject: [vdr] possible busy loop in cDvbPlayer::Action? X-BeenThere: vdr@linuxtv.org X-Mailman-Version: 2.1.5 Precedence: list Reply-To: Klaus Schmidinger's VDR <vdr@linuxtv.org> List-Id: Klaus Schmidinger's VDR <vdr.linuxtv.org> List-Unsubscribe: <http://www.linuxtv.org/cgi-bin/mailman/listinfo/vdr>, <mailto:vdr-request@linuxtv.org?subject=unsubscribe> List-Archive: <http://www.linuxtv.org/pipermail/vdr> List-Post: <mailto:vdr@linuxtv.org> List-Help: <mailto:vdr-request@linuxtv.org?subject=help> List-Subscribe: <http://www.linuxtv.org/cgi-bin/mailman/listinfo/vdr>, <mailto:vdr-request@linuxtv.org?subject=subscribe> X-List-Received-Date: Sat, 02 Jul 2005 06:23:17 -0000 Status: O X-Status: X-Keywords: X-UID: 3363 |
Commit Message
Martin Wache
July 2, 2005, 6:23 a.m. UTC
Hi Klaus, hi list, we have since some time reports on bad performance and a jerky picture when replaying a recording with the vdr and the softdevice as output device. Finally we've been able to track the problem down: It seems to arise when the softdevices buffers are not full, so that Poll() doesn't sleep (since the softdevice still can accept data), but vdr has no more frames to send to the softdevice. In this case the loop inside of cDevbPlayer::Action will busy loop until there are frames again. This seems to happen only for very fast CPUs, and I guess it can also happen for other output devices. I thought about letting the softdevice sleep even when the buffers are not full, but I think the correct solution would be that vdr sleep in case it doesn't have some frames to send. Or did I get the Poll() function wrong? Should Poll() sleep in any case? I attached a patch which causes vdr to sleep in case it doesn't have more frames to send. This cures the problem for the softdevice users. You can find the corresponding thread in the softdevice mailling lists archive: https://lists.berlios.de/pipermail/softdevice-devel/2005q2/000902.html and https://lists.berlios.de/pipermail/softdevice-devel/2005q3/000913.html Bye, Martin
Comments
Martin Wache wrote: > I thought about letting the softdevice sleep even when the buffers are > not full, but I think the correct solution would be that vdr sleep in > case it doesn't have some frames to send. Or did I get the Poll() > function wrong? Should Poll() sleep in any case? from device.h virtual bool Poll(cPoller &Poller, int TimeoutMs = 0); ///< Returns true if the device itself or any of the file handles in ///< Poller is ready for further action. ///< If TimeoutMs is not zero, the device will wait up to the given number ///< of milleseconds before returning in case there is no immediate ///< need for data. Bye
Luca Olivetti schrieb: > Martin Wache wrote: > >> I thought about letting the softdevice sleep even when the buffers are >> not full, but I think the correct solution would be that vdr sleep in >> case it doesn't have some frames to send. Or did I get the Poll() >> function wrong? Should Poll() sleep in any case? > > > from device.h > > virtual bool Poll(cPoller &Poller, int TimeoutMs = 0); > ///< Returns true if the device itself or any of the file handles in > ///< Poller is ready for further action. > ///< If TimeoutMs is not zero, the device will wait up to the > given number > ///< of milleseconds before returning in case there is no immediate > ///< need for data. > I know this header file, I read it many times, but it doesn't answer my question. If you have a closer look at the methods of cDvbDevice and cPoller, you will see that the method Poll() just calls the linux system call poll() with the event POLLOUT. If you now have a look at the man page of poll which states: #define POLLOUT 0x0004 /* Writing now will not block */ wou will see that this is _exactly_ the way the softdevices Poll() method is currently implemented. It will only sleep if a write to the softdevice would block. What would be needed from the softdevice to break the busy loop is that the softdevice would have to sleep even though it _can_ accept data, but I don't think that this makes sense, or do you? To me it makes much more sense if the vdr would sleep if it doesn't have any data to deliver. So I'm interresteted what you think about this... Bye, Martin
Martin Wache wrote: > Finally we've been able to track the problem down: > It seems to arise when the softdevices buffers are not full, so that > Poll() doesn't sleep (since the softdevice still can accept data), but > vdr has no more frames to send to the softdevice. > In this case the loop inside of cDevbPlayer::Action will busy loop until > there are frames again. This is a design flaw in the player, but only matters if the disk read data rate is lower than device write rate, but this obviously can have side effects at startup. The correct way to handle this situation would be to also poll the source file handle, but only if there's currently nothing to read. I had the same situation in streamplayer, and there it is more critical, because the source file is a socket, and network buffers will overflow quickly. Cheers, Udo
Martin Wache wrote: > Luca Olivetti schrieb: > >>Martin Wache wrote: >> >> >>>I thought about letting the softdevice sleep even when the buffers are >>>not full, but I think the correct solution would be that vdr sleep in >>>case it doesn't have some frames to send. Or did I get the Poll() >>>function wrong? Should Poll() sleep in any case? >> >> >>from device.h >> >> virtual bool Poll(cPoller &Poller, int TimeoutMs = 0); >> ///< Returns true if the device itself or any of the file handles in >> ///< Poller is ready for further action. >> ///< If TimeoutMs is not zero, the device will wait up to the >>given number >> ///< of milleseconds before returning in case there is no immediate >> ///< need for data. >> > > > I know this header file, I read it many times, but it doesn't answer my > question. If you have a closer look at the methods of cDvbDevice and > cPoller, you will see that the method Poll() just calls the linux system > call poll() with the event POLLOUT. If you now have a look at the man > page of poll which states: > #define POLLOUT 0x0004 /* Writing now will not block */ > wou will see that this is _exactly_ the way the softdevices Poll() > method is currently implemented. It will only sleep if a write to the > softdevice would block. Oh, I see, I was too fast and I misinterpreted your request: I though you had the same problem I had with the dxr3 plugin (i.e. it didn't wait when it *couldn't* accept data). Maybe a ff card doesn't have this problem because it doesn't have big buffers, but I also wonder how is it possible that vdr hasn't enough data to give to the device: no matter how big the buffer, once it is filled it should stay more or less full, unless you're at the end of the file. Bye
Luca Olivetti wrote: > I also wonder how is it > possible that vdr hasn't enough data to give to the device: no matter > how big the buffer, once it is filled it should stay more or less full, > unless you're at the end of the file. One guess: On playback start, buffers are empty and can accept lots of data. This could result in some busy loops until enough data is read to fill the buffer. If, for some reason, the disk read requires much CPU time, then the busy loop will actually slow down the disk reading too. Maybe, in case of softdevice, this is oscillating: A busy file reader takes CPU from decoding, causing decoding to stall. And as soon as the decoder is catching up, maybe by dropping frames, the file buffers run empty causing the file reader to busy loop again. (speculative, though) Cheers, Udo
On Samstag, 2. Juli 2005 13:21, Luca Olivetti wrote: > Martin Wache wrote: > > > I thought about letting the softdevice sleep even when the buffers are > > not full, but I think the correct solution would be that vdr sleep in > > case it doesn't have some frames to send. Or did I get the Poll() > > function wrong? Should Poll() sleep in any case? > > from device.h > > virtual bool Poll(cPoller &Poller, int TimeoutMs = 0); > ///< Returns true if the device itself or any of the file handles in > ///< Poller is ready for further action. > ///< If TimeoutMs is not zero, the device will wait up to the > given number > ///< of milleseconds before returning in case there is no immediate > ///< need for data. What about this interpretation: So the question is when has the device an immediate need for data ? This is the case when the buffer of the device is empty. If this condition is met, the device has to ignore a given timeout and return immediate. If buffer empty condition is not satisfied, device has to sleep when timout is not zero.
Udo Richter schrieb: > Luca Olivetti wrote: > >>I also wonder how is it >>possible that vdr hasn't enough data to give to the device: no matter >>how big the buffer, once it is filled it should stay more or less full, >>unless you're at the end of the file. > > > One guess: On playback start, buffers are empty and can accept lots of > data. This could result in some busy loops until enough data is read to > fill the buffer. > > If, for some reason, the disk read requires much CPU time, then the busy > loop will actually slow down the disk reading too. > > Maybe, in case of softdevice, this is oscillating: A busy file reader > takes CPU from decoding, causing decoding to stall. And as soon as the > decoder is catching up, maybe by dropping frames, the file buffers run > empty causing the file reader to busy loop again. (speculative, though) I guess this is a possible scenario, at least it explains the symptoms quite acuratly. Hmm, to finally solve this I guess we have to wait until Klaus answers. I think he is now on his holidays, right? So I'll wait until he is back. But thanks for your answers, at least I know now that the softdevice is not the only one which has problems like this. Bye, Martin
Stefan Lucke schrieb: > On Samstag, 2. Juli 2005 13:21, Luca Olivetti wrote: > >>Martin Wache wrote: >> >> >>>I thought about letting the softdevice sleep even when the buffers are >>>not full, but I think the correct solution would be that vdr sleep in >>>case it doesn't have some frames to send. Or did I get the Poll() >>>function wrong? Should Poll() sleep in any case? >> >>from device.h >> >> virtual bool Poll(cPoller &Poller, int TimeoutMs = 0); >> ///< Returns true if the device itself or any of the file handles in >> ///< Poller is ready for further action. >> ///< If TimeoutMs is not zero, the device will wait up to the >>given number >> ///< of milleseconds before returning in case there is no immediate >> ///< need for data. > > > What about this interpretation: > > So the question is when has the device an immediate need for data ? > This is the case when the buffer of the device is empty. If this > condition is met, the device has to ignore a given timeout and > return immediate. If buffer empty condition is not satisfied, > device has to sleep when timout is not zero. > I don't think that this the correct interpretation. When the buffers are empty, we have a buffer underrun. We'll never get a smooth playback if we let the buffers run empty. If we define the immediate need for data different (maybe 1/3 of the buffer filled) we will effectivly make the buffers smaller, since as soon as we sleep after/before the reception of one packet we will reduce the throughput to a level far below the consumption rate. The smallest sensible sleep is about 3ms that means we will be able to transfer only 13 packets per frame (40ms). Usually a frame has more packets than 13, so the buffer fill will soon drop below our immediate need for data limit. No, the longer I think about it the more confident I get that the only sensible solution is that vdr should sleep in case it doesn't have any data. Martin
El Domingo, 3 de Julio de 2005 10:34, Martin Wache escribió: > Udo Richter schrieb: > > Luca Olivetti wrote: > >>I also wonder how is it > >>possible that vdr hasn't enough data to give to the device: no matter > >>how big the buffer, once it is filled it should stay more or less full, > >>unless you're at the end of the file. > > > > One guess: On playback start, buffers are empty and can accept lots of > > data. This could result in some busy loops until enough data is read to > > fill the buffer. > > > > If, for some reason, the disk read requires much CPU time, then the busy > > loop will actually slow down the disk reading too. > > > > Maybe, in case of softdevice, this is oscillating: A busy file reader > > takes CPU from decoding, causing decoding to stall. And as soon as the > > decoder is catching up, maybe by dropping frames, the file buffers run > > empty causing the file reader to busy loop again. (speculative, though) > > I guess this is a possible scenario, at least it explains the symptoms > quite acuratly. > Hmm, to finally solve this I guess we have to wait until Klaus answers. > I think he is now on his holidays, right? So I'll wait until he is back. > But thanks for your answers, at least I know now that the softdevice is > not the only one which has problems like this. > The patch also work well with vdr-xine with a athlon 64 3200. Without the patch there is a heavy cpu load when playing recordings. Before I use the atached patch with have similar effects. Jose Alberto
--- dvbplayer.c.orig Fri Jul 1 15:14:49 2005 +++ dvbplayer.c Fri Jul 1 15:16:50 2005 @@ -497,6 +497,7 @@ p = NULL; } } + else Sleep=true; } } active = running = false;