PX4 / PX4-Flow

Firmware for PX4FLOW board
232 stars 340 forks source link

Improve flow quality #90

Open lericson opened 7 years ago

lericson commented 7 years ago

We noticed quite heavy flickering in the image output from the camera sensor. We tested an array of approaches to mitigate the issue.

Primarily we perform whitening of the camera input. This improves the flow quality and stability immensely for us.

LorenzMeier commented 7 years ago

And can you take a picture of the hardware? If the supply is not properly stabilized you might suffer from a bad HW build.

lericson commented 7 years ago

Hold on, apparently can't send images on github.

lericson commented 7 years ago

https://www.dropbox.com/sc/8jlcyy7gj9tknvy/AAANLvfISKlj71DTSpeL4wZCa

On 15 Jun 2017, at 10:52, Lorenz Meier notifications@github.com wrote:

And can you take a picture of the hardware? If the supply is not properly stabilized you might suffer from a bad HW build.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread.

lericson commented 7 years ago

Pushed the suggested changes!

mhkabir commented 7 years ago

Great job on getting that NUC onto a 250 quad! :O

lericson commented 7 years ago

~FYI the CI failure seems to be a bug, refresh it and it should work -- it checked out old code.~ Sorry my fault, forgot to commit a second change. Force pushing.

lericson commented 7 years ago

By the way, the difference in signal quality from the flow sensors is enormous for us with this whitening turned on. Might be hardware, of course.

LorenzMeier commented 7 years ago

@jgoppert Could you cross-test it?

lericson commented 7 years ago

You're right that AGC should probably not be on, empirically though it seems to be better. I'll look into it again tomorrow.

On 17 Jun 2017, at 20:00, James Goppert notifications@github.com wrote:

@jgoppert commented on this pull request.

