possible busy loop in cDvbPlayer::Action?

  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
  

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