r108141 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r108140‎ | r108141 | r108142 >
Date:14:48, 5 January 2012
Author:hashar
Status:resolved (Comments)
Tags:core, hexmode 
Comment:
rewrite getXCFMetaData() to get ride of im identify

By reading the file header and unpacking the data, we can avoid
shelling out to imagemagick 'identify'. Save up some CPU cycles :D

An XCF is made of a canvas of a given width / height, the various
layers are applied to it which must fit in the canvas. So we just
use the canvas size :-)

I do not think we have any usage for channels count, so I have just
skip that part. I am not sure it makes any sense when the picture
can be made of several layers each using different channels count.

Bits per color is always 8 per definition. Grayscale is 0 - 255
and indexed palette is 256 colors at most.

XCF spec:
http://svn.gnome.org/viewvc/gimp/trunk/devel-docs/xcf.txt?view=markup

pack() / unpack() is familiar to perl monkeys


Fully reimplements r107351
Modified paths:
  • /trunk/phase3/includes/media/XCF.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/media/XCF.php
@@ -42,56 +42,76 @@
4343 return self::getXCFMetaData( $filename );
4444 }
4545
 46+ /**
 47+ * Metadata for a given XCF file
 48+ *
 49+ * Will return false if file magic signature is not recognized
 50+ * @author Hexmode
 51+ * @author Hashar
 52+ *
 53+ * @param $filename String Full path to a XCF file
 54+ * @return false|metadata array just like PHP getimagesize()
 55+ */
