r71168 MediaWiki - Code Review archive

Repository:	MediaWiki
Revision:	< r71167‎ \| r71168 \| r71169 >
Date:	20:58, 16 August 2010
Author:	bawolff
Status:	ok (Comments)
Tags:
Comment:	Add support for extracting XMP and file comments from GIF images. *Also some minor changes to png handling for consistancy.
Modified paths:	/branches/img_metadata/phase3/includes/media/BitmapMetadataHandler.php (modified) (history) /branches/img_metadata/phase3/includes/media/FormatMetadata.php (modified) (history) /branches/img_metadata/phase3/includes/media/GIF.php (modified) (history) /branches/img_metadata/phase3/includes/media/GIFMetadataExtractor.php (modified) (history) /branches/img_metadata/phase3/includes/media/PNG.php (modified) (history) /branches/img_metadata/phase3/includes/media/PNGMetadataExtractor.php (modified) (history) /branches/img_metadata/phase3/languages/messages/MessagesEn.php (modified) (history) /branches/img_metadata/phase3/maintenance/language/messageTypes.inc (modified) (history) /branches/img_metadata/phase3/maintenance/language/messages.inc (modified) (history)

Diff [purge]

Index: branches/img_metadata/phase3/maintenance/language/messages.inc
—	—	@@ -2755,6 +2755,7 @@
2756	2756	'exif-pngfilecomment',
2757	2757	'exif-disclaimer',
2758	2758	'exif-contentwarning',
	2759	+ 'exif-giffilecomment',
2759	2760	),
2760	2761	'exif-values' => array(
2761	2762	'exif-make-value',
Index: branches/img_metadata/phase3/maintenance/language/messageTypes.inc
—	—	@@ -672,4 +672,5 @@
673	673	'exif-pngfilecomment',
674	674	'exif-disclaimer',
675	675	'exif-contentwarning',
	676	+ 'exif-giffilecomment',
676	677	);
Index: branches/img_metadata/phase3/includes/media/FormatMetadata.php
—	—	@@ -655,6 +655,7 @@
656	656	case 'PNGFileComment':
657	657	case 'Disclaimer':
658	658	case 'ContentWarning':
	659	+ case 'GIFFileComment':
659	660
660	661	$val = htmlspecialchars( $val );
661	662	break;
Index: branches/img_metadata/phase3/includes/media/GIFMetadataExtractor.php
—	—	@@ -12,6 +12,8 @@
13	13	static $gif_extension_sep;
14	14	static $gif_term;
15	15
	16	+ const VERSION = 1;
	17	+
