mailarchive of the ptxdist mailing list
 help / color / mirror / Atom feed
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 08:34:45 +0200	[thread overview]
Message-ID: <CABDcavaJg4rtQw1FzOXEq4__6M2aSqQUjP8hNQWjQJo7b9Pryw@mail.gmail.com> (raw)
In-Reply-To: <2d56a057-d0a8-6efb-73bd-211f652a99ec@rohieb.name>

Hi Roland,

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.

>
>>> +
>>> +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.

>
>>> +       --$(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 :)

Thank,

Guillermo Rodriguez Garcia
guille.rodriguez@gmail.com

>
>>> +
>>> +       @$(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)
>>> +       @$(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
>>> --
>>> 1.7.9.5
>>>
>
>
> _______________________________________________
> ptxdist mailing list
> ptxdist@pengutronix.de

_______________________________________________
ptxdist mailing list
ptxdist@pengutronix.de

  reply	other threads:[~2016-09-15  6:34 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 [this message]
2016-09-15  7:56       ` Michael Olbrich
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=CABDcavaJg4rtQw1FzOXEq4__6M2aSqQUjP8hNQWjQJo7b9Pryw@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