4656 static function getXCFMetaData( $filename ) {
47 - global $wgImageMagickIdentifyCommand;
 57+ # Decode master structure
 58+ $f = fopen( $filename, 'rb' );
 59+ if( !$f ) {
 60+ return false;
 61+ }
 62+ # The image structure always starts at offset 0 in the XCF file.
 63+ # So we just read it :-)
 64+ $binaryHeader = fread( $f, 26 );
 65+ fclose($f);
4866
49 - $cmd = wfEscapeShellArg( $wgImageMagickIdentifyCommand ) . ' -verbose ' . wfEscapeShellArg( $filename );
50 - wfDebug( __METHOD__ . ": Running $cmd \n" );
 67+ # Master image structure:
 68+ #
 69+ # byte[9] "gimp xcf " File type magic
 70+ # byte[4] version XCF version
 71+ # "file" - version 0
 72+ # "v001" - version 1
 73+ # "v002" - version 2
 74+ # byte 0 Zero-terminator for version tag
 75+ # uint32 width With of canvas
 76+ # uint32 height Height of canvas
 77+ # uint32 base_type Color mode of the image; one of
 78+ # 0: RGB color
 79+ # 1: Grayscale
 80+ # 2: Indexed color
 81+ # (enum GimpImageBaseType in libgimpbase/gimpbaseenums.h)
 82+ $header = unpack(
 83+ "A9magic" # A: space padded
 84+ . "/a5version" # a: zero padded
 85+ . "/Nwidth" # \
 86+ . "/Nheight" # N: unsigned long 32bit big endian
 87+ . "/Nbase_type" # /
 88+ , $binaryHeader
 89+ );
5190
52 - $retval = null;
53 - $return = wfShellExec( $cmd, $retval );
54 - if( $retval !== 0 ) {
55 - wfDebug( __METHOD__ . ": error encountered while running $cmd\n" );
 91+ # Check values
 92+ if( $header['magic'] !== 'gimp xcf' ) {
 93+ var_dump( $header );
 94+ wfDebug( __METHOD__ . " '$filename' has invalid magic signature.\n" );
5695 return false;
5796 }
 97+ # TODO: we might want to check for sane values of width and height
5898
59 - $colorspace = preg_match_all( '/ *Colorspace: RGB/', $return, $match );
60 - $frameCount = preg_match_all( '/ *Geometry: ([0-9]+x[0-9]+)\+[+0-9]*/', $return, $match );
61 - wfDebug( __METHOD__ . ": Got $frameCount matches\n" );
 99+ wfDebug( __METHOD__ . ": canvas size of '$filename' is {$header['width']} x {$header['height']} px\n" );
62100
63 - /* if( $frameCount == 1 ) { */
64 - /* preg_match( '/([0-9]+)x([0-9]+)/sm', $match[1][0], $m ); */
65 - /* $sizeX = $m[1]; */
66 - /* $sizeY = $m[2]; */
67 - /* } else { */
68 - $sizeX = 0;
69 - $sizeY = 0;
70 -
71 - # Find out the largest width and height used in any frame
72 - foreach( $match[1] as $res ) {
73 - preg_match( '/([0-9]+)x([0-9]+)/sm', $res, $m );
74 - if( $m[1] > $sizeX ) {
75 - $sizeX = $m[1];
76 - }
77 - if( $m[2] > $sizeY ) {
78 - $sizeY = $m[2];
79 - }
80 - }
81 - /* } */
82 -
83 - wfDebug( __METHOD__ . ": Found $sizeX x $sizeY x $frameCount \n" );
84 -
85101 # Forge a return array containing metadata information just like getimagesize()
86102 # See PHP documentation at: http://www.php.net/getimagesize
87103 $metadata = array();
88 - $metadata['frameCount'] = $frameCount;
89 - $metadata[0] = $sizeX;
90 - $metadata[1] = $sizeY;
91 - $metadata[2] = null;
92 - $metadata[3] = "height=\"$sizeY\" width=\"$sizeX\"";
 104+ $metadata[0] = $header['width'];
 105+ $metadata[1] = $header['height'];
 106+ $metadata[2] = null; # IMAGETYPE constant, none exist for XCF.
 107+ $metadata[3] = sprintf(
 108+ 'height="%s" width="%s"', $header['height'], $header['width']
 109+ );
93110 $metadata['mime'] = 'image/x-xcf';
94 - $metadata['channels'] = $colorspace == 1 ? 3 : 4;
 111+ $metadata['channels'] = null;
 112+ $metadata['bits'] = 8; # Always 8-bits per color
95113
 114+ assert( '7 == count($metadata); # return array must contains 7 elements just like getimagesize() return' );
 115+
96116 return $metadata;
97117 }
98118

Follow-up revisions

RevisionCommit summaryAuthorDate
r108181XCF: remove var_dump() statementhashar23:01, 5 January 2012
r108248XCF: suppress warning on unpack + early exit on error...hashar16:17, 6 January 2012
r108253Follow up r108248, r108141: use wfUnpack() so we don't duplicate logic. Could...demon16:58, 6 January 2012

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r107351* First simple XCF thumbnailing. Convert from ImageMagick has buggy...mah00:46, 27 December 2011

Comments

#Comment by Hashar (talk | contribs)   16:34, 5 January 2012

XCF test pictures added with r108149 in tests/phpunit/data/media

#Comment by Bawolff (talk | contribs)   17:27, 5 January 2012

Should possibly use wfUnpack to prevent php warnings in cases of malformed files.

#Comment by Hashar (talk | contribs)   16:17, 6 January 2012

r108248

I just suppressed warnings since I am not willing to handle exceptions thrown by wfUnpack() :-D

#Comment by 😂 (talk | contribs)   16:59, 6 January 2012

Handled the exception in r108253. If someone wants to add $length for sanity they can.

#Comment by Brion VIBBER (talk | contribs)   17:57, 5 January 2012

+ var_dump( $header );

^ this should probably be removed :)

#Comment by Hashar (talk | contribs)   23:01, 5 January 2012

- var_dump( $header );

With r108181. Thanks Brion!

#Comment by Reedy (talk | contribs)   17:23, 27 January 2012

Shouldn't it be Llamas and/or Camels?

#Comment by Hashar (talk | contribs)   09:41, 28 January 2012

> Shouldn't it be Llamas and/or Camels?

Those are the most known animals and there are many more in use. In the end, guess who is riding them? Monkeys!!! :-D

Status & tagging log