apl-ocean-engineering / sonar_image_proc

Other
12 stars 3 forks source link

sonar_pointcloud: clean up handling of intensity threshold #32

Open lauralindzey opened 1 year ago

lauralindzey commented 1 year ago

As we started discussing in https://github.com/apl-ocean-engineering/sonar_image_proc/pull/31, I think there's an issue with how intensities are being calculated. At a minimum, it needs some documentation, but I also think there's an unintentional repeat of the thresholding operation.

It looks like intensities are rescaled to a log scale where 0=0 and 1.0 = INT_MAX. That makes sense ... working in dB for this type of data makes sense. (But we're not actually working in "dB" ... )

Then, we only keep intensities between the threshold and 1.0 (on that scale), and stretch that to be 0->1, which I guess makes sense if you want to map the intensities above the threshold to a colormap.

However, later in the code these rescaled intensities are compared to the threshold again, which seems wrong.

So, I'm not sure what the input intensity_threshold parameter is supposed to mean. (I would have guessed "only display points whose intensity is at least this fraction of maximum possible intensity", but it's not used that way in the code)

micat001 commented 1 year ago

I would like intensity_threshold to do what @lauralindzey is describing. Namely work in conjunction with publish_all_pointsto show only the fraction of the pointcloud above a threshold.

Is the original scaling here: https://github.com/apl-ocean-engineering/sonar_image_proc/blob/a83f3ffae84d98f82eeab376f1a662f2e4ce924d/scripts/sonar_pointcloud.py#L75 proper?

And would it make sense to fix this issue in a new PR that also unifies imaging metadata?

lauralindzey commented 1 year ago

Up to you -- normally, I'd put them in separate merge requests, since smaller self-consistent changes are easier to review. But, if you want to do both while you're in the tank this morning, lumping could make sense.