r71783 MediaWiki - Code Review archive

Repository:	MediaWiki
Revision:	< r71782‎ \| r71783 \| r71784 >
Date:	08:21, 27 August 2010
Author:	tstarling
Status:	deferred
Tags:
Comment:	Merged trunk changes through to r71782.
Modified paths:	/branches/wmf/1.16wmf4/extensions/PagedTiffHandler (modified) (history) /branches/wmf/1.16wmf4/extensions/PagedTiffHandler/PagedTiffHandler.image.php (modified) (history) /branches/wmf/1.16wmf4/extensions/PagedTiffHandler/PagedTiffHandler.php (modified) (history) /branches/wmf/1.16wmf4/extensions/PagedTiffHandler/PagedTiffHandler_body.php (modified) (history) /branches/wmf/1.16wmf4/extensions/PagedTiffHandler/tests/PagedTiffHandlerTest.php (modified) (history) /branches/wmf/1.16wmf4/extensions/PagedTiffHandler/tests/testImages/380mhz.tiff (added) (history) /branches/wmf/1.16wmf4/extensions/PagedTiffHandler/tests/testImages/SOURCES.txt (modified) (history)

Diff [purge]

Index: branches/wmf/1.16wmf4/extensions/PagedTiffHandler/tests/testImages/380mhz.tiff
Cannot display: file marked as a binary type.
svn:mime-type = application/octet-stream
Property changes on: branches/wmf/1.16wmf4/extensions/PagedTiffHandler/tests/testImages/380mhz.tiff
___________________________________________________________________
Added: svn:mime-type
1	1	+ application/octet-stream
Index: branches/wmf/1.16wmf4/extensions/PagedTiffHandler/tests/testImages/SOURCES.txt
—	—	@@ -13,4 +13,8 @@
14	14	----------
15	15
16	16	caspian.tif taken from libtiff test images. Obviously they are under no license. you can obtain the
17		~~- whole set of test images from http://www.libtiff.org/images.html~~
\ No newline at end of file
	17	+ whole set of test images from http://www.libtiff.org/images.html
	18	+
	19	+380mhz.tiff was taken from Wikimedia Commons, at http://commons.wikimedia.org/wiki/File:380mhz.tiff
	20	+ It was published by Travis Bland under the Creative Commons Attribution-Share Alike 3.0
	21	+ Unported license.
Index: branches/wmf/1.16wmf4/extensions/PagedTiffHandler/tests/PagedTiffHandlerTest.php
—	—	@@ -57,6 +57,7 @@
58	58
59	59	$this->multipage_path = dirname(__FILE__) . '/testImages/multipage.tiff';
60	60	$this->truncated_path = dirname(__FILE__) . '/testImages/truncated.tiff';
	61	+ $this->mhz_path = dirname(__FILE__) . '/testImages/380mhz.tiff';
61	62	$this->test_path = dirname(__FILE__) . '/testImages/test.tif';
62	63
63	64	if ( !file_exists( $this->truncated_path ) ) {
—	—	@@ -71,6 +72,12 @@
72	73	return false;
73	74	}
74	75
	76	+ if ( !file_exists( $this->mhz_path ) ) {
	77	+ echo "{$this->mhz_path} cannot be found.\n";
	78	+ $this->preCheckError = true;
	79	+ return false;
	80	+ }
	81	+
75	82	if ( !file_exists( $this->test_path ) ) {
76	83	echo "{$this->test_path} cannot be found.\n";
77	84	$this->preCheckError = true;
—	—	@@ -99,6 +106,17 @@
100	107	}
101	108	}
102	109
	110	+ $mhzTitle = Title::newFromText( 'Image:380mhz.tiff' );
	111	+ $this->mhz_image = wfFindFile( $mhzTitle );
	112	+ if ( !$this->mhz_image && $autoUpload ) {
	113	+ $this->mhz_image = $this->upload( $multipageTitle, $this->mhz_path );
	114	+
	115	+ if ( !$this->mhz_image ) {
	116	+ $this->preCheckError = true;
	117	+ return false;
	118	+ }
	119	+ }
	120	+
103	121	// force re-reading of meta-data
104	122	$truncated_tiff = $this->handler->getTiffImage( $this->truncated_image, $this->truncated_path );
105	123	$truncated_tiff->resetMetaData();
—	—	@@ -106,6 +124,9 @@
107	125	$multipage_tiff = $this->handler->getTiffImage( $this->multipage_image, $this->multipage_path );
108	126	$multipage_tiff->resetMetaData();
109	127
	128	+ $mhz_tiff = $this->handler->getTiffImage( $this->mhz_image, $this->mhz_path );
	129	+ $mhz_tiff->resetMetaData();
	130	+
