RFS: self-serve photo booth software

[Adam Roses Wight]

Dear comrades,

https://github.com/adamwight/booths

I've written a photo booth application based on OpenCV, which uses motion
detection to trigger a 4-up sequence of snapshots. The UI is pretty nice,
and doesn't require any input devices aside from the camera. The code and
configuration are robust, and simple to extend if desired.

A couple people have contacted me about running the software, but compiling
an OpenCV app is not for the faint of heart, so I recently produced a few
.deb packages for various platforms:

https://github.com/adamwight/booths/releases

I'd like help refining this package, to reach a wider audience. It seems
to be the simplest entry in this niche.

Open issues:
* I'm leaning towards GPL-v3.0, any reason to reconsider this?
* Need to finish i18n of the interface text.
* Sound effects are pulled in by the makefile, and belong to http://www.trekcore.com/audio/ -- I don't know if this is fair use or not.
* Debian metadata and documentation are crude, please suggest improvements.

Thank you in advance!
Adam

[Paul Wise]

Post by Adam Roses Wight
* I'm leaning towards GPL-v3.0, any reason to reconsider this?

For this kind of software, GPLv3 seems ideal.

Post by Adam Roses Wight
* Need to finish i18n of the interface text.

i18n would definitely be a good idea, I'd suggest using gettext.

Post by Adam Roses Wight
* Sound effects are pulled in by the makefile, and belong to http://www.trekcore.com/audio/ -- I don't know if this is fair use or not.

For Debian the build process cannot access the network. For Debian we
cannot rely on fair use, we need the copyright holder to give the
audio a DFSG-free license like CC-BY-SA-3.0 or similar.

Post by Adam Roses Wight
* Debian metadata and documentation are crude, please suggest improvements.

As you are upstream, you may want to read this page:

https://wiki.debian.org/UpstreamGuide

You are missing a debian/watch file.

https://wiki.debian.org/debian/watch

The debian/copyright file is completely empty.

None of the files appear to have copyright/license information:

http://lu.is/blog/2012/03/17/on-the-importance-of-per-file-license-information/

You might want to add a Vcs-Browser field to the debian/control file.

The Build-Depends field in debian/control is missing cmake/opencv/etc.
Please try to build the package using pbuilder/cowbuilder/sbuild.

https://www.debian.org/doc/manuals/maint-guide/build.en.html#pbuilder

The maintainer address in debian/control is different to your address
and looks like it might not even work?

Standards-Version in debian/control is old, please check the upgrading
checklist:

https://www.debian.org/doc/debian-policy/upgrading-checklist

The ITP number in debian/changelog does not look correct, these days
it should be at least 6 digits.

http://www.debian.org/devel/wnpp/

The version in debian/changelog should be 1.0.0~rc2-1 as that is the
latest upstream tag (converted to Debian format).

You might want to add a debian/upstream/metadata file:

https://wiki.debian.org/UpstreamMetadata

You might want to add --parallel to the dh call in debian/rules.

Automated checks:

https://anonscm.debian.org/cgit/collab-maint/check-all-the-things.git

$ cme check dpkg
...
Warning in 'control source Standards-Version' value '3.9.4': Current
standards version is 3.9.6
Warning in 'control binary:booths Description' value ' Flail your
arms until the motion detector is activated, then beam your party into
the future!': Line too long in description
2015/04/19 16:25:58 No section found
...

$ cppcheck -j1 --quiet -f . | grep -vF 'cppcheck: error: could not
find or open any of the paths given.'
[sound.hpp:40]: (error) Mismatching allocation and deallocation: buf
[booths.cc:61]: (error) Instance of 'Sound' object is destroyed immediately.
[booths.cc:75]: (error) Instance of 'Sound' object is destroyed immediately.
[booths.cc:143]: (error) Instance of 'Sound' object is destroyed immediately.

$ find -empty
./debian/copyright

$ flawfinder -Q -c .
<various things, some may be important>

$ find -type f \( -iname '*.c' -o -iname '*.cc' -o -iname '*.cxx' -o
-iname '*.cpp' -o -iname '*.h' -o -iname '*.hh' -o -iname '*.hxx' -o
-iname '*.hpp' \) -exec include-what-you-use {} \;
<various things, some may be helpful in reducing build time>

$ grep -riE 'fixme|todo|hack|xxx' .
<some things to fix>
--
bye,
pabs

https://wiki.debian.org/PaulWise

[Adam Roses Wight]

Hi pabs,

Thanks for the thorough first review! I'll work on these suggestions
and respond here with the next release candidate.

-Adam

[Adam Roses Wight]

Post by Paul Wise
i18n would definitely be a good idea, I'd suggest using gettext.

I slogged through the gettext support, noticing too late that QT comes with
its own i18n library. If my ad-hoc gettext integration eventually rabbitholes,
I might try that instead:

http://www.cmake.org/Wiki/CMake:How_To_Build_Qt4_Software#Translations

Post by Paul Wise
http://lu.is/blog/2012/03/17/on-the-importance-of-per-file-license-information/

I think it's unlikely that any file would be taken out of this project in
isolation, so I'll assume the top-level LICENSE is obvious enough for now.

Post by Paul Wise
$ cppcheck -j1 --quiet -f . | grep -vF 'cppcheck: error: could not
find or open any of the paths given.'
[sound.hpp:40]: (error) Mismatching allocation and deallocation: buf