16	18	static function getMetadata( $filename ) {
17	19	self::$gif_frame_sep = pack( "C", ord("," ) );
18	20	self::$gif_extension_sep = pack( "C", ord("!" ) );
—	—	@@ -20,6 +22,8 @@
21	23	$frameCount = 0;
22	24	$duration = 0.0;
23	25	$isLooped = false;
	26	+ $xmp = "";
	27	+ $comment = array();
24	28
25	29	if (!$filename)
26	30	throw new Exception( "No file name specified" );
—	—	@@ -91,37 +95,98 @@
92	96	$term = $term[1];
93	97	if ($term != 0 )
94	98	throw new Exception( "Malformed Graphics Control Extension block" );
	99	+ } elseif ($extension_code == 0xFE) {
	100	+ // Comment block(s).
	101	+ $data = '';
	102	+
	103	+ $subLength = fread( $fh, 1 );
	104	+ if ( $subLength === "\0" ) {
	105	+ throw new Exception( 'Read error, zero-length comment block' );
	106	+ }
	107	+ while( $subLength !== "\0" ) {
	108	+ $data .= fread( $fh, ord( $subLength ) );
	109	+ $subLength = fread( $fh, 1 );
	110	+ }
	111	+
	112	+ // The standard says this should be ASCII, however its unclear if
	113	+ // thats true in practise. Check to see if its valid utf-8, if so
	114	+ // assume its that, otherwise assume its iso-8859-1
	115	+ $dataCopy = $data;
	116	+ // quickIsNFCVerify has the side effect of replacing any invalid characters
	117	+ UtfNormal::quickIsNFCVerify( $dataCopy );
	118	+
	119	+ if ( $dataCopy !== $data ) {
	120	+ wfSuppressWarnings();
	121	+ $data = iconv( 'ISO-8859-1', 'UTF-8', $data );
	122	+ wfRestoreWarnings();
	123	+ }
	124	+
	125	+ $comment[] = $data;
	126	+
95	127	} elseif ($extension_code == 0xFF) {
96	128	// Application extension (Netscape info about the animated gif)
	129	+ // or XMP (or theoretically any other type of extension block)
97	130	$blockLength = fread( $fh, 1 );
98	131	$blockLength = unpack( 'C', $blockLength );
99	132	$blockLength = $blockLength[1];
100	133	$data = fread( $fh, $blockLength );
101	134
102		~~- // NETSCAPE2.0 (application name)~~
103		~~- if ($blockLength != 11 \|\| $data != 'NETSCAPE2.0') {~~
	135	+ if ($blockLength != 11 ) {
	136	+ wfDebug( __METHOD__ . ' GIF application block with wrong length' );
104	137	fseek( $fh, -($blockLength + 1), SEEK_CUR );
105	138	self::skipBlock( $fh );
106	139	continue;
107	140	}
	141	+
	142	+ // NETSCAPE2.0 (application name for animated gif)
	143	+ if ( $data == 'NETSCAPE2.0' ) {
108	144
109		~~- $data = fread( $fh, 2 ); // Block length and introduction, should be 03 01~~
110		-
111		~~- if ($data != "\x03\x01") {~~
112		~~- throw new Exception( "Expected \x03\x01, got $data" );~~
	145	+ $data = fread( $fh, 2 ); // Block length and introduction, should be 03 01
	146	+
	147	+ if ($data != "\x03\x01") {
	148	+ throw new Exception( "Expected \x03\x01, got $data" );
	149	+ }
	150	+
	151	+ // Unsigned little-endian integer, loop count or zero for "forever"
	152	+ $loopData = fread( $fh, 2 );
	153	+ $loopData = unpack( 'v', $loopData );
	154	+ $loopCount = $loopData[1];
	155	+
	156	+ if ($loopCount != 1) {
	157	+ $isLooped = true;
	158	+ }
	159	+
	160	+ // Read out terminator byte
	161	+ fread( $fh, 1 );
	162	+ } elseif ( $data == 'XMP DataXMP' ) {
	163	+ // application name for XMP data.
	164	+ // see pg 18 of XMP spec part 3.
	165	+
	166	+ $xmp = '';
	167	+ $subLength = fread( $fh, 1 );
	168	+ while( $subLength !== "\0" ) {
	169	+ $xmp .= $subLength;
	170	+ $xmp .= fread( $fh, ord( $subLength ) );
	171	+ $subLength = fread( $fh, 1 );
	172	+ }
	173	+
	174	+ if ( substr( $xmp, -257, 3 ) !== "\x01\xFF\xFE"
	175	+ \|\| substr( $xmp, -4 ) !== "\x03\x02\x01\x00" )
	176	+ {
	177	+ // this is just a sanity check.
	178	+ throw new Exception( "XMP does not have magic trailer!" );
	179	+ }
	180	+
	181	+ // strip out trailer.
	182	+ $xmp = substr( $xmp, 0, -257 );
	183	+
	184	+ } else {
	185	+ // unrecognized extension block
	186	+ fseek( $fh, -($blockLength + 1), SEEK_CUR );
	187	+ self::skipBlock( $fh );
	188	+ continue;
113	189	}
114		-
115		~~- // Unsigned little-endian integer, loop count or zero for "forever"~~
116		~~- $loopData = fread( $fh, 2 );~~
117		~~- $loopData = unpack( 'v', $loopData );~~
118		~~- $loopCount = $loopData[1];~~
119		-
120		~~- if ($loopCount != 1) {~~
121		~~- $isLooped = true;~~
122		~~- }~~
123		-
124		~~- // Read out terminator byte~~
125		~~- fread( $fh, 1 );~~
	190	+
126	191	} else {
127	192	self::skipBlock( $fh );
128	193	}
—	—	@@ -137,7 +202,9 @@
138	203	return array(
139	204	'frameCount' => $frameCount,
140	205	'looped' => $isLooped,
141		~~- 'duration' => $duration~~
	206	+ 'duration' => $duration,
	207	+ 'xmp' => $xmp,
	208	+ 'comment' => $comment,
142	209	);
143	210
144	211	}
Index: branches/img_metadata/phase3/includes/media/GIF.php
—	—	@@ -12,24 +12,34 @@
13	13	class GIFHandler extends BitmapHandler {
14	14
15	15	function getMetadata( $image, $filename ) {
16		~~- if ( !isset($image->parsedGIFMetadata) ) {~~
17		~~- try {~~
18		~~- $image->parsedGIFMetadata = GIFMetadataExtractor::getMetadata( $filename );~~
19		~~- } catch( Exception $e ) {~~
20		~~- // Broken file?~~
21		~~- wfDebug( __METHOD__ . ': ' . $e->getMessage() . "\n" );~~
22		~~- return '0';~~
23		~~- }~~
	16	+ try {
	17	+ $parsedGIFMetadata = BitmapMetadataHandler::GIF( $filename );
	18	+ } catch( Exception $e ) {
	19	+ // Broken file?
	20	+ wfDebug( __METHOD__ . ': ' . $e->getMessage() . "\n" );
	21	+ return '0';
24	22	}
25	23
26		~~- return serialize($image->parsedGIFMetadata);~~
27		-
	24	+ return serialize($parsedGIFMetadata);
28	25	}
29	26
30	27	function formatMetadata( $image ) {
31		~~- return false;~~
	28	+ $meta = $image->getMetadata();
	29	+
	30	+ if ( !$meta ) {
	31	+ return false;
	32	+ }
	33	+ $meta = unserialize( $meta );
	34	+ if ( !isset( $meta['metadata'] ) \|\| count( $meta['metadata'] ) <= 1 ) {
	35	+ return false;
	36	+ }
	37	+
	38	+ if ( isset( $meta['metadata']['_MW_GIF_VERSION'] ) ) {
	39	+ unset( $meta['metadata']['_MW_GIF_VERSION'] );
	40	+ }
	41	+ return $this->formatMetadataHelper( $meta['metadata'] );
32	42	}
33		-
	43	+
34	44	function getImageArea( $image, $width, $height ) {
35	45	$ser = $image->getMetadata();
36	46	if ($ser) {
—	—	@@ -54,12 +64,29 @@
55	65	}
56	66
57	67	function isMetadataValid( $image, $metadata ) {
	68	+ if ( $metadata === '0' ) {
	69	+ // Do not repetitivly regenerate metadata on broken file.
	70	+ return self::METADATA_GOOD;
	71	+ }
	72	+
58	73	wfSuppressWarnings();
59	74	$data = unserialize( $metadata );
60	75	wfRestoreWarnings();
61		~~- return (boolean) $data;~~
	76	+
	77	+ if ( !$data \|\| !is_array( $data ) ) {
	78	+ wfDebug(__METHOD__ . ' invalid GIF metadata' );
	79	+ return self::METADATA_BAD;
	80	+ }
	81	+
	82	+ if ( !isset( $data['metadata']['_MW_GIF_VERSION'] )
	83	+ \|\| $data['metadata']['_MW_GIF_VERSION'] != GIFMetadataExtractor::VERSION ) {
	84	+ wfDebug(__METHOD__ . ' old but compatible GIF metadata' );
	85	+ return self::METADATA_COMPATIBLE;
	86	+ }
	87	+ return self::METADATA_GOOD;
62	88	}
63	89
	90	+
64	91	function getLongDesc( $image ) {
65	92	global $wgUser, $wgLang;
66	93	$sk = $wgUser->getSkin();
Index: branches/img_metadata/phase3/includes/media/BitmapMetadataHandler.php
—	—	@@ -9,11 +9,12 @@
10	10	@todo other image formats.
11	11	*/
12	12	class BitmapMetadataHandler {
	13	+
13	14	private $filename;
14	15	private $metadata = Array();
15	16	private $metaPriority = Array(
16	17	20 => Array( 'other' ),
17		~~- 40 => Array( 'file-comment', 'native-png' ),~~
	18	+ 40 => Array( 'native' ),
18	19	60 => Array( 'iptc-good-hash', 'iptc-no-hash' ),
19	20	70 => Array( 'xmp-deprected' ),
20	21	80 => Array( 'xmp-general' ),
—	—	@@ -126,7 +127,7 @@
127	128	$seg = Array();
128	129	$seg = JpegMetadataExtractor::segmentSplitter( $filename );
129	130	if ( isset( $seg['COM'] ) && isset( $seg['COM'][0] ) ) {
130		~~- $meta->addMetadata( Array( 'JPEGFileComment' => $seg['COM'] ), 'file-comment' );~~
	131	+ $meta->addMetadata( Array( 'JPEGFileComment' => $seg['COM'] ), 'native' );
131	132	}
132	133	if ( isset( $seg['PSIR'] ) ) {
133	134	$meta->doApp13( $seg['PSIR'] );
—	—	@@ -170,11 +171,46 @@
171	172	}
172	173	}
173	174	unset( $array['text']['xmp'] );
174		~~- $meta->addMetadata( $array['text'], 'native-png' );~~
	175	+ $meta->addMetadata( $array['text'], 'native' );
175	176	unset( $array['text'] );
176	177	$array['metadata'] = $meta->getMetadataArray();
177		~~- $array['metadata']['_MW_PNG_VERSION'] = '1';~~
	178	+ $array['metadata']['_MW_PNG_VERSION'] = PNGMetadataExtractor::VERSION;
178	179	return $array;
179	180	}
180	181
	182	+ /** function for gif images.
	183	+ *
	184	+ * They don't really have native metadata, so just merges together
	185	+ * XMP and image comment.
	186	+ *
	187	+ * @param $filename full path to file
	188	+ * @return Array metadata array
	189	+ */
	190	+ static public function GIF ( $filename ) {
	191	+
	192	+ $meta = new self( $filename );
	193	+ $baseArray = GIFMetadataExtractor::getMetadata( $filename );
	194	+
	195	+ if ( count( $baseArray['comment'] ) > 0 ) {
	196	+ $meta->addMetadata( array( 'GIFFileComment' => $baseArray['comment'] ), 'native' );
	197	+ }
	198	+
	199	+ if ( $baseArray['xmp'] !== '' && function_exists( 'xml_parser_create_ns' ) ) {
	200	+ $xmp = new XMPReader();
	201	+ $xmp->parse( $baseArray['xmp'] );
	202	+ $xmpRes = $xmp->getResults();
	203	+ foreach ( $xmpRes as $type => $xmpSection ) {
	204	+ $meta->addMetadata( $xmpSection, $type );
	205	+ }
	206	+
	207	+ }
	208	+
	209	+ unset( $baseArray['comment'] );
	210	+ unset( $baseArray['xmp'] );
	211	+
	212	+ $baseArray['metadata'] = $meta->getMetadataArray();
	213	+ $baseArray['metadata']['_MW_GIF_VERSION'] = GIFMetadataExtractor::VERSION;
	214	+ return $baseArray;
	215	+ }
	216	+
181	217	}
Index: branches/img_metadata/phase3/includes/media/PNGMetadataExtractor.php
—	—	@@ -11,6 +11,8 @@
12	12	static $CRC_size;
13	13	static $text_chunks;
14	14
	15	+ const VERSION = 1;
	16	+
15	17	static function getMetadata( $filename ) {
16	18	self::$png_sig = pack( "C8", 137, 80, 78, 71, 13, 10, 26, 10 );
17	19	self::$CRC_size = 4;
Index: branches/img_metadata/phase3/includes/media/PNG.php
—	—	@@ -54,19 +54,23 @@
55	55	}
56	56
57	57	function isMetadataValid( $image, $metadata ) {
	58	+
	59	+ if ( $metadata === '0' ) {
	60	+ // Do not repetitivly regenerate metadata on broken file.
	61	+ return self::METADATA_GOOD;
	62	+ }
	63	+
58	64	wfSuppressWarnings();
59	65	$data = unserialize( $metadata );
60	66	wfRestoreWarnings();
61		~~- if ( $data === '0' ) {~~
62		~~- // Do not repetitivly regenerate metadata on broken file.~~
63		~~- return self::METADATA_GOOD;~~
64		~~- }~~
	67	+
65	68	if ( !$data \|\| !is_array( $data ) ) {
66	69	wfDebug(__METHOD__ . ' invalid png metadata' );
67	70	return self::METADATA_BAD;
68	71	}
69	72
70		~~- if ( !isset( $data['metadata']['_MW_PNG_VERSION'] ) ) {~~
	73	+ if ( !isset( $data['metadata']['_MW_PNG_VERSION'] )
	74	+ \|\| $data['metadata']['_MW_PNG_VERSION'] != PNGMetadataExtractor::VERSION ) {
71	75	wfDebug(__METHOD__ . ' old but compatible png metadata' );
72	76	return self::METADATA_COMPATIBLE;
73	77	}
Index: branches/img_metadata/phase3/languages/messages/MessagesEn.php
—	—	@@ -3824,6 +3824,7 @@
3825	3825	'exif-pngfilecomment' => 'PNG file comment',
3826	3826	'exif-disclaimer' => 'Disclaimer',
3827	3827	'exif-contentwarning' => 'Content warning',
	3828	+'exif-giffilecomment' => 'GIF file comment',
3828	3829
3829	3830
3830	3831	# Make & model, can be wikified in order to link to the camera and model name

Follow-up revisions

Revision	Commit summary	Author	Date
r71326	Misc. fixes....	bawolff	04:18, 20 August 2010

Comments

#Comment by 😂 (talk | contribs) 21:57, 18 August 2010

Just picking a random example here (and I've seen it in other revs too: eg: r71117, r69021).

+			return '0';

Using values like this get confusing. Without context they mean nothing. Class constants would be nice.

Otherwise this commit is ok.

#Comment by Bawolff (talk | contribs) 01:18, 19 August 2010

That makes sense, I'll fix it next time I commit

Status & tagging log

21:57, 18 August 2010 😂 (talk | contribs) changed the status of r71168 [removed: new added: ok]