dynamically sized ringbuffers v1

  > void cRecorder::Receive(uchar *Data, int Length)
> {
>   if (Running()) {
>      int p = ringBuffer->Put(Data, Length);
>      if (p != Length && Running())
>         ringBuffer->ReportOverflow(Length - p);
>      }
> }
> 
> it simply drops any data that does not fit into the buffer, which would be fine
> for live viewing, but isn't ideal when recording.
> 
> Can we try harder not to loose data here? IOW can this function sleep (and retry)?

It took a while to trigger the condition again, but now it happened and trying a bit
harder payed off. Running vdr /w following patch resulted in this log (and no overflow):

20:31:35 vdr: [16328] buffer usage: 70% (tid=16327)
20:31:35 vdr: [16328] buffer usage: 80% (tid=16327)
20:31:35 vdr: [16328] buffer usage: 90% (tid=16327)
20:31:35 vdr: [16328] buffer usage: 100% (tid=16327)
20:31:35 vdr: [16327] Enlarging ring buffer "Result": 262144 bytes (trigger 3)
20:31:35 vdr: [16329] Enlarging ring buffer "TS": 262144 bytes (trigger 2)
20:31:35 vdr: [16328] Enlarging ring buffer "Recorder": 262144 bytes (trigger 3)
20:31:35 vdr: [16328] buffer usage: 0% (tid=16327)
20:31:35 vdr: [16328] saved extra 153 bytes in recorder ring buffer after 80 ms delay

artur
  

On 05/09/07 20:56, Artur Skawina wrote:
>> void cRecorder::Receive(uchar *Data, int Length)
>> {
>>   if (Running()) {
>>      int p = ringBuffer->Put(Data, Length);
>>      if (p != Length && Running())
>>         ringBuffer->ReportOverflow(Length - p);
>>      }
>> }
>>
>> it simply drops any data that does not fit into the buffer, which would be fine
>> for live viewing, but isn't ideal when recording.
>>
>> Can we try harder not to loose data here? IOW can this function sleep (and retry)?
> 
> It took a while to trigger the condition again, but now it happened and trying a bit
> harder payed off. Running vdr /w following patch resulted in this log (and no overflow):
> 
> 20:31:35 vdr: [16328] buffer usage: 70% (tid=16327)
> 20:31:35 vdr: [16328] buffer usage: 80% (tid=16327)
> 20:31:35 vdr: [16328] buffer usage: 90% (tid=16327)
> 20:31:35 vdr: [16328] buffer usage: 100% (tid=16327)
> 20:31:35 vdr: [16327] Enlarging ring buffer "Result": 262144 bytes (trigger 3)
> 20:31:35 vdr: [16329] Enlarging ring buffer "TS": 262144 bytes (trigger 2)
> 20:31:35 vdr: [16328] Enlarging ring buffer "Recorder": 262144 bytes (trigger 3)
> 20:31:35 vdr: [16328] buffer usage: 0% (tid=16327)
> 20:31:35 vdr: [16328] saved extra 153 bytes in recorder ring buffer after 80 ms delay
> 
> artur
> 
> diff --git a/recorder.c b/recorder.c
> index 8bb1621..3c0e002 100644
> --- a/recorder.c
> +++ b/recorder.c
> @@ -157,8 +157,20 @@ void cRecorder::Receive(uchar *Data, int Length)
>  {
>    if (Running()) {
>       int p = ringBuffer->Put(Data, Length);
> -     if (p != Length && Running())
> +     if (p != Length && Running()) {
> +        for (int ms=20; ms<1000; ms+=ms) {
> +           cCondWait::SleepMs(ms);
> +           if (!Running())
> +              return;
> +           int r = ringBuffer->Put(Data+p, Length-p);
> +           p += r;
> +           if (r)
> +              dsyslog("saved extra %d bytes in recorder ring buffer after %d ms delay", r, ms);
> +           if (p == Length || !Running())
> +              return;
> +           }
>          ringBuffer->ReportOverflow(Length - p);
> +        }
>       }
>  }

>From receiver.h:

  virtual void Receive(uchar *Data, int Length) = 0;
               ///< This function is called from the cDevice we are attached to, and
               ///< delivers one TS packet from the set of PIDs the cReceiver has requested.
               ///< The data packet must be accepted immediately, and the call must return
                                    ****************************                    ******
               ///< as soon as possible, without any unnecessary delay. Each TS packet
                    **************************************************
               ///< will be delivered only ONCE, so the cReceiver must make sure that
               ///< it will be able to buffer the data if necessary.


Klaus
  

Klaus Schmidinger wrote:
> On 05/09/07 20:56, Artur Skawina wrote:
>>> void cRecorder::Receive(uchar *Data, int Length)

>>> it simply drops any data that does not fit into the buffer, which would be fine
>>> for live viewing, but isn't ideal when recording.
>>>
>>> Can we try harder not to loose data here? IOW can this function sleep (and retry)?
>> It took a while to trigger the condition again, but now it happened and trying a bit
>> harder payed off. Running vdr /w following patch resulted in this log (and no overflow):

>>From receiver.h:
> 
>   virtual void Receive(uchar *Data, int Length) = 0;
>                ///< This function is called from the cDevice we are attached to, and
>                ///< delivers one TS packet from the set of PIDs the cReceiver has requested.
>                ///< The data packet must be accepted immediately, and the call must return
>                                     ****************************                    ******
>                ///< as soon as possible, without any unnecessary delay. Each TS packet
>                     **************************************************
>                ///< will be delivered only ONCE, so the cReceiver must make sure that
>                ///< it will be able to buffer the data if necessary.
> 

Yes, saw that, that's why asked if it couldn't sleep after all... And is why i made
it sleep only for for short periods. ASAP is relative ;) Avoiding corrupting recordings
is worth some small glitches when live viewing. Now, the max 1.26s sleep is probably
too much, but i'll need to run with this for a few weeks to gather some meaningful
statistics. So far the overflow has triggered twice in three days, first time (vanilla
Receive()) some data was lost, second time the "unnecessary" delays rescued a few bytes.
Another way to look at this is -- given how rarely this happens i don't think it's
statistically significant ;) There will likely be many more errors due to other factors
during these 36h or so of recordings. So this change isn't necessary, but i'd like to
find out if the data loss can be avoided and it lets me monitor that.

The advantage of auto tuning the buffer sizes is that the max size can be increased
cheaply, as the unneeded parts won't be used at all; i'll do this for the TS buffer;
whether that will be enough to cope with the extra delays i'll probably find out in
a few days...

artur