110	131	return !$this->preCheckError;
111	132	}
112	133
—	—	@@ -120,6 +141,34 @@
121	142	$this->handler->getMetadata( $this->multipage_image, $this->multipage_path );
122	143	$this->handler->getMetadata( $this->truncated_image, $this->truncated_path );
123	144
	145	+
	146	+ // ---- Metadata handling
	147	+ // getMetadata
	148	+ $metadata = $this->handler->getMetadata( false, $this->multipage_path );
	149	+ $this->assertTrue( strpos( $metadata, '"page_amount";i:7' ) !== false );
	150	+ $this->assertTrue( $this->handler->isMetadataValid( $this->multipage_image, $metadata ) );
	151	+
	152	+ $metadata = $this->handler->getMetadata( false, $this->mhz_path );
	153	+ $this->assertTrue( strpos( $metadata, '"page_amount";i:1' ) !== false );
	154	+ $this->assertTrue( $this->handler->isMetadataValid( $this->mhz_image, $metadata ) );
	155	+
	156	+ // getMetaArray
	157	+ $metaArray = $this->handler->getMetaArray( $this->mhz_image );
	158	+ if ( !empty( $metaArray['errors'] ) ) $this->fail( join('; ', $metaArray['error']) );
	159	+ $this->assertEquals( $metaArray['page_amount'], 1 );
	160	+
	161	+ $this->assertEquals( strtolower( $metaArray['page_data'][1]['alpha'] ), 'true' );
	162	+
	163	+ $metaArray = $this->handler->getMetaArray( $this->multipage_image );
	164	+ if ( !empty( $metaArray['errors'] ) ) $this->fail( join('; ', $metaArray['error']) );
	165	+ $this->assertEquals( $metaArray['page_amount'], 7 );
	166	+
	167	+ $this->assertEquals( strtolower( $metaArray['page_data'][1]['alpha'] ), 'false' );
	168	+ $this->assertEquals( strtolower( $metaArray['page_data'][2]['alpha'] ), 'true' );
	169	+
	170	+ $interp = $metaArray['exif']['PhotometricInterpretation'];
	171	+ $this->assertTrue( $interp == 2 \|\| $interp == 'RGB' ); //RGB
	172	+
124	173	// ---- Parameter handling and lossy parameter
125	174	// validateParam
126	175	$this->assertTrue( $this->handler->validateParam( 'lossy', '0' ) );
—	—	@@ -129,19 +178,45 @@
130	179	$this->assertTrue( $this->handler->validateParam( 'lossy', 'lossy' ) );
131	180	$this->assertTrue( $this->handler->validateParam( 'lossy', 'lossless' ) );
132	181
	182	+ $this->assertTrue( $this->handler->validateParam( 'width', '60000' ) );
	183	+ $this->assertTrue( $this->handler->validateParam( 'height', '60000' ) );
	184	+ $this->assertTrue( $this->handler->validateParam( 'page', '60000' ) );
	185	+
	186	+ $this->assertFalse( $this->handler->validateParam( 'lossy', '' ) );
	187	+ $this->assertFalse( $this->handler->validateParam( 'lossy', 'quark' ) );
	188	+
	189	+ $this->assertFalse( $this->handler->validateParam( 'width', '160000' ) );
	190	+ $this->assertFalse( $this->handler->validateParam( 'height', '160000' ) );
	191	+ $this->assertFalse( $this->handler->validateParam( 'page', '160000' ) );
	192	+
	193	+ $this->assertFalse( $this->handler->validateParam( 'width', '0' ) );
	194	+ $this->assertFalse( $this->handler->validateParam( 'height', '0' ) );
	195	+ $this->assertFalse( $this->handler->validateParam( 'page', '0' ) );
	196	+
	197	+ $this->assertFalse( $this->handler->validateParam( 'width', '-1' ) );
	198	+ $this->assertFalse( $this->handler->validateParam( 'height', '-1' ) );
	199	+ $this->assertFalse( $this->handler->validateParam( 'page', '-1' ) );
	200	+
133	201	// normaliseParams
134	202	// here, boxfit behavior is tested
135	203	$params = array( 'width' => '100', 'height' => '100', 'page' => '4' );
136	204	$this->assertTrue( $this->handler->normaliseParams( $this->multipage_image, $params ) );
137	205	$this->assertEquals( $params['height'], 75 );
	206	+
