From: Michael Olbrich <m.olbrich@pengutronix.de>
To: ptxdist@pengutronix.de
Subject: Re: [ptxdist] [PATCH] New package: imagemagick
Date: Thu, 15 Sep 2016 09:56:25 +0200 [thread overview]
Message-ID: <20160915075625.toop5x6s4k7qnsql@pengutronix.de> (raw)
In-Reply-To: <CABDcavaJg4rtQw1FzOXEq4__6M2aSqQUjP8hNQWjQJo7b9Pryw@mail.gmail.com>
Hi,
On Thu, Sep 15, 2016 at 08:34:45AM +0200, Guillermo Rodriguez Garcia wrote:
> Thank you for reviewing. Some comments below:
>
> 2016-09-14 23:32 GMT+02:00 Roland Hieber <rohieb@rohieb.name>:
> > Hey hey,
> >
> > On 14.09.2016 08:31, Guillermo Rodriguez Garcia wrote:
> >> Hello, any feedback on this one?
> >>
> >> Guillermo
> >>
> >> 2016-09-02 12:14 GMT+02:00 Guillermo Rodriguez <guille.rodriguez@gmail.com>:
> >>> This adds a new package for ImageMagick 7. Some configuration
> >>> options are set to sensible defaults for embedded targets (quantum
> >>> depth set to 8 pixels, HDRI disabled). See notes in imagemagick.make.
> >>>
> >>> Signed-off-by: Guillermo Rodriguez <guille.rodriguez@gmail.com>
> >>>
> >>> ---
> >>> rules/imagemagick.in | 41 ++++++++++++++
> >>> rules/imagemagick.make | 142 ++++++++++++++++++++++++++++++++++++++++++++++++
> >>> 2 files changed, 183 insertions(+)
> >>> create mode 100644 rules/imagemagick.in
> >>> create mode 100644 rules/imagemagick.make
> >>>
> >>> diff --git a/rules/imagemagick.in b/rules/imagemagick.in
> >>> new file mode 100644
> >>> index 0000000..90a0eb6
> >>> --- /dev/null
> >>> +++ b/rules/imagemagick.in
> >>> @@ -0,0 +1,41 @@
> >>> +## SECTION=multimedia_tools
> >>> +
> >>> +menuconfig IMAGEMAGICK
> >>> + tristate
> >>> + select ZLIB if IMAGEMAGICK_USE_ZLIB
> >>> + select LIBPNG if IMAGEMAGICK_USE_LIBPNG
> >>> + select LIBJPEG if IMAGEMAGICK_USE_LIBJPEG
> >
> > I guess these should be tab-aligned before "if", at least I got that as
> > feedback for my patch :)
>
> This one is easy :)
>
> >
> >>> + prompt "imagemagick "
> >>> + help
> >>> + ImageMagick® is a software suite to create, edit, compose, or convert
> >>> + bitmap images. It can read and write images in a variety of formats
> >>> + (over 200) including PNG, JPEG, JPEG-2000, GIF, TIFF, DPX, EXR, WebP,
> >>> + Postscript, PDF, and SVG. Use ImageMagick to resize, flip, mirror,
> >>> + rotate, distort, shear and transform images, adjust image colors, apply
> >>> + various special effects, or draw text, lines, polygons, ellipses and
> >>> + Bézier curves.
> >>> +
> >>> +if IMAGEMAGICK
> >>> +
> >>> +config IMAGEMAGICK_USE_ZLIB
> >>> + bool
> >>> + default y
> >>> + prompt "Enable ZLIB"
> >>> + help
> >>> + Enable ZLIB support
> >>> +
> >>> +config IMAGEMAGICK_USE_LIBPNG
> >>> + bool
> >>> + default y
> >>> + prompt "Enable PNG"
> >>> + help
> >>> + Enable PNG support
> >>> +
> >>> +config IMAGEMAGICK_USE_LIBJPEG
> >>> + bool
> >>> + default y
> >>> + prompt "Enable JPEG"
> >>> + help
> >>> + Enable JPEG support
> >>> +
> >>> +endif
> >>> diff --git a/rules/imagemagick.make b/rules/imagemagick.make
> >>> new file mode 100644
> >>> index 0000000..f07bab7
> >>> --- /dev/null
> >>> +++ b/rules/imagemagick.make
> >>> @@ -0,0 +1,142 @@
> >>> +# -*-makefile-*-
> >>> +#
> >>> +# Copyright (C) 2016 by Guillermo Rodriguez <guille.rodriguez@gmail.com>
> >>> +#
> >>> +# See CREDITS for details about who has contributed to this project.
> >>> +#
> >>> +# For further information about the PTXdist project and license conditions
> >>> +# see the README file.
> >>> +#
> >>> +
> >>> +#
> >>> +# We provide this package
> >>> +#
> >>> +PACKAGES-$(PTXCONF_IMAGEMAGICK) += imagemagick
> >>> +
> >>> +#
> >>> +# Paths and names
> >>> +#
> >>> +IMAGEMAGICK_VERSION := 7.0.2-10
> >>> +IMAGEMAGICK_MD5 := e1cb23d9c10a8eff228ef30ee281711a
> >>> +IMAGEMAGICK := ImageMagick-$(IMAGEMAGICK_VERSION)
> >>> +IMAGEMAGICK_SUFFIX := tar.xz
> >>> +IMAGEMAGICK_URL := ftp://ftp.nluug.nl/pub/ImageMagick/$(IMAGEMAGICK).$(IMAGEMAGICK_SUFFIX)
> >>> +IMAGEMAGICK_SOURCE := $(SRCDIR)/$(IMAGEMAGICK).$(IMAGEMAGICK_SUFFIX)
> >>> +IMAGEMAGICK_DIR := $(BUILDDIR)/$(IMAGEMAGICK)
> >>> +IMAGEMAGICK_LICENSE := Apache-2.0
> >>> +
> >>> +# ----------------------------------------------------------------------------
> >>> +# Prepare
> >>> +# ----------------------------------------------------------------------------
> >>> +
> >>> +IMAGEMAGICK_QUANTUM_DEPTH := 8
> >
> > If this is an option which is changed often, make it a text field in the
> > .in file.
>
> As far as I can tell there is no point in changing this currently.
> Allowed values are 8, 16, and 32. Most image formats don't support
> more than 8 bits per pixel quantum. Since I only enabled jpeg and png
> for now, changing the quantum depth would have no actual benefit, and
> would certainly have an impact in performance and RAM consumption.
> According to the ImageMagick site: "For example, using sixteen-bit
> pixel quantums can cause ImageMagick to run 15% to 50% slower (and
> take twice as much memory) than when it is built to support eight-bit
> pixel quantums".
>
> That's why I currently hardcode this to 8, instead of making it
> configurable. If/when more image formats are enabled, it may make
> sense to have this as an option in the .in file, but right now there
> is no point in that.
Add a short comment about this above the definition. No kconfig option for
now is ok.
> >
> >>> +
> >>> +IMAGEMAGICK_PATH := PATH=$(CROSS_PATH)
> >>> +IMAGEMAGICK_ENV := $(CROSS_ENV)
> >>> +
> >>> +#
> >>> +# See: http://www.imagemagick.org/script/advanced-unix-installation.php
> >>> +#
> >>> +# Notes:
> >>> +# - Threading is disabled as it brings in a dependency with libgomp.so
> >>> +# (OpenMP) which fails at runtime; disabling openmp itself doesn't seem
> >>> +# to be enough.
> >>> +# - Quantum depth is set to 8. Most display adapters and image formats
> >>> +# don't support more than 8 bits per pixel quantum (i.e. per each of the
> >>> +# R, G, B, and alpha components), and higher values have an impact in
> >>> +# runtime performance.
> >>> +# - HDRI is disabled. It is not supported by most image formats, and has
> >>> +# a severe impact in runtime performance.
> >>> +# - The configure script will try to detect external "helper" programs
> >>> +# available in the host and store their paths in delegates.xml. These
> >>> +# are obviously not applicable on the target. Just ignore the generated
> >>> +# delegates.xml file.
> >>> +#
> >>> +IMAGEMAGICK_AUTOCONF := \
> >>> + $(CROSS_AUTOCONF_USR) \
> >>> + --disable-docs \
> >>> + --enable-shared \
> >>> + --disable-static \
> >>> + --disable-openmp \
> >>> + --without-threads \
> >>> + --without-modules \
> >>> + --with-quantum-depth=$(IMAGEMAGICK_QUANTUM_DEPTH) \
> >>> + --disable-hdri \
> >>> + --without-autotrace \
> >>> + --without-bzlib \
> >>> + --without-djvu \
> >>> + --without-dps \
> >>> + --without-fftw \
> >>> + --without-flif \
> >>> + --without-fpx \
> >>> + --without-fontconfig \
> >>> + --without-freetype \
> >>> + --without-gslib \
> >>> + --without-gvc \
> >>> + --without-jbig \
> >>> + --without-lcms \
> >>> + --without-lqr \
> >>> + --without-lzma \
> >>> + --without-magick-plus-plus \
> >>> + --without-openexr \
> >>> + --without-openjp2 \
> >>> + --without-pango \
> >>> + --without-perl \
> >>> + --without-raqm \
> >>> + --without-rsvg \
> >>> + --without-tiff \
> >>> + --without-webp \
> >>> + --without-wmf \
> >>> + --without-x \
> >>> + --without-xml \
> >
> > These options mostly look like they should be additional flags in the
> > .in file, and pull in the respective packages.
>
> Sure. But I didn't want to just enable a heap of options without
> proper testing. I only enabled the options that I use myself and thus
> I can guarantee that they work. I though it would be better to submit
> this patch as a starting point, then other users can submit patches as
> they need other features, after they have tested them.
I agree. But make sure that all options are sorted the same ways as in the
output of './configure --help'.
> >
> >>> + --$(call ptx/wwo, PTXCONF_IMAGEMAGICK_USE_ZLIB)-zlib \
> >>> + --$(call ptx/wwo, PTXCONF_IMAGEMAGICK_USE_LIBPNG)-png \
> >>> + --$(call ptx/wwo, PTXCONF_IMAGEMAGICK_USE_LIBJPEG)-jpeg
> >
> > ... like here :)
> >
> >>> +
> >>> +# ----------------------------------------------------------------------------
> >>> +# Target-Install
> >>> +# ----------------------------------------------------------------------------
> >>> +
> >>> +$(STATEDIR)/imagemagick.targetinstall:
> >>> + @$(call targetinfo)
> >>> +
> >>> + @$(call install_init, imagemagick)
> >>> + @$(call install_fixup, imagemagick, PRIORITY, optional)
> >>> + @$(call install_fixup, imagemagick, SECTION, base)
> >>> + @$(call install_fixup, imagemagick, AUTHOR, "Guillermo Rodriguez <guille.rodriguez@gmail.com>")
> >>> + @$(call install_fixup, imagemagick, DESCRIPTION, missing)
> >>> +
> >>> + @$(call install_copy, imagemagick, 0, 0, 0755, -, /usr/bin/magick)
> >>> + @$(call install_lib, imagemagick, 0, 0, 0644, libMagickCore-7.Q$(IMAGEMAGICK_QUANTUM_DEPTH))
> >>> + @$(call install_lib, imagemagick, 0, 0, 0644, libMagickWand-7.Q$(IMAGEMAGICK_QUANTUM_DEPTH))
> >>> +
> >>> + @$(call install_link, imagemagick, magick, /usr/bin/compare)
> >>> + @$(call install_link, imagemagick, magick, /usr/bin/composite)
> >>> + @$(call install_link, imagemagick, magick, /usr/bin/conjure)
> >>> + @$(call install_link, imagemagick, magick, /usr/bin/convert)
> >>> + @$(call install_link, imagemagick, magick, /usr/bin/identify)
> >>> + @$(call install_link, imagemagick, magick, /usr/bin/magick-script)
> >>> + @$(call install_link, imagemagick, magick, /usr/bin/mogrify)
> >>> + @$(call install_link, imagemagick, magick, /usr/bin/montage)
> >>> + @$(call install_link, imagemagick, magick, /usr/bin/stream)
> >>> +# These require X11 support:
> >>> +# @$(call install_link, imagemagick, magick, /usr/bin/animate)
> >>> +# @$(call install_link, imagemagick, magick, /usr/bin/display)
> >>> +# @$(call install_link, imagemagick, magick, /usr/bin/import)
> >
> > ... then you can also install those when the user selects --with-x
>
> Yes, same as above :)
Just leave the stuff out. We can add it again when needed.
> >
> >>> +
> >>> + @$(call install_alternative, imagemagick, 0, 0, 0644, /etc/ImageMagick-7/coder.xml)
> >>> + @$(call install_alternative, imagemagick, 0, 0, 0644, /etc/ImageMagick-7/colors.xml)
> >>> +# This file contains paths to external programs detected in the host:
> >>> +# @$(call install_alternative, imagemagick, 0, 0, 0644, /etc/ImageMagick-7/delegates.xml)
How are these detected? AC_PATH_PROG or something like that? In that case,
add correct environment variables to the _CONF_ENV should fix that.
Michael
> >>> + @$(call install_alternative, imagemagick, 0, 0, 0644, /etc/ImageMagick-7/log.xml)
> >>> + @$(call install_alternative, imagemagick, 0, 0, 0644, /etc/ImageMagick-7/magic.xml)
> >>> + @$(call install_alternative, imagemagick, 0, 0, 0644, /etc/ImageMagick-7/mime.xml)
> >>> + @$(call install_alternative, imagemagick, 0, 0, 0644, /etc/ImageMagick-7/policy.xml)
> >>> + @$(call install_alternative, imagemagick, 0, 0, 0644, /etc/ImageMagick-7/quantization-table.xml)
> >>> + @$(call install_alternative, imagemagick, 0, 0, 0644, /etc/ImageMagick-7/thresholds.xml)
> >>> +
> >>> + @$(call install_finish, imagemagick)
> >>> +
> >>> + @$(call touch)
> >>> +
> >>> +# vim: syntax=make
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
_______________________________________________
ptxdist mailing list
ptxdist@pengutronix.de
next prev parent reply other threads:[~2016-09-15 7:56 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-09-02 10:14 Guillermo Rodriguez
2016-09-14 6:31 ` Guillermo Rodriguez Garcia
2016-09-14 21:32 ` Roland Hieber
2016-09-15 6:34 ` Guillermo Rodriguez Garcia
2016-09-15 7:56 ` Michael Olbrich [this message]
2016-09-15 8:48 ` Guillermo Rodriguez Garcia
2016-10-14 9:00 ` [ptxdist] [PATCH v2] " Guillermo Rodriguez
2016-10-15 10:16 ` Guillermo Rodriguez Garcia
2016-10-21 9:32 ` Guillermo Rodriguez Garcia
2016-10-21 9:47 ` Michael Olbrich
2016-10-21 10:10 ` Guillermo Rodriguez Garcia
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20160915075625.toop5x6s4k7qnsql@pengutronix.de \
--to=m.olbrich@pengutronix.de \
--cc=ptxdist@pengutronix.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox