Initial support for MMAL camera on raspberry pi

[jogu]

Originally by dozencrows on github; mostly this commit:

dozencrows/motion@6d1ed5b

Changes to merge back into motion upstream made by Joseph Heenan (with
help from lowflyerUK on github, see dozencrows/motion#1 ):

• changed back to using autoconf etc as per the main project
• submitted the one change we need to the raspicam files back upstream
  ( Add name to anonymous structs in RaspiCamControl.h raspberrypi/userland#332 )
• imported latest versions of files in raspicam, now exactly match upstream
• added README.txt to raspicam directory with brief explanation
• replaced some tabs with spaces
• removed dependency on checkout of raspberrypi userland repo; now we
  use the headers provided by libraspberrypi-dev
• merged in a couple of the trivial later bugfix commits
• merged in mmalcam_control_params commit to allow config of camera options
• fixed many merge conflicts rebased on top of latest upstream motion

[jogu]

So this is a very first attempt at submitting some of the code from the motion-mmal fork. This is only the very first commit which adds initial support; there are many other commits in the fork, many of which make large scale optimisations to some of the core of motion that I plan to look at separately later.

I've tested this on a raspberry pi 3 with the v1 picam and it seems to work well - just compile (as per normal but apt-get install libraspberrypi-dev) and then run: ./motion -c configs/motion-mmalcam.conf

The changes to the core of motion in this commit are minimal, so it poses very little risk of breaking anything.

[tosiara]

Since this is quite a major feature (more than 3k lines added), why don't we create an "official" branch and have it for more testing? So when the branch is stable enough (and doc updated) we could merge it into master

[jogu]

There's very little risk of regressions as 100% of the code is all protected by the HAVE_MMAL setting, and not built when that option is not set, so I don't see a strong reason to have a branch - I'd be keener to merge it straight to master.

I'll sort out the docs though; what's missing - I need to make updates to the wiki version of the motion guide for the two new config options I guess, anything else?

[tosiara]

Ok, fair enough. If it is well surrounded by ifdefs - then it is better to checkin into master to avoid later marge conflicts

[tosiara]

This config file should probably be processed (generated) as it is done with motion-dist.conf.in during configure

[tosiara]

Also, this commit creates two new directories: configs and raspicam. Do we really need directory structure in this case?

[tosiara]

Another suggestion is for file naming to be similar to existing, ex, not RaspiCamControl.c, but maybe video_raspicam.c?

[jogu]

"Also, this commit creates two new directories: configs and raspicam. Do we really need directory structure in this case?"

I think config should go and I'll sort out the new config file to go through configure as per the others, thanks for spotting that.

raspicam I'm keen to leave as it is - it's a clear directory that contains source we use but don't change (other than possibly taking in new versions from upstream); it has absolutely zero. (TBH this isn't 100% great, but upstream don't expose these files as a library, and adding it as a git submodule would be painful.) I added a README.txt in that directory (I hope to explain).

[Mr-DaveDev]

This looks great at getting the fork down to the minimum necessary to get the functionality. Couple of comments. I agree on eliminating the new config file. It only seems to reference two new options for mmal and has some old options in there. Next, the motion.c also adds profiling. I've not used this previously. Are we sure we'll be using this? Should we be waiting for a separate commit on that? Finally on the raspicam files, I don't want to do any auto-update from git and would prefer to stick with something that we know works. I don't want to be answering questions of why something doesn't work/compile only to track it back to upstream changes. We can periodically look/test whether it needs to be updated until a library becomes available that exposes what we need.

[lowflyerUK]

Surely the main risk of it not building/compiling/working on a Raspberry Pi is if the mmal libraries (in /opt/vc) are updated in a way that breaks all our mmal calls - not just those in RaspiCamControl and RaspiCLI? Maybe we sould do some version checking on mmal and issue a warning in our configure script?

[jogu]

"Surely the main risk of it not building/compiling/working on a Raspberry Pi is if the mmal libraries (in /opt/vc) are updated in a way that breaks all our mmal calls"

/opt/vc is a published public API, so should very rarely, if ever, change in an incompatible way I believe.

We could add a compile time test on MMAL_VERSION_MAJOR define I guess. Not sure it's important :)

[jogu]

Thanks, MrDave.

I'll pull out the profiling stuff into a separate commit to look at later.

I'm not 100% sure on the config file thing; it would be nice to distribute a "just works" config file that raspberry pi users can use without further edits just to get the basic functionality working. If anyone knows the best way to achieve that let me know :) Possibly I can make so that configure generates such a file from motion-dist.conf.in we HAVE_MMAL is set? Not sure.

[lowflyerUK]

@jogu For what it's worth, there weren't any problems with mmal-motion caused by updates to /opt/vc or userland, even with the new version camera. So historically, any changes have not made it incompatible.

[Mr-Dave]

The practice for the config has been to put in all the possible parameters. For the mmal, we would put the two lines in as commented out. SDL works like this. The configuration option is always there even when compiled and installed without SDL We would however want to include defaults for the options if possible so that users could get something working simply by removing the ";"

[Mr-Dave]

One other ask on this. Has anyone run a valgrind on this to make sure we don't have any leaks?

[lowflyerUK]

@Mr-Dave Not a valgrind, but I have run my version of this for over 6 months on 4 different machines with no problems. Even with only 380MBytes available RAM (according to top).

[jogu]

Okay, I believe I've addressed all the comments on this now.

I've removed the new config file and the configs directory, and added the new options commented out to motion-dist.conf.in.

I tried running valgrind, but it dies with "disInstr(arm): unhandled instruction: 0xF1010200" - I'm guessing it doesn't understand something in the armv8 instruction set.

Also, I spotted that there was a missing # in the second line of the new config descriptions, so fixed that.

I think this should be ready to merge now.

[Mr-Dave]

I purchased a camera last week and when I pulled the code then, there was a problem with the make complaining /opt/vc/include/interface/vcos/vcos_types.h:38:33: fatal error: vcos_platform_types.h: No such file or directory
I will pull your latest code onto a fresh image today and see if they persist.

[jogu]

Awesome that you have a camera now!

The error you saw is odd. vcos_platform_types.h should come from libraspberrypi-dev... but so does vcos_types.h. It's in /opt/vc/include/interface/vcos/pthreads/vcos_platform_types.h on mine. I just tried rebuilding this all from scratch on my pi3 and it was okay - I'm running raspbian jessie, gcc 4.9.2-10. Could be some include path problem but unsure what might be different on your setup.

[Mr-Dave]

It all works once I did a dist-upgrade