138	207	// lossy and lossless
139	208	$params = array('width'=>'100', 'height'=>'100', 'page'=>'1');
140	209	$this->handler->normaliseParams($this->multipage_image, $params );
141	210	$this->assertEquals($params['lossy'], 'lossy');
	211	+
142	212	$params = array('width'=>'100', 'height'=>'100', 'page'=>'2');
143	213	$this->handler->normaliseParams($this->multipage_image, $params );
144	214	$this->assertEquals($params['lossy'], 'lossless');
145	215
	216	+ // single page
	217	+ $params = array('width'=>'100', 'height'=>'100', 'page'=>'1');
	218	+ $this->handler->normaliseParams($this->mhz_image, $params );
	219	+ $this->assertEquals($params['lossy'], 'lossless');
	220	+
146	221	// makeParamString
147	222	$this->assertEquals(
148	223	$this->handler->makeParamString(
—	—	@@ -185,6 +260,8 @@
186	261
187	262	// pageCount
188	263	$this->assertEquals( $this->handler->pageCount( $this->multipage_image ), 7 );
	264	+ $this->assertEquals( $this->handler->pageCount( $this->mhz_image ), 1 );
	265	+
189	266	// getPageDimensions
190	267	$this->assertEquals( $this->handler->getPageDimensions( $this->multipage_image, 0 ), array( 'width' => 1024, 'height' => 768 ) );
191	268	$this->assertEquals( $this->handler->getPageDimensions( $this->multipage_image, 1 ), array( 'width' => 1024, 'height' => 768 ) );
—	—	@@ -194,27 +271,17 @@
195	272	$this->assertEquals( $this->handler->getPageDimensions( $this->multipage_image, 5 ), array( 'width' => 1024, 'height' => 768 ) );
196	273	$this->assertEquals( $this->handler->getPageDimensions( $this->multipage_image, 6 ), array( 'width' => 1024, 'height' => 768 ) );
197	274	$this->assertEquals( $this->handler->getPageDimensions( $this->multipage_image, 7 ), array( 'width' => 768, 'height' => 1024 ) );
	275	+
	276	+ $this->assertEquals( $this->handler->getPageDimensions( $this->mhz_image, 0 ), array( 'width' => 643, 'height' => 452 ) );
	277	+
198	278	// return dimensions of last page if page number is too high
199	279	$this->assertEquals( $this->handler->getPageDimensions( $this->multipage_image, 8 ), array( 'width' => 768, 'height' => 1024 ) );
	280	+ $this->assertEquals( $this->handler->getPageDimensions( $this->mhz_image, 1 ), array( 'width' => 643, 'height' => 452 ) );
	281	+
200	282	// isMultiPage
201	283	$this->assertTrue( $this->handler->isMultiPage( $this->multipage_image ) );
	284	+ $this->assertFalse( $this->handler->isMultiPage( $this->mhz_image ) );
202	285
203		~~- // ---- Metadata handling~~
204		~~- // getMetadata~~
205		~~- $metadata = $this->handler->getMetadata( false, $this->multipage_path );~~
206		~~- $this->assertTrue( strpos( $metadata, '"page_amount";i:7' ) !== false );~~
207		~~- // isMetadataValid~~
208		~~- $this->assertTrue( $this->handler->isMetadataValid( $this->multipage_image, $metadata ) );~~
209		~~- // getMetaArray~~
210		~~- $metaArray = $this->handler->getMetaArray( $this->multipage_image );~~
211		-
212		~~- $this->assertEquals( $metaArray['page_amount'], 7 );~~
213		~~- //this is also strtolower in PagedTiffHandler::getThumbExtension~~
214		~~- $this->assertEquals( strtolower( $metaArray['page_data'][1]['alpha'] ), 'false' );~~
215		~~- $this->assertEquals( strtolower( $metaArray['page_data'][2]['alpha'] ), 'true' );~~
216		-
217		~~- $interp = $metaArray['exif']['PhotometricInterpretation'];~~
218		~~- $this->assertTrue( $interp == 2 \|\| $interp == 'RGB' ); //RGB~~
219	286	// formatMetadata
220	287	$formattedMetadata = $this->handler->formatMetadata( $this->multipage_image );
221	288
Index: branches/wmf/1.16wmf4/extensions/PagedTiffHandler/PagedTiffHandler_body.php
—	—	@@ -143,7 +143,17 @@
144	144	function validateParam( $name, $value ) {
145	145	if ( in_array( $name, array( 'width', 'height', 'page', 'lossy' ) ) ) {
146	146	if ( $name == 'lossy' ) {
147		~~- return in_array( $value, array( 1, 0, '1', '0', 'true', 'false', 'lossy', 'lossless' ) );~~
	147	+ # NOTE: make sure to use === for comparison. in PHP, '' == 0 and 'foo' == 1.
	148	+
	149	+ if ( $value === 1 \|\| $value === 0 \|\| $value === '1' \|\| $value === '0' ) {
	150	+ return true;
	151	+ }
	152	+
	153	+ if ( $value === 'true' \|\| $value === 'false' \|\| $value === 'lossy' \|\| $value === 'lossless' ) {
	154	+ return true;
	155	+ }
	156	+
	157	+ return false;
148	158	} elseif ( $value <= 0 \|\| $value > 65535 ) { // ImageMagick overflows for values > 65536
149	159	return false;
150	160	} else {
—	—	@@ -482,19 +492,17 @@
483	493	* If it returns false, Image will reload the metadata from the file and update the database
484	494	*/
485	495	function isMetadataValid( $image, $metadata ) {
486		-
487		~~- if ( !empty( $metadata ) && $metadata != serialize( array() ) ) {~~
488		~~- $meta = unserialize( $metadata );~~
489		~~- if ( isset( $meta['errors'] ) ) {~~
490		~~- // metadata contains an error message, but it's valid.~~
491		~~- // don't try to re-render until the error is resolved!~~
492		~~- return true;~~
493		~~- }~~
494		~~- if ( !empty( $meta['page_amount'] ) && !empty( $meta['page_data'] ) ) {~~
495		~~- return true;~~
496		~~- }~~
	496	+ if ( is_string( $metadata ) ) $metadata = unserialize( $metadata );
	497	+
	498	+ if ( !isset( $metadata['TIFF_METADATA_VERSION'] ) ) {
	499	+ return false;
497	500	}
498		~~- return false;~~
	501	+
	502	+ if ( $metadata['TIFF_METADATA_VERSION'] != TIFF_METADATA_VERSION ) {
	503	+ return false;
	504	+ }
	505	+
	506	+ return true;
499	507	}
500	508
501	509	/**
Index: branches/wmf/1.16wmf4/extensions/PagedTiffHandler/PagedTiffHandler.php
—	—	@@ -124,6 +124,8 @@
125	125	$wgHooks['UploadVerification'][] = 'PagedTiffHandler::check';
126	126	$wgHooks['LanguageGetMagic'][] = 'PagedTiffHandler::addTiffLossyMagicWordLang';
127	127
	128	+define('TIFF_METADATA_VERSION', '1.0');
	129	+
128	130	//$wgHooks['PagedTiffHandlerRenderCommand'][] = 'PagedTiffHandler::renderCommand';
129	131	//$wgHooks['PagedTiffHandlerTiffData'][] = 'PagedTiffImage::tiffData';
130	132	//$wgHooks['PagedTiffHandlerExifData'][] = 'PagedTiffImage::exifData';
Index: branches/wmf/1.16wmf4/extensions/PagedTiffHandler/PagedTiffHandler.image.php
—	—	@@ -171,16 +171,44 @@
172	172	$this->_meta['exif'] = $data;
173	173	}
174	174	}
	175	+
	176	+ unset( $this->_meta['exif']['Image'] );
	177	+ unset( $this->_meta['exif']['filename'] );
	178	+ unset( $this->_meta['exif']['Base filename'] );
	179	+ unset( $this->_meta['exif']['XMLPacket'] );
	180	+ unset( $this->_meta['exif']['ImageResources'] );
	181	+
	182	+ $this->_meta['TIFF_METADATA_VERSION'] = TIFF_METADATA_VERSION;
	183	+
175	184	wfProfileOut( 'PagedTiffImage::retrieveMetaData' );
176	185	}
177		~~- unset( $this->_meta['exif']['Image'] );~~
178		~~- unset( $this->_meta['exif']['filename'] );~~
179		~~- unset( $this->_meta['exif']['Base filename'] );~~
180		~~- unset( $this->_meta['exif']['XMLPacket'] );~~
181		~~- unset( $this->_meta['exif']['ImageResources'] );~~
	186	+
182	187	return $this->_meta;
183	188	}
184	189
	190	+ private function addPageEntry( $entry, &$metadata, &$prevPage ) {
	191	+ if ( !isset( $entry['page'] ) ) {
	192	+ $entry['page'] = $prevPage +1;
	193	+ } else {
	194	+ if ( $prevPage >= $entry['page'] ) {
	195	+ $metadata['errors'][] = "inconsistent page numbering in TIFF directory";
	196	+ return false;
	197	+ }
	198	+ }
	199	+
	200	+ $prevPage = max($prevPage, $entry['page']);
	201	+
	202	+ if ( !isset( $entry['alpha'] ) ) {
	203	+ $entry['alpha'] = 'false';
	204	+ }
	205	+
	206	+ $entry['pixels'] = $entry['height'] * $entry['width'];
	207	+ $metadata['page_data'][$entry['page']] = $entry;
	208	+
	209	+ $entry = array();
	210	+ return true;
	211	+ }
	212	+
185	213	/**
186	214	* helper function of retrieveMetaData().
187	215	* parses shell return from tiffinfo-command into an array.
—	—	@@ -196,6 +224,8 @@
197	225
198	226	$entry = array();
199	227
	228	+ $prevPage = 0;
	229	+
200	230	foreach ( $rows as $row ) {
201	231	$row = trim( $row );
202	232
—	—	@@ -216,12 +246,12 @@
217	247	if ( $error ) continue;
218	248
219	249	if ( preg_match('/^TIFF Directory at/', $row) ) {
220		~~- if ( isset( $entry['page'] ) ) {~~
221		~~- $entry['pixels'] = $entry['height'] * $entry['width'];~~
222		~~- $data['page_data'][$entry['page'] +1] = $entry;~~
223		-
224		~~- $entry = array();~~
225		~~- $entry['alpha'] = 'false';~~
	250	+ if ( $entry ) {
	251	+ $ok = $this->addPageEntry($entry, $data, $prevPage);
	252	+ if ( !$ok ) {
	253	+ $error = true;
	254	+ continue;
	255	+ }
226	256	}
227	257	} else if ( preg_match('#^(TIFF.?Directory): (.?/.?): (.)#i', $row, $m) ) {
228	258	$bypass = false;
—	—	@@ -244,10 +274,9 @@
245	275
246	276	if ( $key == 'Page Number' && preg_match('/(\d+)-(\d+)/', $value, $m) ) {
247	277	$data['page_amount'] = (int)$m[2];
248		~~- $entry['page'] = (int)$m[1];~~
	278	+ $entry['page'] = (int)$m[1] +1;
249	279	} else if ( $key == 'Samples/Pixel' ) {
250	280	if ($value == '4') $entry['alpha'] = 'true';
251		~~- else $entry['alpha'] = 'false';~~
252	281	} else if ( $key == 'Extra samples' ) {
253	282	if (preg_match('.alpha.', $value)) $entry['alpha'] = 'true';
254	283	} else if ( $key == 'Image Width' \|\| $key == 'PixelXDimension' ) {
—	—	@@ -258,17 +287,21 @@
259	288	} else {
260	289	// strange line
261	290	}
	291	+
262	292	}
263	293
264		~~- if ( !empty( $entry['page'] ) ) {~~
265		~~- $entry['pixels'] = $entry['height'] * $entry['width'];~~
266		~~- $data['page_data'][$entry['page'] +1] = $entry;~~
	294	+ if ( $entry ) {
	295	+ $ok = $this->addPageEntry($entry, $data, $prevPage);
267	296	}
268	297
269	298	if ( !isset( $data['page_amount'] ) ) {
270	299	$data['page_amount'] = count( $data['page_data'] );
271	300	}
272	301
	302	+ if ( ! $data['page_data'] ) {
	303	+ $data['errors'][] = 'no page data found in tiff directory!';
	304	+ }
	305	+
273	306	return $data;
274	307	}
275	308
Property changes on: branches/wmf/1.16wmf4/extensions/PagedTiffHandler
___________________________________________________________________
Modified: svn:mergeinfo
276	309	Merged /trunk/extensions/PagedTiffHandler:r71621-71782

Past revisions this follows-up on

Revision	Commit summary	Author	Date
r71782	remove debug output. follow-up to r71698	daniel	08:10, 27 August 2010

Status & tagging log

19:30, 21 November 2010 Reedy (talk | contribs) changed the status of r71783 [removed: new added: deferred]