On Sat, Feb 4, 2012 at 5:04 PM, Thomas Moulard wrote: > On Sat, Feb 4, 2012 at 5:54 PM, Jack O'Quin wrote: >> Multi-threading is tricky in this driver, see the comments in >> Camera1394Driver::poll(). I am wondering if we could just call the >> updater there in the poll() thread, instead of creating another >> thread. Is there any large overhead in calling the updater? > > The updater is publishing in /diagnostics at a fixed rate so no it is probably > not that time consumming. However, it is still a shame to slow down the > whole node. It's not a matter of slowing down the node. I tried to explain the problem in this comment in the poll() method: // The mutex lock should be sufficient, but the Linux pthreads // implementation does not guarantee fairness, and the reconfig() // callback thread generally suffers from lock starvation for many // seconds before getting to run. So, we avoid acquiring the lock // if there is a reconfig() pending. I worry that introducing another thread will make things worse. I can experiment with different approaches, but it's hard to believe that our test coverage is adequate for those kinds of problems. >> Also, I am still a little confused about how the diagnostics updater >> learns the actual frame rate of the camera. Shouldn't there be a >> common data structure update? Wouldn't that require a mutex lock? > > The patch is missing a line: topic_diagnostics_.tick() should be called > each time a message is published. This function is thead safe so we can > safely publish diagnostics in one thread and tick in the main thread. I see. It looks like FrequencyStatusParam() is a wrapper for the general DiagnosticStatus message. I wonder what the right tolerance is for general 1394 camera frame rates? The frame rate is unlikely to run fast, but I have noticed that cameras tend to slow down in low light situations due to the need for longer exposure times. For various applications a slower rate may or may not be acceptable. We may want to introduce a user parameter to set the tolerance interval. Also, it will be tricky to determine the frame rate when running Format 7 modes. Unfortunately, the configured frame_rate has no effect in that situation. That is not the fault of the driver, but of the IIDC standard. --  joq