In src/modules/flow/dcmi.c:

}
  • else if (dcmi_image_buffer_unused == 2)
  • {
  • for (uint16_t pixel = 0; pixel < image_size; pixel++)
  • (*current_image)[pixel] = (uint8_t)(dcmi_image_buffer_8bit_2[pixel]);
  • }
  • else
  • {
  • for (uint16_t pixel = 0; pixel < image_size; pixel++)
  • (*current_image)[pixel] = (uint8_t)(dcmi_image_buffer_8bit_3[pixel]);
  • for (uint16_t pixel = 0; pixel < image_size; pixel++)
  • (*current_image)[pixel] = source[pixel]; +}
  • +void whitened_image(uint8_t source, uint8_t dest, uint16_t image_size) { So I'm wondering how this compares to the auto gain and brightness controls within the firmware. Both pieces of software have the same goal, so you might just be eating more cpu here. You might want to check. But if your method is better than the default firmware, maybe you want to turn auto gain off.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread.

lericson commented 7 years ago

The noise is about the same with and without AGC, so I removed it as per your recommendation @jgoppert -- one less moving part.

LorenzMeier commented 7 years ago

Wait, wait. Did you test in sunlight and at very low indoor conditions? Because without AGC you might perform worse in one of the two. Please make sure to run a solid test.

lericson commented 7 years ago

I primarily tested indoors. Disabling AGC shouldn't affect the signal quality however, since the aim of the AGC mechanism is the same as the whitening operation. I'll try it with sunlight in a second.

lericson commented 7 years ago

Seems to work well in sunlight as well actually. How low light are low light conditions?

jlecoeur commented 7 years ago

Here is an example where the image quality drops (this is the floor of my office right now): scene

EDIT: on master, the quality for the same images was respectively 130, 190 and 150

lericson commented 7 years ago

Great feedback, @jlecoeur, thanks. Indeed it does have issues with high-variance areas; in particular multi-modalities such as the one you presented.

jgoppert commented 7 years ago

Yeah, agc likely controls how many photons it takes to make 1 bit, so it will have a different effect. In software with gain you are stretching the raw data out. That is why I never looked into a post camera image whitening. Do you have any intuitive sense why this is helping more than AGC and desired brightness?

jlecoeur commented 7 years ago

Indeed @lericson , see the histograms: histograms However computing the histograms and detecting multi-modes might be too expensive, can we afford calling compute_flow two times (before and after whitening), and keep the best one?

lericson commented 7 years ago

I think it won't be worth it, @jlecoeur -- what we could do though is some kind of naive outlier rejection. This would mean half the image becomes either completely black or completely white, which would in this scenario be more appropriate. Something like having two estimates, one for pixels over 127 (i.e. 50%) and one for those under. Let's try that.

lericson commented 7 years ago

@jgoppert The gain mechanism in the camera firmware is just a multiplier AFAICT.

jgoppert commented 7 years ago

For the point grey cameras it is photons to bits. So worth checking. I think you would still want your whitening though because the agc lags and would explain the flickering.

lericson commented 7 years ago

@jgoppert Hm ok, so you're saying AGC + whitening would make sense then?

lericson commented 7 years ago

@jlecoeur Tried binning, not very surprisingly it turns out to give rise to flickering as the mode changes between the bins. Three bins was pretty good though.

lericson commented 7 years ago

Could probably make it flicker less by introducing some penalty for switching mode.

jgoppert commented 7 years ago

@lericson Yes, I think AGC + whitening makes sense. Basically AGC is your first pass before data collection, you have to guess what the gain should be, then after you collect the image it computes the mean and variance and updates the gain for the next loop. Your whitening method would then operate on the output of that loop and fix transients.

jlecoeur commented 7 years ago

@lericson I fixed the problem with high variance areas on this branch https://github.com/PX4/Flow/compare/master...lis-epfl:master_KTH_image_whitening?expand=1

I added a mode WHITENING_AUTO where optical flow is computed on whitened images, then if the flow quality is lower than a threshold, flow is re-computed using non-whitened images. Now the quality does not fall to 0 like in my previous comment.

The performance cost goes increasing: IMAGE_WHITENING_DISABLED: no performance cost compared to master IMAGE_WHITENING_ALWAYS: fixed cost for whitening of current image IMAGE_WHITENING_AUTO: fixed cost for whitening of current image + variable cost for 2nd flow computation only when quality is poor.

Feel free to cherry-pick the 3 last commits if you like it.

lericson commented 7 years ago

Great stuff, @jlecoeur! Did you try it in practice? Is the added complexity and performance penalty worth the savings compared to just whitening all the time? I've been experimenting with this in a mixed light indoor environment (i.e. semi-occluded sunlight) and I've not found any significant performance hit.

In fact one of our bigger problems right now isn't the flow estimation -- it seems to work more or less flawlessly -- but making the LPE module of the PX4 firmware integrate the flow data into its estimation of the pose in a meaningful way. Right now it seems it trusts the accelerometer too much or something, and the velocity estimate just flies off the handle. Got any experience with this?

jlecoeur commented 7 years ago

@lericson I tried whitening in a building with sharp shadows on the ground. And indeed it was an issue, the drone was rejecting position control only when flying above shadow patterns. I assume this was the same phenomenon I witnessed in my office, but I have not tested the IMAGE_WHITENING_AUTO fix in that same building. I will do it probably on Monday.

Regarding the performance penalty, hand-held test showed that the 2nd flow computation happens only when there s a sharp brightness change in view. So in most cases this is not more expensive than whitening only.

Regarding estimators, I had the same trouble and finally settled with ekf2. As I understood it is not ready for full missions in GPS denied environments yet. But I take off and land in stabilized mode, so I do not care. I increased a bit flow innovation gate to prevent it from rejecting flow when it is not perfect. I also had to lower vibrations to around 1g~2g on x and y axes.

lericson commented 7 years ago

Too bad nothing is happening here! A demonstration of a hexacopter flying indoors using the code in this PR: https://youtu.be/mgxxTmgxnQ0

LorenzMeier commented 7 years ago

@jlecoeur @lericson Sorry for being slow on you. I'm very much impressed by your execution speed and contributions.

@ChristophTobler Could you help to get this in once you're back? Thanks!

ChristophTobler commented 7 years ago

@LorenzMeier Yes. I'll have a look at it as soon as possible.

ChristophTobler commented 7 years ago

@lericson I have bench-tested this PR and I'm not very convinced. When pointing to my relatively dark carpet the output looks like this: px4flow_whitening and reports valid flow but is not actually usable.

I got better results using https://github.com/PX4/Flow/pull/86 Same spot: px4flow_exposure_indoor and the flow output is actually usable. This image doesn't really represent the quality very good. I was able to see the texture.

@jgoppert FYI

jlecoeur commented 7 years ago

@ChristophTobler These are strange images, can you test with master and send an image ? A picture from smartphone camera may help too.

Your carpet seems to have very bright spots on dark background. So I wonder if you have the same problem as in the images I sent above.

lericson commented 7 years ago

@jlecoeur The grid in the second image is a test pattern. See the pertinent PR for a discussion on them.

@ChristophTobler Strange, I'm curious how your camera quality is on average if PR #86 works better than this does -- I heard some hardware batches are worse than others? The device is completely useless to us without the changes suggested in this pull request.

It looks like your carpet is dark enough and in a dark enough environment that all pixels are the same value, it would explain why the quality is 100% and it would explain the resulting image as the variance would be near or actually at zero. It doesn't seem worthwhile to set a minimum, because if the variance actually is that low, tracking flow will be impossible.

jlecoeur commented 7 years ago

@lericson ok that makes a lot more sense now :)

