This is still in a just-for-testing phase. Found a way to better stress test this code, hence these changes:
- decreased the growth threshold from 1/2 to 1/3. IOW the goal is always for 66% of the buffer to be free; this increases the working set, but the alternative (handling overflows by spilling over to the unused portion) makes thing more complex. - made buffers grow by 1.5x, not 2x. Related to above. - increased the initial bufsize from 128k to 192k. - removed some now redundant Read() code. - dropped the controversial cRecorder::Receive hunk (I'm keeping this locally as it prevents (very rare) data corruption while testing. Overflows will be logged even w/o this change, so it makes no difference otherwise). - added verbose cRecorder::Action sleep logging. Shorter delays are a side effect, but do not help all that much (from the few times this triggered here so far less than half of the delays were <<100ms).
Patch attached (apparently this makes extracting it easier for gmail users).
artur
diff --git a/dvbdevice.c b/dvbdevice.c index 955483e..20dcf29 100644 --- a/dvbdevice.c +++ b/dvbdevice.c @@ -1184,7 +1184,7 @@ bool cDvbDevice::OpenDvr(void) CloseDvr(); fd_dvr = DvbOpen(DEV_DVB_DVR, CardIndex(), O_RDONLY | O_NONBLOCK, true); if (fd_dvr >= 0) - tsBuffer = new cTSBuffer(fd_dvr, MEGABYTE(2), CardIndex() + 1); + tsBuffer = new cTSBuffer(fd_dvr, MEGABYTE(8), CardIndex() + 1); return fd_dvr >= 0; }
diff --git a/recorder.c b/recorder.c index 8bb1621..0b625d6 100644 --- a/recorder.c +++ b/recorder.c @@ -171,8 +171,22 @@ void cRecorder::Action(void) int Count = remux->Put(b, r); if (Count) ringBuffer->Del(Count); - else - cCondWait::SleepMs(100); // avoid busy loop when resultBuffer is full in cRemux::Put() - } + else { // avoid busy loop when resultBuffer is full in cRemux::Put() + int ms; + for (ms=5; Running() && ms<100; ms+=ms) { + cCondWait::SleepMs(ms); + if (!Running()) + return; + Count = remux->Put(b, r); + if (Count) { + ringBuffer->Del(Count); + dsyslog("cRecorder::Action() ring buffer consumer slept %d ms", ms); + break; + } + } + if (ms>=100) + dsyslog("cRecorder::Action() ring buffer consumer slept >100 ms"); + } + } } } diff --git a/remux.c b/remux.c index da805b7..bd29b2f 100644 --- a/remux.c +++ b/remux.c @@ -1851,7 +1851,7 @@ void cTS2PES::ts_to_pes(const uint8_t *Buf) // don't need count (=188)
// --- cRemux ----------------------------------------------------------------
-#define RESULTBUFFERSIZE KILOBYTE(256) +#define RESULTBUFFERSIZE MEGABYTE(2)
cRemux::cRemux(int VPid, const int *APids, const int *DPids, const int *SPids, bool ExitOnFailure) { diff --git a/ringbuffer.c b/ringbuffer.c index 0633bd3..68ce081 100644 --- a/ringbuffer.c +++ b/ringbuffer.c @@ -151,13 +151,31 @@ void cRingBufferLinear::PrintDebugRBL(void) } #endif
+// cRingBufferLinear are dynamically sized, or at least we can pretend they are ;) +// We treat 'Size' as the maximum size, but start with a small buffer, which can +// grow later as it fills up. Memory is always allocated for the full buffer, but +// the unused RAM portion remains untouched until (if at all) it is actually needed. +// Note that we can not start with a larger than requested buffer because there are +// ring buffer users that cause crashes then (eg softdevice mpegdecoder absolutely +// needs 32/64k). + +// Startup size. 64k still causes overflows before buffer starts to grow, 128k doesn't. +// The buffers grow to 200..500k anyway, so maybe increasing this a bit more would +// make sense, but let's first see if it's really needed. +// In fact 128k is on the low side, but let's try not going for 256k yet. +#define DEFRBSIZE KILOBYTE(192) + cRingBufferLinear::cRingBufferLinear(int Size, int Margin, bool Statistics, const char *Description) -:cRingBuffer(Size, Statistics) +:cRingBuffer(min(Size,DEFRBSIZE), Statistics) { description = Description ? strdup(Description) : NULL; tail = head = margin = Margin; gotten = 0; buffer = NULL; + maxsize = Size; + growthresh = size<maxsize ? size/3 : maxsize+1; + growbuf = 0; + dsyslog("New ring buffer "%s" size: %d margin: %d", description, Size, Margin ); if (Size > 1) { // 'Size - 1' must not be 0! if (Margin <= Size / 2) { buffer = MALLOC(uchar, Size); @@ -183,6 +201,8 @@ cRingBufferLinear::~cRingBufferLinear() #ifdef DEBUGRINGBUFFERS DelDebugRBL(this); #endif + dsyslog("Deleting ring buffer "%s" size: %d / %d (used %g%% of requested size)", description, + size, maxsize, size/(double)maxsize*100.0 ); free(buffer); free(description); } @@ -205,8 +225,20 @@ void cRingBufferLinear::Clear(void) EnablePut(); }
+// Must only be called by the producer (ie Read()/Put()), not the consumer (Get()). +void cRingBufferLinear::GrowBuffer(void) +{ + size = min(size+size/2, maxsize); + growthresh = size<maxsize ? size/3 : maxsize+1; + dsyslog("Enlarging ring buffer "%s": %d bytes (trigger %d, thresh %d)", + description, size, growbuf, growthresh); + growbuf = 0; +} + int cRingBufferLinear::Read(int FileHandle, int Max) { + if (growbuf && head>=tail && size<maxsize) + GrowBuffer(); int Tail = tail; int diff = Tail - head; int free = (diff > 0) ? diff - 1 : Size() - head; @@ -219,6 +251,10 @@ int cRingBufferLinear::Read(int FileHandle, int Max) Count = safe_read(FileHandle, buffer + head, free); if (Count > 0) { int Head = head + Count; + if (Available()+Count >= growthresh) + growbuf = 2; + if (growbuf && head>=tail && size<maxsize) + GrowBuffer(); if (Head >= Size()) Head = margin; head = Head; @@ -245,6 +281,10 @@ int cRingBufferLinear::Read(int FileHandle, int Max) int cRingBufferLinear::Put(const uchar *Data, int Count) { if (Count > 0) { + if (Available()+Count >= growthresh) + growbuf = 3; + if (growbuf && head>=tail && size<maxsize) + GrowBuffer(); int Tail = tail; int rest = Size() - head; int diff = Tail - head; diff --git a/ringbuffer.h b/ringbuffer.h index dba0e61..3545520 100644 --- a/ringbuffer.h +++ b/ringbuffer.h @@ -18,11 +18,11 @@ private: cCondWait readyForPut, readyForGet; int putTimeout; int getTimeout; - int size; time_t lastOverflowReport; int overflowCount; int overflowBytes; protected: + int size; tThreadId getThreadTid; int maxFill;//XXX int lastPercent; @@ -59,7 +59,11 @@ private: int margin, head, tail; int gotten; uchar *buffer; + int growbuf; + int growthresh; + int maxsize; char *description; + void GrowBuffer(void); public: cRingBufferLinear(int Size, int Margin = 0, bool Statistics = false, const char *Description = NULL); ///< Creates a linear ring buffer.
It seems your patch is against VDR 1.5.x. Could you please backport it to VDR 1.4.x?
2007/5/15, VDR User user.vdr@gmail.com:
In case anyone is having trouble, here's an attached patch of v3 for vdr-1.5.2.
You mean 1.4 :) Thanks.
On 5/15/07, Jaroslaw Swierczynski swiergot@gmail.com wrote:
2007/5/15, VDR User user.vdr@gmail.com:
In case anyone is having trouble, here's an attached patch of v3 for vdr-1.5.2.
You mean 1.4 :) Thanks.
No, I mean vdr-1.5.2. Artur said he had attached a patch to his post but there was no attachment and since copy & paste the patch from his post was causing problems for some reason, I patched a vanilla vdr-1.5.2 (which is what I use) by hand. If it applies to any 1.4.x then I guess that's just a bonus.
2007/5/15, VDR User user.vdr@gmail.com:
No, I mean vdr-1.5.2. Artur said he had attached a patch to his post but there was no attachment and since copy & paste the patch from his post was causing problems for some reason, I patched a vanilla vdr-1.5.2 (which is what I use) by hand. If it applies to any 1.4.x then I guess that's just a bonus.
Sorry, I was confused by the fact that the patch didn't apply to 1.4.7 so I assumed it was against 1.5.x as I didn't think differences between 1.4.x releases were so significant. Artur has cleared up the issue stating he had made the patch with 1.4.6. However now when I'm thinking about it perhaps copy&paste was the problem.
I just wanted to give some feedback on this, im using the patch on my vdr 1.4.7 and it workes without problems for now. Good work, thanks.
Dortje
In case anyone is having trouble, here's an attached patch of v3 for vdr-1.5.2.
vdr mailing list vdr@linuxtv.org http://www.linuxtv.org/cgi-bin/mailman/listinfo/vdr
I've recorded/watched a lot of tv in the last couple days and so far I haven't noticed any problems with this patch. I am currently using v3 (the patch I attached to my previous post) with vdr-1.5.2.
VDR User wrote:
I've recorded/watched a lot of tv in the last couple days and so far I haven't noticed any problems with this patch. I am currently using v3 (the patch I attached to my previous post) with vdr-1.5.2.
Do you know how large the buffers usually grow? If you haven't reduced vdrs log level (-l) this info should be somewhere in /var/log/ and you can take a look at the buffer usage w/ eg:
$ grep vdr /var/log/* | grep 'ring buffer' | less
Did any overflows happen (you can search for 'overflow' in the output of above commands to find them)?
I've now checked my vdr logs here (starting ~ a week ago) and not a single buffer overflowed. I would have expected a few problems, but it looks like the tuning worked. (However that vdr has all data moving threads running at real time priority and this may have helped too.)
Thanks,
artur
On 5/20/07, Artur Skawina art_k@o2.pl wrote:
VDR User wrote:
I've recorded/watched a lot of tv in the last couple days and so far I haven't noticed any problems with this patch. I am currently using v3 (the patch I attached to my previous post) with vdr-1.5.2.
Do you know how large the buffers usually grow? If you haven't reduced vdrs log level (-l) this info should be somewhere in /var/log/ and you can take a look at the buffer usage w/ eg:
I use level 3 logging for vdr. I emailed you a copy of my logs so you should see that by the time you read this.
VDR User wrote:
On 5/20/07, Artur Skawina art_k@o2.pl wrote:
VDR User wrote:
I've recorded/watched a lot of tv in the last couple days and so far I haven't noticed any problems with this patch. I am currently using v3 (the patch I attached to my previous post) with vdr-1.5.2.
Do you know how large the buffers usually grow? If you haven't reduced vdrs log level (-l) this info should be somewhere in /var/log/ and you can take a look at the buffer usage w/ eg:
I use level 3 logging for vdr. I emailed you a copy of my logs so you should see that by the time you read this.
Thanks. It seems that 648k was the largest size ever used for a buffer, which is less than 1/3 of the full size (2M for Result,Transfer and TS). The Recorder buffer was typically at 5% of its max size, ie 300k instead of 5M. No overflows w/ v3 of the patch (two with the previous version, which was starting out with just 128k).
I get a lot worse results here (buffers grow to a few M), but that's probably because the vdr box is not a dedicated one, but also handles quite a few other services; IO stalls are not uncommon and several simultaneous recordings are sometimes done while streaming via streamdev to another vdr. Still, no overflows.
artur
On 5/20/07, Artur Skawina art_k@o2.pl wrote:
Thanks. It seems that 648k was the largest size ever used for a buffer, which is less than 1/3 of the full size (2M for Result,Transfer and TS). The Recorder buffer was typically at 5% of its max size, ie 300k instead of 5M. No overflows w/ v3 of the patch (two with the previous version, which was starting out with just 128k).
I get a lot worse results here (buffers grow to a few M), but that's probably because the vdr box is not a dedicated one, but also handles quite a few other services; IO stalls are not uncommon and several simultaneous recordings are sometimes done while streaming via streamdev to another vdr. Still, no overflows.
My vdr box is dedicated with the only other software running being an ftpd for my lan. No streaming video or anything like that. One dedicated vdr box connected to one tv in my living room, thats it. :)
2007/5/15, Artur Skawina art_k@o2.pl:
actually it's vs 1.4.6, but applies to 1.5.2 too.
Ah, I see. I tried to apply it to 1.4.7. The patch provided by VDR User applies.