Protonect ignores ^C

[hanyazou]

Overview Description:
The Protonect does not show the window nor images and ignore ^C and ESC key though kill command and ^\ can stop the Protonect.

Version, Platform, and Hardware Bug Found:

1. git log -1 --oneline

   9c80ccd usb: Issue reboot command on Mac OS X

2. uname -a

   Darwin iMac.local 15.2.0 Darwin Kernel Version 15.2.0: Fri Nov 13 19:56:56 PST 2015; root:xnu-3248.20.55~2/RELEASE_X86_64 x86_64

3. lsusb -t

   Bus 000.Dev 001: USB 2.0 Bus,
   Bus 000.Dev 001: USB 2.0 Bus,
   Bus 000.Dev 001: USB 3.0 Bus,
   |__ Bus 020.Dev 018: NuiSensor Adaptor, 480M
   |__ Bus 020.Dev 026: iPad, 480M
   |__ Bus 020.Dev 019: NuiSensor Adaptor, 5G
   |__ Bus 020.Dev 003: Xbox NUI Sensor, 5G
   |__ Bus 026.Dev 001: Hub, 480M
   |__ Bus 026.Dev 002: FaceTime HD Camera (Built-in), 480M
   |__ Bus 029.Dev 001: Hub, 480M
   |__ Bus 029.Dev 002: Hub, 480M
   |__ Bus 029.Dev 003: BRCM20702 Hub, 12M
   |__ Bus 029.Dev 006: Bluetooth USB Host Controller, 12M

4. lspci -nn

   lspci: command not found

Steps to Reproduce:

1. execute ./bin/Protonect
2. hit control + C key
3. repeat 1. and 2.

Actual Results:
Protonect does not show the window nor any images and ignores ^C

Expected Results:
show the window and image and stop when I hit ^C

Reproducibility:
1/100~1/1000

Additional Information:
The Protonect seems not to receive any frames at all though no error messages were displayed. So listener.waitForNewFrame(frames); in the main loop won't return and the variable protonect_shutdown could not take effect.

It might be better to use SyncMultiFrameListener::waitForNewFrame(FrameMap &frame, int milliseconds) instead. But that is not implemented on Mac OS X (and Windows?). It seems that we should add wait(_mutexT &aMutex, const libfreenect2::chrono::duration& rel_time) in tinythread.h at first.

Any comments?

[hanyazou]

Or add some watch dog function inside the library to kick some recovery procedure.

[xlz]

Are you are reporting two bugs here?

1. Protonect does not show the window - ??
2. ignores ^C - this is not quite a bug, as Protonect catches SIGINT but not SIGQUIT. So just use ^, which is perfectly fine.

1/100~1/1000

What is this?

[xlz]

It might be better to use SyncMultiFrameListener::waitForNewFrame(FrameMap &frame, int milliseconds)

You are looking for wait_for(). I have looked at it #494 (comment)

[hanyazou]

Are you are reporting two bugs here?
1.Protonect does not show the window - ??
2.ignores ^C - this is not quite a bug, as Protonect catches SIGINT but not SIGQUIT.

Former is the problem. But I think this problem is depending on the Mac OS X USB or Kinect sensor firmware. So we couldn't solve this directly. Un-plug and re-plug or send shutdown command could recover the Kinect sensor. Here is the issue no.2. If you use SIGQUIT or SIGKILL, the Protonect exit immediately and it does not send the shutdown command. In this case the sensor stays in wrong state until you unplug the USB cable manually.

1/100~1/1000
What is this?

I mean reproducibility probability. It might be lower than 0.1%.

You are looking for wait_for(). I have looked at it #494 (comment)

In your comment, you described "wait_for is actually quite non-trivial to get right". I agree with you. So this issue is "How should we implement the function?" in other words.

I think I might be able to implement wait(mutex, millisecond) using pthread_cond_timewait(). Is there any reason not to use pthread_cond_timedwait() in tinythread.h?

[xlz]

