Message ID | 46421958.6070104@o2.pl |
---|---|
State | New |
Headers |
Received: from mx12.go2.pl ([193.17.41.142] helo=poczta.o2.pl) by www.linuxtv.org with esmtp (Exim 4.50) id 1HlrLO-0001Kg-11 for vdr@linuxtv.org; Wed, 09 May 2007 20:57:00 +0200 Received: from poczta.o2.pl (mx12 [127.0.0.1]) by poczta.o2.pl (Postfix) with ESMTP id A654E3E80FE for <vdr@linuxtv.org>; Wed, 9 May 2007 20:56:25 +0200 (CEST) Received: from dki146.neoplus.adsl.tpnet.pl (dki146.neoplus.adsl.tpnet.pl [83.24.12.146]) by poczta.o2.pl (Postfix) with ESMTP for <vdr@linuxtv.org>; Wed, 9 May 2007 20:56:25 +0200 (CEST) Received: (qmail 16763 invoked from network); 9 May 2007 18:56:25 -0000 Received: from unknown (HELO ?172.19.43.221?) (172.19.43.221) by 172.19.43.250 with SMTP; 9 May 2007 18:56:25 -0000 Message-ID: <46421958.6070104@o2.pl> Date: Wed, 09 May 2007 20:56:24 +0200 From: Artur Skawina <art_k@o2.pl> User-Agent: Thunderbird 3.0a1 (X11/20070320) MIME-Version: 1.0 To: VDR Mailing List <vdr@linuxtv.org> Subject: Re: [vdr] [PATCH] dynamically sized ringbuffers v1 References: <463FA500.4010200@o2.pl> <4640BFA8.50102@o2.pl> In-Reply-To: <4640BFA8.50102@o2.pl> X-Enigmail-Version: 0.95b Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-BeenThere: vdr@linuxtv.org X-Mailman-Version: 2.1.5 Precedence: list Reply-To: VDR Mailing List <vdr@linuxtv.org> List-Id: VDR Mailing List <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: Wed, 09 May 2007 18:57:00 -0000 Status: O X-Status: X-Keywords: X-UID: 12830 |
Commit Message
Artur Skawina
May 9, 2007, 6:56 p.m. UTC
> 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
Comments
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
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); + } } }