From patchwork Sun Aug 14 11:26:43 2005 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Klaus Schmidinger X-Patchwork-Id: 11975 Received: from tiger.cadsoft.de ([217.7.101.210]) by www.linuxtv.org with esmtp (Exim 4.34) id 1E4Gdc-0000fL-TU for vdr@linuxtv.org; Sun, 14 Aug 2005 13:26:49 +0200 Received: from raven.cadsoft.de (raven.cadsoft.de [217.7.101.211]) by tiger.cadsoft.de (8.12.7/8.12.7) with ESMTP id j7EBQmHS030109 for ; Sun, 14 Aug 2005 13:26:48 +0200 Received: from [192.168.100.10] (hawk.cadsoft.de [192.168.100.10]) by raven.cadsoft.de (8.13.3/8.13.3) with ESMTP id j7EBQjxq018373 for ; Sun, 14 Aug 2005 13:26:46 +0200 Message-ID: <42FF2A73.3030707@cadsoft.de> Date: Sun, 14 Aug 2005 13:26:43 +0200 From: Klaus Schmidinger Organization: CadSoft Computer GmbH User-Agent: Mozilla Thunderbird 1.0.2 (X11/20050317) X-Accept-Language: en MIME-Version: 1.0 To: vdr@linuxtv.org Subject: Re: [vdr] Centralized 'thread active' handling References: <42FDF3B7.3050302@cadsoft.de> <42FE07B1.2060504@cadsoft.de> <42FE0D6C.10209@gmx.de> <42FE0FC3.20706@cadsoft.de> In-Reply-To: X-Greylist: Sender DNS name whitelisted, not delayed by milter-greylist-2.0 (tiger.cadsoft.de [217.7.101.210]); Sun, 14 Aug 2005 13:26:48 +0200 (CEST) X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-2.0 (raven.cadsoft.de [192.168.1.1]); Sun, 14 Aug 2005 13:26:47 +0200 (CEST) X-BeenThere: vdr@linuxtv.org X-Mailman-Version: 2.1.5 Precedence: list Reply-To: Klaus Schmidinger's VDR List-Id: Klaus Schmidinger's VDR List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 14 Aug 2005 11:26:49 -0000 Status: O X-Status: X-Keywords: X-UID: 4167 Stefan Huelswitt wrote: > On 13 Aug 2005 Klaus Schmidinger wrote: > > >>Now, with the naming problem settled, are there any actual, technical >>issues with this modification? > > > *g* > > If you could provide an updated patch, I'll go and test it. Ok, here it is. cThread::Active() is as it was before, and the new function is called cThread::Running() as suggested by Luca Olivetti. Klaus =================================================================== RCS file: ./RCS/cutter.c retrieving revision 1.8 diff -u -r1.8 ./cutter.c --- ./cutter.c 2005/05/15 14:21:08 1.8 +++ ./cutter.c 2005/08/14 10:51:54 @@ -18,7 +18,6 @@ class cCuttingThread : public cThread { private: const char *error; - bool active; int fromFile, toFile; cFileName *fromFileName, *toFileName; cIndexFile *fromIndex, *toIndex; @@ -35,7 +34,6 @@ :cThread("video cutting") { error = NULL; - active = false; fromFile = toFile = -1; fromFileName = toFileName = NULL; fromIndex = toIndex = NULL; @@ -53,7 +51,6 @@ cCuttingThread::~cCuttingThread() { - active = false; Cancel(3); delete fromFileName; delete toFileName; @@ -67,7 +64,8 @@ if (Mark) { fromFile = fromFileName->Open(); toFile = toFileName->Open(); - active = fromFile >= 0 && toFile >= 0; + if (fromFile < 0 || toFile < 0) + return; int Index = Mark->position; Mark = fromMarks.Next(Mark); int FileSize = 0; @@ -78,7 +76,7 @@ uchar buffer[MAXFRAMESIZE]; bool LastMark = false; bool cutIn = true; - while (active) { + while (Running()) { uchar FileNumber; int FileOffset, Length; uchar PictureType; =================================================================== RCS file: ./RCS/device.c retrieving revision 1.103 diff -u -r1.103 ./device.c --- ./device.c 2005/06/12 13:39:11 1.103 +++ ./device.c 2005/08/14 10:52:08 @@ -156,8 +156,6 @@ SetVideoFormat(Setup.VideoFormat); - active = false; - mute = false; volume = Setup.CurrentVolume; @@ -1126,25 +1124,25 @@ void cDevice::Action(void) { - if (active && OpenDvr()) { - for (; active;) { - // Read data from the DVR device: - uchar *b = NULL; - if (GetTSPacket(b)) { - if (b) { - int Pid = (((uint16_t)b[1] & PID_MASK_HI) << 8) | b[2]; - // Distribute the packet to all attached receivers: - Lock(); - for (int i = 0; i < MAXRECEIVERS; i++) { - if (receiver[i] && receiver[i]->WantsPid(Pid)) - receiver[i]->Receive(b, TS_SIZE); - } - Unlock(); - } - } - else - break; - } + if (Running() && OpenDvr()) { + while (Running()) { + // Read data from the DVR device: + uchar *b = NULL; + if (GetTSPacket(b)) { + if (b) { + int Pid = (((uint16_t)b[1] & PID_MASK_HI) << 8) | b[2]; + // Distribute the packet to all attached receivers: + Lock(); + for (int i = 0; i < MAXRECEIVERS; i++) { + if (receiver[i] && receiver[i]->WantsPid(Pid)) + receiver[i]->Receive(b, TS_SIZE); + } + Unlock(); + } + } + else + break; + } CloseDvr(); } } @@ -1188,10 +1186,8 @@ Receiver->device = this; receiver[i] = Receiver; Unlock(); - if (!active) { - active = true; + if (!Running()) Start(); - } return true; } } @@ -1218,10 +1214,8 @@ else if (receiver[i]) receiversLeft = true; } - if (!receiversLeft) { - active = false; + if (!receiversLeft) Cancel(3); - } } void cDevice::DetachAll(int Pid) @@ -1246,13 +1240,11 @@ delivered = false; ringBuffer = new cRingBufferLinear(Size, TS_SIZE, true, "TS"); ringBuffer->SetTimeouts(100, 100); - active = true; Start(); } cTSBuffer::~cTSBuffer() { - active = false; Cancel(3); delete ringBuffer; } @@ -1262,20 +1254,20 @@ if (ringBuffer) { bool firstRead = true; cPoller Poller(f); - for (; active;) { - if (firstRead || Poller.Poll(100)) { - firstRead = false; - int r = ringBuffer->Read(f); - if (r < 0 && FATALERRNO) { - if (errno == EOVERFLOW) - esyslog("ERROR: driver buffer overflow on device %d", cardIndex); - else { - LOG_ERROR; - break; - } - } - } - } + while (Running()) { + if (firstRead || Poller.Poll(100)) { + firstRead = false; + int r = ringBuffer->Read(f); + if (r < 0 && FATALERRNO) { + if (errno == EOVERFLOW) + esyslog("ERROR: driver buffer overflow on device %d", cardIndex); + else { + LOG_ERROR; + break; + } + } + } + } } } =================================================================== RCS file: ./RCS/device.h retrieving revision 1.60 diff -u -r1.60 ./device.h --- ./device.h 2005/07/30 09:31:53 1.60 +++ ./device.h 2005/08/13 11:44:13 @@ -233,7 +233,6 @@ // PID handle facilities private: - bool active; virtual void Action(void); protected: enum ePidType { ptAudio, ptVideo, ptPcr, ptTeletext, ptDolby, ptOther }; @@ -518,7 +517,6 @@ private: int f; int cardIndex; - bool active; bool delivered; cRingBufferLinear *ringBuffer; virtual void Action(void); =================================================================== RCS file: ./RCS/dvbdevice.c retrieving revision 1.131 diff -u -r1.131 ./dvbdevice.c --- ./dvbdevice.c 2005/06/19 11:00:43 1.131 +++ ./dvbdevice.c 2005/08/14 10:52:26 @@ -76,7 +76,6 @@ cCiHandler *ciHandler; cChannel channel; const char *diseqcCommands; - bool active; bool useCa; time_t startTime; eTunerStatus tunerStatus; @@ -101,7 +100,6 @@ frontendType = FrontendType; ciHandler = CiHandler; diseqcCommands = NULL; - active = false; useCa = false; tunerStatus = tsIdle; startTime = time(NULL); @@ -113,7 +111,6 @@ cDvbTuner::~cDvbTuner() { - active = false; tunerStatus = tsIdle; newSet.Signal(); Cancel(3); @@ -294,8 +291,7 @@ void cDvbTuner::Action(void) { dvb_frontend_event event; - active = true; - while (active) { + while (Running()) { Lock(); if (tunerStatus == tsSet) { while (GetFrontendEvent(event)) =================================================================== RCS file: ./RCS/dvbplayer.c retrieving revision 1.36 diff -u -r1.36 ./dvbplayer.c --- ./dvbplayer.c 2005/07/30 10:00:24 1.36 +++ ./dvbplayer.c 2005/08/14 10:52:45 @@ -79,7 +79,6 @@ int wanted; int length; bool hasData; - bool active; cCondWait newSet; protected: void Action(void); @@ -98,13 +97,11 @@ buffer = NULL; wanted = length = 0; hasData = false; - active = false; Start(); } cNonBlockingFileReader::~cNonBlockingFileReader() { - active = false; newSet.Signal(); Cancel(3); free(buffer); @@ -147,8 +144,7 @@ void cNonBlockingFileReader::Action(void) { - active = true; - while (active) { + while (Running()) { Lock(); if (!hasData && f >= 0 && buffer) { int r = safe_read(f, buffer + length, wanted - length); @@ -187,8 +183,6 @@ cIndexFile *index; int replayFile; bool eof; - bool active; - bool running; bool firstPacket; ePlayModes playMode; ePlayDirs playDir; @@ -207,7 +201,7 @@ public: cDvbPlayer(const char *FileName); virtual ~cDvbPlayer(); - bool Active(void) { return active; } + bool Active(void) { return cThread::Running(); } void Pause(void); void Play(void); void Forward(void); @@ -233,8 +227,6 @@ backTrace = NULL; index = NULL; eof = false; - active = true; - running = false; firstPacket = true; playMode = pmPlay; playDir = pdForward; @@ -353,11 +345,8 @@ if (replayFile >= 0) Start(); } - else if (active) { - running = false; + else Cancel(9); - active = false; - } } void cDvbPlayer::Action(void) @@ -374,8 +363,7 @@ int Length = 0; bool Sleep = false; - running = true; - while (running && (NextFile() || readIndex >= 0 || ringBuffer->Available() || !DeviceFlush(100))) { + while (Running() && (NextFile() || readIndex >= 0 || ringBuffer->Available() || !DeviceFlush(100))) { if (Sleep) { cCondWait::SleepMs(3); // this keeps the CPU load low Sleep = false; @@ -501,7 +489,6 @@ Sleep = true; } } - active = running = false; cNonBlockingFileReader *nbfr = nonBlockingFileReader; nonBlockingFileReader = NULL; =================================================================== RCS file: ./RCS/recorder.c retrieving revision 1.13 diff -u -r1.13 ./recorder.c --- ./recorder.c 2005/01/16 12:53:17 1.13 +++ ./recorder.c 2005/08/14 10:53:28 @@ -29,7 +29,6 @@ uchar pictureType; int fileSize; int recordFile; - bool active; time_t lastDiskSpaceCheck; bool RunningLowOnDiskSpace(void); bool NextFile(void); @@ -43,7 +42,6 @@ cFileWriter::cFileWriter(const char *FileName, cRemux *Remux) :cThread("file writer") { - active = false; fileName = NULL; remux = Remux; index = NULL; @@ -63,7 +61,6 @@ cFileWriter::~cFileWriter() { - active = false; Cancel(3); delete index; delete fileName; @@ -96,13 +93,11 @@ void cFileWriter::Action(void) { time_t t = time(NULL); - active = true; - while (active) { + while (Running()) { int Count; uchar *p = remux->Get(Count, &pictureType); if (p) { - //XXX+ active??? see old version (Busy) - if (!active && pictureType == I_FRAME) // finish the recording before the next 'I' frame + if (!Running() && pictureType == I_FRAME) // finish the recording before the next 'I' frame break; if (NextFile()) { if (index && pictureType != NO_PICTURE) @@ -124,15 +119,12 @@ t = time(NULL); } } - active = false; } cRecorder::cRecorder(const char *FileName, int Ca, int Priority, int VPid, const int *APids, const int *DPids, const int *SPids) :cReceiver(Ca, Priority, VPid, APids, Setup.UseDolbyDigital ? DPids : NULL, SPids) ,cThread("recording") { - active = false; - // Make sure the disk is up and running: SpinUpDisk(FileName); @@ -157,25 +149,22 @@ writer->Start(); Start(); } - else if (active) { - active = false; + else Cancel(3); - } } void cRecorder::Receive(uchar *Data, int Length) { - if (active) { + if (Running()) { int p = ringBuffer->Put(Data, Length); - if (p != Length && active) + if (p != Length && Running()) ringBuffer->ReportOverflow(Length - p); } } void cRecorder::Action(void) { - active = true; - while (active) { + while (Running()) { int r; uchar *b = ringBuffer->Get(r); if (b) { =================================================================== RCS file: ./RCS/recorder.h retrieving revision 1.3 diff -u -r1.3 ./recorder.h --- ./recorder.h 2005/01/15 16:35:53 1.3 +++ ./recorder.h 2005/08/13 11:31:18 @@ -23,7 +23,6 @@ cRingBufferLinear *ringBuffer; cRemux *remux; cFileWriter *writer; - bool active; protected: virtual void Activate(bool On); virtual void Receive(uchar *Data, int Length); =================================================================== RCS file: ./RCS/remote.c retrieving revision 1.42 diff -u -r1.42 ./remote.c --- ./remote.c 2005/03/20 13:25:31 1.42 +++ ./remote.c 2005/08/14 10:53:55 @@ -213,7 +213,6 @@ :cRemote("KBD") ,cThread("KBD remote control") { - active = false; tcgetattr(STDIN_FILENO, &savedTm); struct termios tm; if (tcgetattr(STDIN_FILENO, &tm) == 0) { @@ -230,7 +229,6 @@ cKbdRemote::~cKbdRemote() { kbdAvailable = false; - active = false; Cancel(3); tcsetattr(STDIN_FILENO, TCSANOW, &savedTm); } @@ -261,12 +259,11 @@ void cKbdRemote::Action(void) { cPoller Poller(STDIN_FILENO); - active = true; - while (active) { + while (Running()) { if (Poller.Poll(100)) { uint64 Command = 0; uint i = 0; - while (active && i < sizeof(Command)) { + while (Running() && i < sizeof(Command)) { uchar ch; int r = read(STDIN_FILENO, &ch, 1); if (r == 1) { =================================================================== RCS file: ./RCS/remote.h retrieving revision 1.29 diff -u -r1.29 ./remote.h --- ./remote.h 2004/05/28 14:14:02 1.29 +++ ./remote.h 2005/08/13 11:28:10 @@ -81,7 +81,6 @@ class cKbdRemote : public cRemote, private cThread { private: - bool active; static bool kbdAvailable; static bool rawMode; struct termios savedTm; =================================================================== RCS file: ./RCS/sections.c retrieving revision 1.11 diff -u -r1.11 ./sections.c --- ./sections.c 2005/05/29 11:43:17 1.11 +++ ./sections.c 2005/08/14 10:54:39 @@ -44,7 +44,6 @@ { shp = new cSectionHandlerPrivate; device = Device; - active = false; statusCount = 0; on = false; waitForLock = false; @@ -54,7 +53,6 @@ cSectionHandler::~cSectionHandler() { - active = false; Cancel(3); cFilter *fi; while ((fi = filters.First()) != NULL) @@ -166,9 +164,8 @@ void cSectionHandler::Action(void) { - active = true; SetPriority(19); - while (active) { + while (Running()) { Lock(); if (waitForLock) =================================================================== RCS file: ./RCS/sections.h retrieving revision 1.4 diff -u -r1.4 ./sections.h --- ./sections.h 2004/08/08 13:44:17 1.4 +++ ./sections.h 2005/08/13 11:23:55 @@ -25,7 +25,6 @@ private: cSectionHandlerPrivate *shp; cDevice *device; - bool active; int statusCount; bool on, waitForLock; time_t lastIncompleteSection; =================================================================== RCS file: ./RCS/thread.c retrieving revision 1.43 diff -u -r1.43 ./thread.c --- ./thread.c 2005/05/29 11:40:30 1.43 +++ ./thread.c 2005/08/14 11:15:42 @@ -197,7 +197,7 @@ cThread::cThread(const char *Description) { - running = false; + active = running = false; childTid = 0; description = NULL; SetDescription(Description); @@ -205,6 +205,7 @@ cThread::~cThread() { + Cancel(); // just in case the derived class didn't call it free(description); } @@ -234,20 +235,21 @@ if (Thread->description) dsyslog("%s thread ended (pid=%d, tid=%ld)", Thread->description, getpid(), pthread_self()); Thread->running = false; + Thread->active = false; return NULL; } bool cThread::Start(void) { - if (!running) { - running = true; + if (!active) { + active = running = true; if (pthread_create(&childTid, NULL, (void *(*) (void *))&StartThread, (void *)this) == 0) { pthread_detach(childTid); // auto-reap pthread_setschedparam(childTid, SCHED_RR, 0); } else { LOG_ERROR; - running = false; + active = running = false; return false; } } @@ -256,7 +258,7 @@ bool cThread::Active(void) { - if (running) { + if (active) { // // Single UNIX Spec v2 says: // @@ -271,7 +273,7 @@ if (err != ESRCH) LOG_ERROR; childTid = 0; - running = false; + active = running = false; } else return true; @@ -281,7 +283,8 @@ void cThread::Cancel(int WaitSeconds) { - if (running) { + running = false; + if (active) { if (WaitSeconds > 0) { for (time_t t0 = time(NULL) + WaitSeconds; time(NULL) < t0; ) { if (!Active()) @@ -292,7 +295,7 @@ } pthread_cancel(childTid); childTid = 0; - running = false; + active = false; } } =================================================================== RCS file: ./RCS/thread.h retrieving revision 1.28 diff -u -r1.28 ./thread.h --- ./thread.h 2005/05/29 11:31:24 1.28 +++ ./thread.h 2005/08/14 11:14:34 @@ -75,6 +75,7 @@ class cThread { friend class cThreadLock; private: + bool active; bool running; pthread_t childTid; cMutex mutex; @@ -86,13 +87,30 @@ void Lock(void) { mutex.Lock(); } void Unlock(void) { mutex.Unlock(); } virtual void Action(void) = 0; + ///< A derived cThread class must implement the code it wants to + ///< execute as a separate thread in this function. If this is + ///< a loop, it must check Running() repeatedly to see whether + ///< it's time to stop. + bool Running(void) { return running; } + ///< Returns false if a derived cThread object shall leave its Action() + ///< function. void Cancel(int WaitSeconds = 0); + ///< Cancels the thread by first setting 'running' to false, so that + ///< the Action() loop can finish in an orderly fashion and then waiting + ///< up to WaitSeconds seconds for the thread to actually end. If the + ///< thread doesn't end by itself, it is killed. public: cThread(const char *Description = NULL); + ///< Creates a new thread. + ///< If Description is present, a log file entry will be made when + ///< the thread starts and stops. The Start() function must be called + ///< to actually start the thread. virtual ~cThread(); void SetDescription(const char *Description, ...); bool Start(void); + ///< Actually starts the thread. bool Active(void); + ///< Checks whether the thread is still alive. static bool EmergencyExit(bool Request = false); }; =================================================================== RCS file: ./RCS/transfer.c retrieving revision 1.28 diff -u -r1.28 ./transfer.c --- ./transfer.c 2005/02/19 14:38:55 1.28 +++ ./transfer.c 2005/08/14 10:55:03 @@ -21,7 +21,6 @@ ringBuffer = new cRingBufferLinear(TRANSFERBUFSIZE, TS_SIZE * 2, true, "Transfer"); remux = new cRemux(VPid, APids, Setup.UseDolbyDigital ? DPids : NULL, SPids); needsBufferReserve = Setup.UseDolbyDigital && VPid != 0 && DPids && DPids[0] != 0; - active = false; } cTransfer::~cTransfer() @@ -34,21 +33,17 @@ void cTransfer::Activate(bool On) { - if (On) { - if (!active) - Start(); - } - else if (active) { - active = false; + if (On) + Start(); + else Cancel(3); - } } void cTransfer::Receive(uchar *Data, int Length) { - if (IsAttached() && active) { + if (IsAttached() && Running()) { int p = ringBuffer->Put(Data, Length); - if (p != Length && active) + if (p != Length && Running()) ringBuffer->ReportOverflow(Length - p); return; } @@ -70,8 +65,7 @@ bool GotBufferReserve = false; int RequiredBufferReserve = KILOBYTE(DvbCardWith4MBofSDRAM ? 288 : 576); #endif - active = true; - while (active) { + while (Running()) { #ifdef FW_NEEDS_BUFFER_RESERVE_FOR_AC3 if (needsBufferReserve && !GotBufferReserve) { //XXX For dolby we've to fill the buffer because the firmware does @@ -145,7 +139,6 @@ } } } - active = false; } // --- cTransferControl ------------------------------------------------------ =================================================================== RCS file: ./RCS/transfer.h retrieving revision 1.9 diff -u -r1.9 ./transfer.h --- ./transfer.h 2005/01/15 16:39:39 1.9 +++ ./transfer.h 2005/08/13 10:16:02 @@ -21,7 +21,6 @@ cRingBufferLinear *ringBuffer; cRemux *remux; bool needsBufferReserve; - bool active; protected: virtual void Activate(bool On); virtual void Receive(uchar *Data, int Length);