Improve flow quality #90

lericson · 2017-06-15T08:46:40Z

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.

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

Awesome, thanks! Would you mind doing the optimizations I suggested and re-test?

LorenzMeier · 2017-06-15T08:49:58Z

src/modules/flow/dcmi.c

+	double mean = sum / image_size;
+	double rss = 0.0;
+	for (uint16_t pixel = 0; pixel < image_size; pixel++)
+		rss += pow(source[pixel] - mean, 2);


This executes a double instruction and pow instead of square. Please replace with: (source[pixel] - mean) * (source[pixel] - mean). This will be orders of magnitude faster.

LorenzMeier · 2017-06-15T08:50:22Z

src/modules/flow/dcmi.c

+	double rss = 0.0;
+	for (uint16_t pixel = 0; pixel < image_size; pixel++)
+		rss += pow(source[pixel] - mean, 2);
+	double stddev = sqrt(rss/(image_size - 1));


Please use sqrtf() to keep this in float and use float variables.

LorenzMeier · 2017-06-15T08:51:14Z

src/modules/flow/dcmi.c

+}
+
+void whitened_image(uint8_t *source, uint8_t *dest, uint16_t image_size) {
+	double sum = 0.0;


double is very expensive on embedded hardware and you don't need this here (unless you need a resolution better than about 0.0000001). Please replace with float.

Good point, will do.

LorenzMeier · 2017-06-15T08:52:22Z

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 · 2017-06-15T09:59:49Z

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

lericson · 2017-06-15T10:03:03Z

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

…

On 15 Jun 2017, at 10:52, Lorenz Meier ***@***.***> 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 · 2017-06-15T10:05:17Z

Pushed the suggested changes!

mhkabir · 2017-06-15T10:09:19Z

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

lericson · 2017-06-15T11:14:27Z

~~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 · 2017-06-16T15:50:56Z

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.

jgoppert · 2017-06-17T18:00:46Z

src/modules/flow/dcmi.c

+		(*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 camera 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 whitening methods coded in the camera firmware, maybe you want to turn auto gain etc. off.

http://www.onsemi.com/pub/Collateral/MT9V034-D.PDF

LorenzMeier · 2017-06-18T10:38:51Z

@jgoppert Could you cross-test it?

lericson · 2017-06-18T10:56:30Z

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

…

On 17 Jun 2017, at 20:00, James Goppert ***@***.***> 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 · 2017-06-19T08:29:23Z

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

LorenzMeier · 2017-06-19T09:33:08Z

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 · 2017-06-19T11:56:40Z

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 · 2017-06-19T11:58:28Z

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

jlecoeur

Hi!
It greatly improved flow quality on low-textured floor.

However the flow quality drops to almost 0 when the camera sees bright areas that are created by sunlight through a window.

jlecoeur · 2017-06-19T15:56:23Z

src/modules/flow/main.c

@@ -108,10 +108,10 @@ volatile uint32_t boot_time10_us = 0;
 #define TIMER_LPOS		8
 #define MS_TIMER_COUNT		100 /* steps in 10 microseconds ticks */
 #define LED_TIMER_COUNT		500 /* steps in milliseconds ticks */
-#define SONAR_TIMER_COUNT 	100	/* steps in milliseconds ticks */
+#define SONAR_TIMER_COUNT	9999/* steps in milliseconds ticks */


Any particular reason you modified this? This gave very slow sonar update when I tested.

Whops, not intended to be included. I did this since we don't use this data at all, as the sonar is actually really bad.

Can you revert?

jlecoeur · 2017-06-19T16:15:25Z

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

above dark area: quality = 255
above bright area: quality = 255
above border: quality = 0

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

lericson · 2017-06-19T16:28:32Z

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

jgoppert · 2017-06-19T16:45:40Z

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 · 2017-06-19T16:49:37Z

Indeed @lericson , see the 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 · 2017-06-19T17:09:53Z

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.

jlecoeur · 2017-06-23T16:11:32Z

@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 · 2017-06-23T17:45:09Z

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 · 2017-06-23T19:03:50Z

@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 · 2017-08-09T21:04:28Z

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

LorenzMeier · 2017-08-09T21:08:05Z

@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 · 2017-08-14T07:31:08Z

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

ChristophTobler

I Still have to actually test it...

ChristophTobler · 2017-08-14T15:22:06Z

src/modules/flow/main.c

@@ -108,10 +108,10 @@ volatile uint32_t boot_time10_us = 0;
 #define TIMER_LPOS		8
 #define MS_TIMER_COUNT		100 /* steps in 10 microseconds ticks */
 #define LED_TIMER_COUNT		500 /* steps in milliseconds ticks */
-#define SONAR_TIMER_COUNT 	100	/* steps in milliseconds ticks */
+#define SONAR_TIMER_COUNT	9999/* steps in milliseconds ticks */


Can you revert?

ChristophTobler · 2017-08-14T15:23:08Z

src/modules/flow/mt9v034.c

@@ -134,7 +136,7 @@ void mt9v034_context_configuration(void)
 		desired_brightness = 58; // VALID RANGE: 8-64
 		resolution_ctrl = 0x0202;//10 bit linear
 		hdr_enabled = 0x0000; // off
-		aec_agc_enabled = 0x0303; // on
+		aec_agc_enabled = 0x0101; // aec on, agc off


I guess agc can stay enabled.?

ChristophTobler · 2017-08-14T15:23:23Z

src/modules/flow/mt9v034.c

 		resolution_ctrl = 0x0202;//10bit linear
 		hdr_enabled = 0x0000; // off
-		aec_agc_enabled = 0x0303; // on
+		aec_agc_enabled = 0x0101; // aec on, agc off


Same here. See above

ChristophTobler · 2017-08-15T15:06:26Z

@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:

and reports valid flow but is not actually usable.

I got better results using #86
Same spot:

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 · 2017-08-15T18:40:23Z

@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 · 2017-08-15T20:05:20Z

@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 · 2017-08-15T22:11:50Z

@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 · 2017-08-16T12:18:06Z

@jlecoeur Here's the image from master (before the merge):

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:

Here's how it looks with this PR + 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.

lericson · 2017-08-16T12:23:06Z

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 · 2017-08-16T12:23:54Z

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

ChristophTobler · 2017-08-16T12:37:18Z

@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 · 2017-08-16T12:40:29Z

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 · 2017-08-16T12:44:17Z

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 · 2017-08-16T12:44:19Z

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 · 2017-08-16T12:45:18Z

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 · 2017-08-16T12:52:23Z

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 · 2017-08-16T12:56:18Z

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

lericson · 2017-08-16T12:57:21Z

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 · 2017-08-16T13:49:37Z

@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 · 2017-08-16T13:52:33Z

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

jlecoeur · 2017-08-16T13:54:44Z

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

jlecoeur · 2017-08-16T14:03:24Z

@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 · 2017-08-16T14:07:46Z

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.

lericson · 2017-08-16T14:08:58Z

I should note that I'm a bit skeptical about using what was in #86 with this. I've tried it once before, and the results are available a bit further up in the comment history here -- I will try again, though.

ChristophTobler · 2017-08-16T14:11:36Z

@lericson

I should note that I'm a bit skeptical about using what was in #86 with this. I've tried it once before, and the results are available a bit further up in the comment history here -- I will try again, though.

Strange... #86 didn't have too many changes

Improve flow quality

6feb336

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 requested changes Jun 15, 2017

View reviewed changes

lericson added 2 commits June 15, 2017 13:15

Use floats for whitening

87bee29

Add 4.8.2 as allowed compiler

11a9616

lericson force-pushed the master branch from 291aeaa to 11a9616 Compare June 15, 2017 11:17

lericson mentioned this pull request Jun 16, 2017

Exposure for indoors #86

Merged

jgoppert reviewed Jun 17, 2017

View reviewed changes

Disable automatic gain control, set gain to 1

1263206

lericson added 3 commits June 19, 2017 13:49

Add whitening parameter

30eb3d5

Optimize types of whitening variables

f0771ee

Increase manual gain, desired brightness

dd01d8d

jlecoeur reviewed Jun 19, 2017

View reviewed changes

ChristophTobler suggested changes Aug 14, 2017

View reviewed changes

jlecoeur mentioned this pull request Aug 31, 2017

Image whitening #96

Closed

Improve flow quality #90

Are you sure you want to change the base?

Improve flow quality #90

Conversation

lericson commented Jun 15, 2017

LorenzMeier left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

LorenzMeier commented Jun 15, 2017

lericson commented Jun 15, 2017

lericson commented Jun 15, 2017 via email

lericson commented Jun 15, 2017

mhkabir commented Jun 15, 2017

lericson commented Jun 15, 2017 • edited Loading

lericson commented Jun 16, 2017

jgoppert Jun 17, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

LorenzMeier commented Jun 18, 2017

lericson commented Jun 18, 2017 via email

lericson commented Jun 19, 2017

LorenzMeier commented Jun 19, 2017

lericson commented Jun 19, 2017

lericson commented Jun 19, 2017

jlecoeur left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jlecoeur commented Jun 19, 2017 • edited Loading

lericson commented Jun 19, 2017

jgoppert commented Jun 19, 2017

jlecoeur commented Jun 19, 2017

lericson commented Jun 19, 2017

jlecoeur commented Jun 23, 2017

lericson commented Jun 23, 2017

jlecoeur commented Jun 23, 2017

lericson commented Aug 9, 2017

LorenzMeier commented Aug 9, 2017

ChristophTobler commented Aug 14, 2017

ChristophTobler left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ChristophTobler commented Aug 15, 2017 • edited Loading

jlecoeur commented Aug 15, 2017

lericson commented Aug 15, 2017

jlecoeur commented Aug 15, 2017

ChristophTobler commented Aug 16, 2017

lericson commented Aug 16, 2017

lericson commented Aug 16, 2017

ChristophTobler commented Aug 16, 2017

lericson commented Aug 16, 2017

mhkabir commented Aug 16, 2017

ChristophTobler commented Aug 16, 2017

lericson commented Aug 16, 2017

ChristophTobler commented Aug 16, 2017

jlecoeur commented Aug 16, 2017

lericson commented Aug 16, 2017

jlecoeur commented Aug 16, 2017 • edited Loading

ChristophTobler commented Aug 16, 2017

jlecoeur commented Aug 16, 2017 • edited Loading

jlecoeur commented Aug 16, 2017 • edited Loading

lericson commented Aug 16, 2017

lericson commented Aug 16, 2017

ChristophTobler commented Aug 16, 2017

lericson commented Jun 15, 2017 •

edited

Loading

jgoppert Jun 17, 2017 •

edited

Loading

jlecoeur commented Jun 19, 2017 •

edited

Loading

ChristophTobler commented Aug 15, 2017 •

edited

Loading

jlecoeur commented Aug 16, 2017 •

edited

Loading

jlecoeur commented Aug 16, 2017 •

edited

Loading

jlecoeur commented Aug 16, 2017 •

edited

Loading