I mean reproducibility probability. It might be lower than 0.1%.

I know it's probability. If it's only 0.1%, how do I (or yourself because I don't even have a Mac) verify the fix?

More importantly, how did Protonect not show any images, how is the log?

Do you mean this is such a state that when the Kinect enters it, all subsequence runs of Protonect will get the same freeze? This is an entirely different bug.

wait_for

Here are two implementations of wait_for

https://github.com/henry4k/vman/blob/c680c9f9299d65e4ca1a0887be9ec66b8546d6a6/third-party/tinythreadpp/source/tinythread.h#L510
https://github.com/tklab-tud/umundo/blob/2c1fe724d8e087c383cb04d58300ed647c23f818/src/umundo/core/thread/tinythread.h#L427

I couldn't verify their correctness.

[hanyazou]

how do I (or yourself because I don't even have a Mac) verify the fix?

I isn't so hard (on my Mac). Just run Protonect -frames 1 repeatedly. Once the problem occurs, the Protonect does not exit and run forever. But it might take for 1~10 hours to reproduce. To verify a fix, it might be more difficult. If it runs repeatedly more than 10000 times or 24 hours w/o this problem, it tells the fix works.

Do you mean this is such a state that when the Kinect enters it, all subsequence runs of Protonect will get the same freeze?

Sorry, I thought 'yes'. But it was not true. Once the problem occurs it tends to occur again in short term. But it will be recovered without shutdown nor up-plug. For example,

1. the Protonect run 1000 times w/o this problem
2. the problem occurs

Version: 0.2.0
Environment variables: LOGFILE=<protonect.log>
Usage: ./bin/Protonect [-gpu=] [gl | cl | cuda | cpu] []
[-noviewer] [-norgb | -nodepth] [-help] [-version]
[-frames ]
To pause and unpause: pkill -USR1 Protonect
[Info] [OpenCLDepthPacketProcessorImpl] devices:
[Info] [OpenCLDepthPacketProcessorImpl] 0: Intel(R) Core(TM) i7-3770 CPU @ 3.40GHz
^@ (CPU)[Intel^@]
[Info] [OpenCLDepthPacketProcessorImpl] 1: GeForce GTX 675MX^@ (GPU)[NVIDIA^@]
[Info] [OpenCLDepthPacketProcessorImpl] selected device: GeForce GTX 675MX^@ (GPU)[N
VIDIA^@]
[Info] [OpenCLDepthPacketProcessorImpl] building OpenCL program...
[Info] [OpenCLDepthPacketProcessorImpl] OpenCL program built successfully
[Info] [Freenect2Impl] enumerating devices...
[Info] [Freenect2Impl] 13 usb devices connected
[Info] [Freenect2Impl] found valid Kinect v2 @20:24 with serial 005697142447
[Info] [Freenect2Impl] found 1 devices
[Info] [Freenect2DeviceImpl] opening...
[Info] [Freenect2DeviceImpl] opened
[Info] [Freenect2DeviceImpl] starting...
[Debug] [Freenect2DeviceImpl] status 0x090000: 1024
[Debug] [Freenect2DeviceImpl] status 0x090000: 9729
[Debug] [Freenect2DeviceImpl] status 0x090000: 9731
[Info] [Freenect2DeviceImpl] submitting rgb transfers...
[Info] [Freenect2DeviceImpl] submitting depth transfers...
[Info] [Freenect2DeviceImpl] started
device serial: 005697142447
device firmware: 4.0.3912.0
[Info] [VTRgbPacketProcessor] avg. time: 5.36851ms -> ~186.272Hz
^C[Info] [VTRgbPacketProcessor] avg. time: 5.42299ms -> ~184.4Hz
snipped
Terminated: 15 ./bin/Protonect cl -frames 1

3. the problem occurs 0~5 times
4. Protonect -frames 1 run normaly for 1000 times again.

More importantly, how did Protonect not show any images, how is the log?

Like above, the Protonect outputs [VTRgbPacketProcessor] avg. line repeatedly and ignore ^C. I think this means listener.waitForNewFrame(frames); in the main loop does not return.

[Info] [VTRgbPacketProcessor] avg. time: 5.36851ms -> ~186.272Hz

This might be a problem of Kinect FW, Mac USB, OpenCL or packet processors. WaitForNewFrames() is not a root cause. I however think that is good idea to enable timeout function of waitForFrame() on Mac (and Windows) so that client application can detect this kind of problem and handle it properly.

/// [loop start]
while(!protonect_shutdown && (framemax == (size_t)-1 || framecount < framemax))
{

• listener.waitForNewFrame(frames);
• if (!listener.waitForNewFrame(frames, 10*1000)) // 10 sconds

•  return -1;
  

Here are two implementations of wait_for
I couldn't verify their correctness.

I took a look at them. I think they are so neat implementation that we can use. How should we verify? I'd like to do. Do you mean that we must have some multi thread test cases to prove that the new wait_for implementation is correct and race condition problem free?

[xlz]

If you want to add a default timeout to waitForNewFrame(frame) I can accept that. To verify is not to use test cases. I mean check the implementation's compliance with standards and APIs. Anyway you can try to add a wait_for implementation then I'll review it.

[hanyazou]

I've considered wait_for implementation in C++03 further and found that 'auto p = bind(...)' notation in waitForNewFrame() was troublesome. 'auto' is not supported in C++03 and the return type of bind() is 'unspecified type'. I can use #ifdef of course. But it might be better that I just specify ENABLE_CXX11=ON to cmake instead of adding wait_for and half-done C++11 mimic.

@xlz Can we add timeout argument only in Protonect.cpp without adding wait_for in the tinythread? This will work if you compile with ENABLE_CXX11=ON and tinythread version just ignore the timeout value.

How about this change?

while(!protonect_shutdown && (framemax == (size_t)-1 || framecount < framemax))
{

• listener.waitForNewFrame(frames);

• if (!listener.waitForNewFrame(frames, 10*1000)) // 10 sconds
• {

•  std::cout << "timeout!" << std::endl;
  

•  return -1;
  

• }

[xlz]

Can we add timeout argument only in Protonect.cpp without adding wait_for in the tinythread?

OK. This is easier than implementing wait_for, ok.

waitForNewFrame(FrameMap &frame, int milliseconds) can still be implemented using wait_for() without a predicate. Just use a while(!impl_->hasNewFrame()) loop.

[hanyazou]

Okey. Before implementing wait_for, I'd like to add more std::chrono mimic in src/tinythread. Current implementation of chrono in tinythread.h is minimal. I think we can borrow some lines from existing implementation if its license allows.

For example, http://llvm.org/svn/llvm-project/libcxx/trunk/include/chrono is provided under MIT and BSD like license. Can we copy some part of that into libfreenect2's src/tinythread/chrono? Is MIT license compatible and acceptable to the project?

And could you tell me what is the goal of supporting old standard, C++03? Is there any concrete target which does not support modern compiler? I can propose better idea if I understood the purpose.

[xlz]

I mean just do this:

add timeout argument only in Protonect.cpp without adding wait_for in the tinythread

C++03 is to be supported until Ubuntu 14.04 reaches end of life.

[xlz]

Maybe we can switch to C++11 default earlier than Ubuntu 14.04 EOL, but C++03 support is there to stay. At least now Jetson TXK1 requires C++03 for CUDA 6.5. And CUDA 7.x requires Visual Studio 2013 which does not support C++11. ALso, Ubuntu 14.04 uses C++03 ABI, which is incompatible with C++11 ABI.

There are a lot of reasons to support C++03 and stay with it as default for now.

[hanyazou]

I wil do this:

add timeout argument only in Protonect.cpp without adding wait_for in the tinythread

Then, can we copy some of chrono functions from LLVM libcxx for C++03 suport?

[xlz]

Less code is better then more code. I'd say just leave it.