-
Notifications
You must be signed in to change notification settings - Fork 36
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Mmal test #1
base: mmal-test
Are you sure you want to change the base?
Mmal test #1
Conversation
Added option to output both secondary and normal pictures.
Thank you, lowflyerUK!! As you note in this very helpful discussion (with a link also to the binary package you've built): it seems that this pull request should resolve the problems that I've see discussed all over when folks try to use motion on debian jessie or the latest raspbian, i.e. errors like:
e.g. at:
I spent hours wading thru those pages before finding your easy, open-source solution - thanks! For more on what this "MMAL" stuff is all about, this seems useful: And wouldn't it be nice if either the upstream motion package accepted these patches as requested over and over at Feature Request: Raspberry camera and infra support (FeatureRequest2013x12x04x172756 < Motion < Foswiki), or if someone just supported Video4Linux for MMAL in general? |
Hi @dozencrows (and @lowflyerUK ) There's an effort going on to re-vitalise motion with the help of the current maintainer, to try and get an official new one that has all necessary fixes etc in it: https://github.com/Motion-Project/motion What would you think about merging the mmal support into the main repo? Either doing this yourself or one of the project members doing it. (Apologies for commenting here; I know this isn't an appropriate place but I can't find a better way to get in touch, if anyone has an email address or other way to contact dozencrows feel free to let me know, or to pass him my email - it's in my github profile.) Thanks Joseph |
Hi Joseph, Yes, I should be delighted if we could merge it all. I am not sure that I am clever enough to do it myself, but I would very keen to help an expert. I tried to contact @dozencrows but without success :( Good luck!! |
It's great to hear that motion is being revitalised! And it would also be great to get good MMAL support for the Pi camera in there. However I think it's not a trivial job; my mmal-test branch was an experiment and I suspect won't merge easily. This is because of a number of reasons:
It might be more productive to use my branch as inspiration for re-implementing MMAL support in the current Motion main-line. Unfortunately I have no time that I can give to motion for the forseeable future, but may be able to answer occasional questions - it has been quite a long time since I looked at the code. |
Howdy - thanks for the insights and all your work, @dozencrows. @lowflyerUK, would that let people at least re-create what you've built at https://www.dropbox.com/s/6ruqgv1h65zufr6/motion-mmal-lowflyerUK-20151114.tar.gz? That one works for me on a raspberry pi 3. And how is that related to https://www.dropbox.com/s/jw5r1wss32tdibb/motion-mmal-opt.tar.gz which is from May of 2014, but which is linked to from https://github.com/lowflyerUK/motion/tree/mmal-test? |
@nealmcb, yes, I think that this pull request should make dozencrows:mmal-test the same as lowflyerUK:mmal-test - i.e. there were no commits to dozencrows:mmal-test after I cloned it. So, also yes, that should compile to the same as https://www.dropbox.com/s/6ruqgv1h65zufr6/motion-mmal-lowflyerUK-20151114.tar.gz. I think that the older https://www.dropbox.com/s/jw5r1wss32tdibb/motion-mmal-opt.tar.gz was compiled from dozencrows:mmal-test, so does not have any of my hacks. It is referred to in the "Original MMAL README". Actually isn't it a bad idea to have pre-compiled versions that will become out of date? Either we need proper version control over the compiled versions or just make it so easy to compile that everybody can and does? |
Um, just how different are the starting points of @dozencrows fork and https://github.com/Motion-Project/motion? Is there a summary of the evolution anywhere? I would be keen to help re-implementing MMAL support as suggested very sensibly by @dozencrows |
I think there's maybe 2 years development in the project repo since the fork. The major changes are summarised on https://github.com/Motion-Project/motion/releases/tag/release-3.4.1 I had a go at rebasing @dozencrows's work on top of the latest code and unless there's something terrible I've not yet discovered it may not be as bad as was feared: https://github.com/jogu/motion/tree/merge-mmal Note: I literally just got that tree to compile. If it compiles for anyone else, I'd be surprised. If it did anything other than crash straight away on startup I'd be shocked. It uses the standard motion compile method (autoreconf -fiv && ./configure && make). Even if that works there's still a long way to go merging it in, making sure there's no adverse affects on other features etc. I've not really looked at the changes in this pull request; I'm kind of presuming they're already present in the new repo so aren't necessary. |
@dozencrows btw, any preference on how things are attributed? Normally I would leave the git author on the commits as yourself, but add comments to the commit messages to say that I'd made changes and it's likely my fault if it's broken etc. Some people prefer not to have their names on commits other people have fiddled with though. |
@jogu Yes, most of my changes should be covered, except a useful trick to save both the high resolution and low resolution stills. It is useful to give a small thumbnail for a movie as well as having access to the full resolution still if needed. I will have a look at your merge-mmal. How exciting!! |
"except a useful trick to save both the high resolution and low resolution stills". I may not have spotted that commit, apologies. If it's not pi specific I'd encourage you to put it in a pull request against https://github.com/Motion-Project/motion |
Um, it is mmal specific. Does that mean it is Pi specific? |
I would say so, yep. :) That may be something we need to come back to later then, if I can find a way to get what we have already merged. |
The best way would be to fork the latest motion project, reimplement mmal support there and then push as a separate branch so it is available in the new official repo and can be enhanced from there |
So I managed to get https://github.com/jogu/motion/tree/merge-mmal to compile on a Raspberry Pi and run mmal :) I ran autoreconf -fiv then ./configure then hacked the Makefile to include all the mmal stuff.
Now I need to try to get autoconf to do it. |
Oh, awesome! I'd forgotten to set HAVE_MMAL or whatever it is when testing my builds so forget to add the mmal files to Makefile.in :) |
@jogu I have modified Makefile.in and configure.ac to check for mmal on the Raspberry Pi and make it if present. And I have updated the local copies of the mmal stuff to the latest versions. However I can't seem to fork your https://github.com/jogu/motion/tree/merge-mmal as I have already forked some previous fork of motion. So I can't issue a pull request :( Any suggestions? Thanks!! |
@lowflyerUK nice! If you just create a new branch in your existing repo with the changes (if you cloned my repo, you might need to add your original one as a new remote) you should be able to create a pull request from there. Even if you can't create the pull request then at least I should be able to see the commits and manually pull them across :) |
@jogu thanks for your help! Um, I am not very good with github. I hope I did it right! Have a look at https://github.com/lowflyerUK/motion.git branch merge-mmal-1 |
Great! I've pulled your two commits across into my branch, and also rebased my branch on top of the latest upstream (so you may find you need to use git reset --hard rather than a merge). Does this mean you've tested my merge and it actually works okay?! I don't have a /home/$USER/userland that the configure now tests for - are there instructions somewhere in here for how to get that? |
I will have a go later at re-merging or whatever... Yes, it configures, makes, compiles and runs on my Pi with that config file. Of course there are lots of other combinations that I haven't started to test yet. I will write a BUILDME-MMAL-ON-RPI. To get the userland stuff you just do Hope this helps! |
Also I will tidy up a bit - there are some files that are now not needed and will confuse. |
See https://github.com/lowflyerUK/motion.git branch merge-mmal-2. |
Sorry clicked the wrong thing. |
Thanks, @lowflyerUK! Excellent instructions! I got this to build okay on my pi. I'm going to try cleaning this up a bit so we use unmodified source from userland and use the headers from the libraspberrypi-dev dpkg package, and (as a first step) try and get at least the very first commit that adds basic support for the mmal camera merged into the main motion project repo. [I think the rest of the code will need more testing on non-pi platforms and possibly more documentation before we could look at merging that in, hence the reason for starting with just that first commit that should have very little impact on the other platforms.] |
@jogu Good plan! Although if by "the rest of the code" you mean my addition of the option to give both hisg and low resolution pictures, it is only a few lines and entirely within mmal if statements, so only ever applicable to Raspberry Pi (as per our discussion earlier in this thread). |
@lowflyerUK I have installed mmal-2 on my daytime Pi A+ I haven't managed to get your dropbox conf (as amended) to accept either on_picture_save or on_event_end to ftp to my WDMyCloud NAS box, the command I used works manually: Paul |
@pmtate Hi! I don't think that your question is mmal specific so you could look at the very good motion guide and forum at http://www.lavrsen.dk/foswiki/bin/view/Motion/WebHome. Have you tried putting your script in an executable file that is called by on_picture_save? |
@lowflyerUK I'm thinking way way simpler for the first merge into upstream; literally the very first commit on dozencrow's repo that adds just the video support - here's how it is looking: I've managed to clean things up so that a userland git checkout isn't required, and the RaspiCamControl.c now exactly match what is in userland git - no changes at all in our versions. Need to do a bit of testing now. The later commits in dozencrow's repo have a lot of changes to the core of motion that need much more checking and consideration (and likely documentation too) before we can look to merge them into the main project. |
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 ( 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
First merge pull request to the upstream motion is now live at: Motion-Project/motion#148 - let's see how this goes :-) |
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 ( 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
Hi @jogu! Great start with Motion-Project#148. |
Hi @lowflyerUK I'm not going to have much time to do anything for a while to be honest. I just did a quick update of my merge so that it's based on top of the latest work, and it seems to compile okay: https://github.com/jogu/motion/tree/mmal-merge The next step would be to pick the next part to merge. I'm not sure if it is all still necessary (I think the stills mode was originally added because the picam had limited resolutions that support video, but I understand that's not the case anymore - so is the stills mode still needed?). Once the next logic part to be merged has been picked, a git commit with just that part in it would need to be prepared, (and a bit more might need to be added to the commit message to explain the reasoning behind the change), then tested with the picam - if it works there it can then be tested with netcams and the other types motion supports to, then it should be ready to go into main repo. Or something like that :) Unfortunately some unrelated parts are mixed together in the same commit just now (eg. stills mode + general pi optimisations are together) which means it needs a bit of unpicking to generate logical parts to test+merge. |
Following dozencrows' suggestion, here is my very first pull request. Please let me know if I need to do anything else.