That was a real memory leak, fantastic!

I think I worked through all your other points, looking forward to the
next iteration.

Thanks,
Adam

[Adam Roses Wight]

I see it's not recommended to include a debian/ directory in my
upstream source control [1]. What's the best practice for hosting Debian
packaging files? Should I request a new repo on Alioth, fork my
upstream, and add the debian dir on a branch? [2, 3]

Also, is it standard procedure to upload locally built packages to mentors.d.n,
or is that not necessary if I already have hosting for the files? [4]

Thanks,
Adam

[1] https://wiki.debian.org/DebianMentorsFaq#What.27s_wrong_with_upstream_shipping_a_debian.2F_directory.3F
[2] http://anonscm.debian.org/cgit/debian-science/packages/opencv.git/tree/
[3] https://alioth.debian.org/register/
[4] https://github.com/adamwight/photo-booth/releases

Post by Adam Roses Wight

Post by Paul Wise
i18n would definitely be a good idea, I'd suggest using gettext.

I slogged through the gettext support, noticing too late that QT comes with
its own i18n library. If my ad-hoc gettext integration eventually rabbitholes,
http://www.cmake.org/Wiki/CMake:How_To_Build_Qt4_Software#Translations

Post by Paul Wise
http://lu.is/blog/2012/03/17/on-the-importance-of-per-file-license-information/

I think it's unlikely that any file would be taken out of this project in
isolation, so I'll assume the top-level LICENSE is obvious enough for now.

Post by Paul Wise
$ cppcheck -j1 --quiet -f . | grep -vF 'cppcheck: error: could not
find or open any of the paths given.'
[sound.hpp:40]: (error) Mismatching allocation and deallocation: buf

That was a real memory leak, fantastic!
I think I worked through all your other points, looking forward to the
next iteration.
Thanks,
Adam
--

[Paul Wise]

Post by Adam Roses Wight
I see it's not recommended to include a debian/ directory in my
upstream source control [1]. What's the best practice for hosting Debian
packaging files? Should I request a new repo on Alioth, fork my
upstream, and add the debian dir on a branch? [2, 3]

That is essentially up to you as the package maintainer, everyone has
different opinions. I personally prefer either no VCS for debian/
where I am the sole maintainer. For team-maintained packages, I like a
repository containing only the contents of debian/.

Post by Adam Roses Wight
Also, is it standard procedure to upload locally built packages to mentors.d.n,
or is that not necessary if I already have hosting for the files? [4]

mentors does not distribute binary packages, which is what you have
uploaded to github. Some sponsors are fine interacting with random git
trees but others like to use the Debian source package (.dsc), which
you haven't uploaded anywhere. Personally I'd suggest uploading the
source package to mentors.
--
bye,
pabs

https://wiki.debian.org/PaulWise

[Adam Roses Wight]

A few more small questions, otherwise the photo-booth package is ready for
another review. [1] Thanks for all the help so far!

* The package name has changed since I filed the ITP bug. How can I update
the bug with a new package name, should I just reply with a human-readable
note to that effect?

* I accidentally ran "dput" without a host argument, which pushed my package
files to the production upload box. I've sent some commands to dcut rm,
hopefully that works correctly. It looks like the commands were consumed with
no effect on the photo-booth* files.

* Not sure why the copyright file is not linting. In the documentation, I
see that repeated licenses only need to be specified once. [2] There are
three files with the same license, and I only give the standalone paragraph
for the third. The second file causes a lint warning! Possibly a bug in
the lintian job?

-Adam

[1] http://mentors.debian.net/package/photo-booth
[2] https://www.debian.org/doc/packaging-manuals/copyright-format/1.0/#stand-alone-license-paragraph

Post by Paul Wise

Post by Adam Roses Wight
Also, is it standard procedure to upload locally built packages to mentors.d.n,
or is that not necessary if I already have hosting for the files? [4]

mentors does not distribute binary packages, which is what you have
uploaded to github. Some sponsors are fine interacting with random git
trees but others like to use the Debian source package (.dsc), which
you haven't uploaded anywhere. Personally I'd suggest uploading the
source package to mentors.
--
bye,
pabs
https://wiki.debian.org/PaulWise
--

[Adam Roses Wight]

Post by Adam Roses Wight
* Not sure why the copyright file is not linting. In the documentation, I
see that repeated licenses only need to be specified once. [2] There are
three files with the same license, and I only give the standalone paragraph
for the third. The second file causes a lint warning! Possibly a bug in
the lintian job?
[2] https://www.debian.org/doc/packaging-manuals/copyright-format/1.0/#stand-alone-license-paragraph

Solved by reading the lintian source. To reuse a license, it must be declared
as a License: short-name and full text, but with no Files. I think the docs
at [2] should be updated to mention this fact.

-a

[Paul Wise]

[I'm subscribed, no need to CC]

Post by Adam Roses Wight
I slogged through the gettext support, noticing too late that QT comes with
its own i18n library. If my ad-hoc gettext integration eventually rabbitholes,
http://www.cmake.org/Wiki/CMake:How_To_Build_Qt4_Software#Translations

I would guess that using the native Qt i18n would be more appropriate
than gettext.
--
bye,
pabs

https://wiki.debian.org/PaulWise