I am still curious about that first image @ChristophTobler what does it say with AGC enabled? I remember I reverted that change when testing on this branch

ChristophTobler commented 7 years ago

@jlecoeur Here's the image from master (before the merge): px4flow_master With default parameters quality was 0. But when lowering the flow feature threshold to 20, the quality went up to about 180 and more importantly, the flow values seemed to be correct. And this is how the carpet looks like: office_carpet

Here's how it looks with this PR + agc px4flow_white_agc Worked ok but the flow scale was a bit off. The weird thing was that now it didn't work well on other good textured surfaces. E.g. px4flow_white_agc_2

lericson commented 7 years ago

I think that looks really odd. It just looks like noise, no? We have a fairly well-lit testing area; is yours really dark?

lericson commented 7 years ago

Also I don't really understand the last image, the camera output looks fine, doesn't it?

ChristophTobler commented 7 years ago

@lericson

is yours really dark?

Not really. Maybe a little bit darker in real life than on the image above that was taken by my phone.

Also I don't really understand the last image, the camera output looks fine, doesn't it?

That's the odd thing, yes. The picture looks good but it doesn't detect any optical flow... Have a look at the Analyze plot in the lower left of the image. Everything is zero (which means no flow/low quality).

lericson commented 7 years ago

Well, you're not plotting the quality at all, so I can't speak for that, but having a zero flow when the camera is still would seem to make sense. (The gyro plots are also zero, leading me to believe you had the camera at perfect rest.)

mhkabir commented 7 years ago

The gyro plots are also zero, leading me to believe you had the camera at perfect rest.

You would still see some gyro noise. It wouldn't be absolutely zero :)

ChristophTobler commented 7 years ago

That's not how it works... Even when perfectly still, PX4Flow outputs some noise on both pixel flow and especially gyro (both have noise). When everything is perfectly zero, it means that flow quality is not good enough and it won't integrate values -> 0 flow quality. You can test it yourself by e.g. covering the lens

lericson commented 7 years ago

Agreed that there should be gyro noise, but during development of this whitening, we typically had zero flow when the camera was completely still.

ChristophTobler commented 7 years ago

Pixel flow is zero if you have very little image noise, true. But with this PR the image is flickering heavily. Especially on this carpet.

jlecoeur commented 7 years ago

@ChristophTobler I did not see flickering with this PR @mhkabir @ChristophTobler I agree there should be gyro noise

lericson commented 7 years ago

So, it seems like @jlecoeur and I both are unlucky enough to suffer from flickering WITHOUT this pull request. When I use this whitening method, the flickering goes away; in fact, that is exactly why I went for whitening.

jlecoeur commented 7 years ago

@lericson in fact flickering never stroke me as being a problem, with or without this PR. But for sure I saw an improvement above an almost feature-less linoleum floor. Improvement both in reported flow quality (see pictures above except the "border" one), and flight performance (it was often rejecting position control before this PR)

ChristophTobler commented 7 years ago

@jlecoeur Have you tried with latest master/this PR? It performs much better in low light situations.

jlecoeur commented 7 years ago

@ChristophTobler No, I compared with master before #86 was merged

jlecoeur commented 7 years ago

@lericson As #86 was merged, most comments in this discussion became irrelevant. I suggest to open a new PR with a rebase to master, re-enabled AGC, and reverted sonar dt. It'd be much easier for everyone to test the same thing.

lericson commented 7 years ago

Sounds like a good plan, @jlecoeur. I will try to find the time for it; I would need to test it on the hardware as well.