On Mon, Mar 15, 2010 at 10:03 PM, Jack O'Quin <span dir="ltr"><<a href="mailto:jack.oquin@gmail.com" target="_blank">jack.oquin@gmail.com</a>></span> wrote:<br><div class="gmail_quote"><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">

Initial check-in for ROS port of Player camera1394 driver committed to<br>
ros-pkg/trunk/stacks/camera_drivers_experimental/camera1394.<br></blockquote><div><br>Excellent, thanks for your work on this.<br><br></div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">


 * The code is GPL due to the Player driver license<br></blockquote><div><br>We strongly prefer the BSD license, especially for components like this one that are likely to see wide use. Even if there is an excellent GPLed dc1394 ROS driver, some users will be compelled to write their own to avoid the licensing issues.<br>

<br>The LGPL license for libdc1394 is quite OK. The LGPL does not place restrictions on the license of a work that links to the library.<br><br></div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">

 
* It publishes CameraInfo and rviz can display images, but rviz<br>
complains about a bad P value (projection matrix).<br>
<br>
This matrix is not being set in the driver. Should it be? Is there a<br>
useful default value? Or, is that always handled later in the<br>
image_pipeline?<br></blockquote><div><br>For monocular cameras you should set:<br> * R (rotation matrix) to the 3x3 identity matrix.<br> * P (projection matrix) to K (camera matrix) with a column of zeros appended.<br><br>

</div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;"> 
* There is Bayer decoding in the driver.<br>
<br>
Is that OK, or is this only supposed to be done later in the image_pipeline?<br></blockquote><div><br>In the other drivers we've opted not to do any bayer decoding, leaving that to image_proc / stereo_image_proc. For consistency I'd say the image_raw topic should be the raw (bayer) data straight from the camera. If you want to do bayer decoding in the driver I would make that a separate topic.<br>

<br></div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;"> 
* Should this package be supported on Mac OS X?<br></blockquote></div><br>I'm sure Mac users would appreciate it :). If you don't have a Mac to test with perhaps someone else can do that.<br><br>Cheers,<br>Patrick<br>