New VideoToolbox rgb packet processor

[fran6co]

Mac OS X >= 10.8 jpeg decoding (a bit hidden).

It's around the same speed as JpegTurbo but it's a lot gentler on the cpu:

[VTRgbPacketProcessor] avg. time: 20.3996ms -> ~49.0207Hz

vs

[TurboJpegRgbPacketProcessor] avg. time: 23.2275ms -> ~43.0525Hz

[xlz]

How is the CPU usage improvement?

[fran6co]

It's hard to measure, OS X creates a separate system process for decoding. Should I just throw in the %?

I did a double check on it, and it's not using a hardware accelerated jpeg decoder contrary to what I thought. So, not sure if it's worth it.

[xlz]

Ideally it should be using hardware acceleration. Even if not, an alternative is always good. Some users reported color corruption with custom compiled turbojpeg. Besides making it possible to remove the hard dependency on turbojpeg on Mac is also nice.

[fran6co]

Now it replaces TurboJPEG for Mac OS X.

[xlz]

VideoToolbox does not conflict with Turbojpeg. I don't think it needs to replace turbojpeg? Turbojpeg is fine along with this. Making VT the default is sure.

OK it seems the above commit just makes turbojpeg optional for Mac OS. It is fine.

[fran6co]

Not sure what's the use of compiling TurboJPEG if it's not going to be used. In any case just updated the PR to build it anyways if it's available.

[fran6co]

Rebased

[fran6co]

Rebased

[fran6co]

Rebased

[RyanGordon]

Just tested on my MacBook Air, and it's definitely faster than TurboJPEG by about 7-10Hz

[xlz]

Can you rebase again? I took a look and it seems there is no major conflict. If you test OK, I'll merge it quickly.

Also, can you add a paragraph saying "You are still able to build with optional TurboJPEG support with brew install jpeg-turbo along with other dependencies"? (Also, delete[] NULL is legal).

[fran6co]

@xlz Rebased

[xlz]

Can you create a new function to do this?

static RgbPacketProcessor *getDefaultRgbPacketProcessor()
{
#ifdef LIBFREENECT2_WITH_VT_SUPPORT
  return new VTRgbPacketProcessor();
#endif
//in the future
//#ifdef LIBFREENECT_WITH_VAAPI_SUPPORT
//  return ...
//#endif
  return new TurboJpegRgbPacketProcessor();
}

[fran6co]

Done. Had to change it a bit

[xlz]

I don't think returning NULL is well intentioned. But the configuration in CMake makes it never return NULL, so I'm ok with this.

[xlz]

There are things to consider for improvement:

• Does this decoder allocate new buffers internally and memcpy the data from packet.buffer to the internal buffers? Is there memory mapping mechanism that can be used?
• It looks like all the returned Frame share the same pixel buffers? How did you design it such that when the user is reading the content of a Frame, the content is not overwritten by the procesor?
• How to add an option to make it decode in gray and YUV formats?

[xlz]

I see a thing in your code: nil. Did you mean NULL? You should change it next time.

[fran6co]

• The code creates a CMBlockBufferRef that just wraps around packet.buffer, it doesn't create a copy it just access it directly
• VTDecompressionSessionDecodeFrame creates a new pixel buffer, that's why VTFrame locks to it and then frees it when it's done. VTFrame it's just a wrapper around CVPixelBufferRef so it can be used by libfreenect. They are not sharing the same pixel buffer
• https://github.com/OpenKinect/libfreenect2/pull/365/files#diff-7917e764ecaf4c2e59db2ecaff7fd9b5R81 is where it can be set the output format. It looks like it's easy to change it to gray or YUV => https://developer.apple.com/library/mac/documentation/QuartzCore/Reference/CVPixelFormatDescriptionRef/#//apple_ref/doc/constant_group/Pixel_Format_Types
• nil is from Objective-c++, I switched to the C interface and forgot to change it back. It compiles no problem anyways.

[xlz]

It seems you didn't pass some metadata as in https://github.com/OpenKinect/libfreenect2/blob/master/src/turbo_jpeg_rgb_packet_processor.cpp#L93

Can you create a new PR to fix this missing metadata, fix the nil, and fix returning NULL in getDefaultRgbPacketProcessor()? (There must be a fallback jpeg decoder. Returning NULL is just pushing the problem back to runtime instead of build time).