Mailing List archive
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[vdr] Re: vdr 1.3.18 cCondWait::SleepMs
Rainer Zocholl wrote:
ludwig.nussel@gmx.de(Ludwig Nussel) 15.01.05 23:42
Klaus Schmidinger wrote:
Ludwig Nussel wrote:
Klaus Schmidinger wrote:
Wherever it is feasible VDR does wait on conditions.
[...]
- cCondWait::SleepMs(1); // this keeps the CPU load
low + playMode_condition.Wait(mutex);
[...]
Why so complicated?
All that needs to be done is to wait a little while at this point,
making sure the time slice will be given up. It doesn't really
matter whether we wait 1, 2 or 3 milliseconds - even 10 would be ok
here.
You said "Wherever it is feasible VDR does wait on conditions" which
obviously is not true in the above case. What has to be done here is
not to wait for time to pass but to wait for playmode to change. No
matter how long you make the sleep it will still be busy waiting. So
instead of causing useless context switches all the time (while
still holding the lock!) you can just suspend the execution of the
thread until Play() changes the playmode. A condition variable looks
exactly like the right tool for that job.
ACK.
But if you look higher in the loop, there are several other
variables beeing polled too!
That maybe the reason, why no real suspend is done here.
Exactly.
IMHO: "normally" all threads should wait "at" something, a "mutex" or a
"condition" or a "queue" or a "mailbox" or a "fifo" or....
But if you look in VDR, most/many (have not counted) threads
seems to be "waiting" in a sleep.
That costs time, consumes power and makes the software slow.
Ok, let's take a closer look at all SleepMs() places:
diseqc.c- int n = strtol(s, &p, 10);
diseqc.c- if (!errno && p != s && n >= 0) {
diseqc.c- if (!parsing)
diseqc.c: cCondWait::SleepMs(n);
diseqc.c- return p;
diseqc.c- }
diseqc.c- esyslog("ERROR: illegal value for wait time in '%s'", s - 1);
--> Here it generates a pause, so I guess it is ok to use SleepMs() - what
else should it wait for?
dvbdevice.c-{
dvbdevice.c- if (digitalAudio != On) {
dvbdevice.c- if (digitalAudio)
dvbdevice.c: cCondWait::SleepMs(1000); // Wait until any leftover digital data has been flushed
dvbdevice.c- SetVolumeDevice(On || IsMute() ? 0 : CurrentVolume());
dvbdevice.c- digitalAudio = On;
dvbdevice.c- }
--> When switching from digital to analog output there is typically still
some leftover data somewhere in the buffers, but there's no way to find
out when all these buffers are actually empty - or is there? So we wait
one second, which is apparently enough to have the buffers flushed.
Try removing this to hear the chirping sound it makes.
dvbdevice.c-#define MIN_IFRAME 400000
dvbdevice.c- for (int i = MIN_IFRAME / Length + 1; i > 0; i--) {
dvbdevice.c- safe_write(fd_video, Data, Length);
dvbdevice.c: cCondWait::SleepMs(3); // allows the buffer to be displayed in case the progress display is active
dvbdevice.c- }
dvbdevice.c-#endif
dvbdevice.c-}
--> For some reason, displaying a still image only works with this sleep. Again,
there's nothing we could reasonably wait for.
rcu.c- sprintf(buf, "C0D%c", *Code);
rcu.c- String(buf);
rcu.c- SetCode(*Code);
rcu.c: cCondWait::SleepMs(2 * REPEATDELAY);
rcu.c- if (receivedCommand) {
rcu.c- SetMode(modeB);
rcu.c- String("----");
--> When a remote control signal comes in, it waits here to see whether it is
a repeated key press. This is done only if a key is actually pressed, so
I don't think this will hurt.
sections.c- if (poll(pfd, NumFilters, 1000) > 0) {
sections.c- bool DeviceHasLock = device->HasLock();
sections.c- if (!DeviceHasLock)
sections.c: cCondWait::SleepMs(100);
sections.c- for (int i = 0; i < NumFilters; i++) {
sections.c- if (pfd[i].revents & POLLIN) {
sections.c- cFilterHandle *fh = NULL;
--> Here we just wait a little in case the device doesn't have a lock, yet.
This should only happed rarely, so no big waste here. Maybe the sleep
could be dropped altogether, since there is a poll() just before it.
thread.c- for (time_t t0 = time(NULL) + WaitSeconds; time(NULL) < t0; ) {
thread.c- if (!Active())
thread.c- return;
thread.c: cCondWait::SleepMs(10);
thread.c- }
thread.c- esyslog("ERROR: thread %ld won't end (waited %d seconds) - canceling it...", childTid, WaitSeconds);
thread.c- }
--> Here we wait until the thread has ended, which is only of importance if
it doesn't end "normally". Again, what else should we wait for here?
thread.c- else if (ret == pid)
thread.c- break;
thread.c- i--;
thread.c: cCondWait::SleepMs(100);
thread.c- }
thread.c- if (!i) {
thread.c- kill(pid, SIGKILL);
--> Same as above.
transfer.c- //XXX audio with the video frames. 2004/09/09 Werner
transfer.c- if (!GotBufferReserve) {
transfer.c- if (ringBuffer->Available() < 3 * MAXFRAMESIZE / 2) {
transfer.c: cCondWait::SleepMs(20); // allow the buffer to collect some reserve
transfer.c- continue;
transfer.c- }
transfer.c- else
--> This just gives the ring buffer a chance to fill up, which only happens
at the beginning of a Transfer Mode, so no big deal in normal operation.
dvbplayer.c- }
dvbplayer.c- }
dvbplayer.c- else
dvbplayer.c: cCondWait::SleepMs(3); // this keeps the CPU load low
dvbplayer.c- }
dvbplayer.c-
dvbplayer.c- // Store the frame in the buffer:
--> Finally, here's the only place where SleepMs() is used in "normal" operation.
But then again, it's only in case of pause mode, and I really don't think that
it should hurt too much. As far as I understood the postings regarding sleeping,
only sleep times up to 2ms cause a busy wait, everything above will imediately
give up the time slice and wait non-busy.
@Ludwig: as Rainer already stated there is also the "running" flag to take
into account here, so I still believe it's ok to use SleepMs() here.
@all: if SleepMs() in it's current implementation (with the addition of making
sure that no sleep time <=2ms will be used) doesn't give up the thread's
time slice, then please suggest another method of generating a non-busy
sleep.
Klaus
Home |
Main Index |
Thread Index