From: Guillermo Rodriguez Garcia <guille.rodriguez@gmail.com>
To: "ptxdist@pengutronix.de" <ptxdist@pengutronix.de>
Subject: Re: [ptxdist] [PATCH] New package: imagemagick
Date: Thu, 15 Sep 2016 10:48:34 +0200 [thread overview]
Message-ID: <CABDcava0qvwJPs0e-Gdzf8sFjwch=YRCWWL3HnsbGHhpYAPHug@mail.gmail.com> (raw)
In-Reply-To: <20160915075625.toop5x6s4k7qnsql@pengutronix.de>
Hi Michael,
2016-09-15 9:56 GMT+02:00 Michael Olbrich <m.olbrich@pengutronix.de>:
> 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.
In the submitted .make file there is a comment right below explaining
this along with other decisions (see "Notes:" below).
>
>> >
>> >>> +
>> >>> +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'.
Will do.
>
>> >
>> >>> + --$(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.
Ok.
>
>> >
>> >>> +
>> >>> + @$(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.
Yes, AC_PATH_PROG is used. Re. environment vars in _CONF_ENV: I don't
have much experience with autoconf, can you point to an example .make
file in ptxdist where this is done ?
Thank you,
Guillermo
>
> 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
--
Guillermo Rodriguez Garcia
guille.rodriguez@gmail.com
_______________________________________________
ptxdist mailing list
ptxdist@pengutronix.de
next prev parent reply other threads:[~2016-09-15 8:48 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
2016-09-15 8:48 ` Guillermo Rodriguez Garcia [this message]
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='CABDcava0qvwJPs0e-Gdzf8sFjwch=YRCWWL3HnsbGHhpYAPHug@mail.gmail.com' \
--to=guille.rodriguez@gmail.com \
--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