Hi Kevin,<br><br>I don't quite understand your feedback. I think you may have interpreted the REP in a way very different from what I intended.<br><br><div class="gmail_quote">On Tue, Dec 6, 2011 at 6:07 PM, Kevin Walchko <span dir="ltr"><<a href="mailto:kevin.walchko@gmail.com" target="_blank">kevin.walchko@gmail.com</a>></span> wrote:<br>

<blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;"><div style="word-wrap: break-word;"><div>-1</div><div><br></div><div>It is a little hard to understand this trade due to lack of info, but I will justify my answer:</div>

<div><br></div><div>- There is a major re-write of code (both core ROS and community provided code) that could take significant time and effort to perform. It would be useful if an estimate of this impact could be given (even a WAG). However, maybe I am wrong.</div>

</div></blockquote><div><br>The proposed depth image representation is <i>exactly</i> the one currently used by the ROS OpenNI driver. The intent of this REP is to codify existing practice. What code would need to be rewritten?<br>

<br></div><blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;"><div style="word-wrap: break-word;"><div>- There appear to be no crucial information included in the new DisparityImage to really justify the change and the effort above. At least there is no clear, compelling argument in the REP. </div>

</div></blockquote><div><br>sensor_msgs/DisparityImage is not new - it's been around since pre-Box Turtle. Using sensor_msgs/Image to represent dense depth data is a comparatively new practice, and the subject of the REP.<br>
<br></div><blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;"><div style="word-wrap: break-word;">
<div>- I dislike the justification being because one vendor has set a precedent. What happens when the kinect 2 comes out with a different format, will we adapt that format as the next default? What happens if OpenNI goes away and Nintendo (just picked someone at random) invents a really cool depth sensor that works different? I think our depth image should be based on what is right for us and not what one vender provides today. Just remember, the Kinect is only a little over one year old. What will next year this time bring?</div>

<div><br></div><div>I did like the idea of uint16 taking less bandwidth and potentially running on simpler hardware (ARM, netbook, etc). However, I find none of the arguments compelling.</div></div></blockquote><div><br>
The canonical format - following standard ROS practices - is to use floats, as described in "Canonical Representation." Depth image consumers need only support the float format.<br><br></div><blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;">
<div style="word-wrap: break-word;"><div>So you are proposing:</div>
<div>DisparityImage of type uint16</div><div>- focal length, baseline, principal point</div><div>- valid_window (seems unnecessary)</div><div>- min/max disparity</div><div>- delta_d</div></div></blockquote><div><br>Not at all. I am arguing against DisparityImage, for the reasons described in "Why Not sensor_msgs/DisparityImage."<br>
<br></div><blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;"><div style="word-wrap: break-word;"><div>I assume we would get rid of the need for CameraInfo when used with DisparityImage message if this is done.</div>
</div></blockquote><div><br>Well, this is one of the issues with the old DisparityImage message. It included focal length and baseline to allow converting disparities to depths, but to properly compute a point cloud you need the rest of the CameraInfo parameters anyway.<br>
<br></div><blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;"><div style="word-wrap: break-word;">
<div>What is the impact of instead publishing a separate topic to fill in these couple of missing metadata pieces? This changes things from (Image, CameraInfo) pair to (Image, CameraInfo, ??) triplet … not sure that is super great, but the impact may be much less. </div>
</div></blockquote><div><br>I think we're in agreement here - I'm in favor of a separate topic with the missing metadata, should we eventually find it necessary.<br><br></div><blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;">
<div style="word-wrap: break-word;">
<div>Also, not to be too critical, but I had a problem following the logic of this entire REP. I am sure it makes complete sense to the author, however I found the logic to jump around.</div></div></blockquote><div><br>The REP follows the standard structure (<a href="http://www.ros.org/reps/rep-0001.html#what-belongs-in-a-successful-rep">http://www.ros.org/reps/rep-0001.html#what-belongs-in-a-successful-rep</a>). If some of my language was confusing, please let me know.<br>
<br>Cheers,<br>Patrick<br>
</div></div>