r86567 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r86566‎ | r86567 | r86568 >
Date:23:15, 20 April 2011
Author:bawolff
Status:resolved (Comments)
Tags:
Comment:
Remove the JPEG/TIFF specific metadata code from BitmapHandler and put it in JpegOrTiff handler
to stop mostly irrelevent classes from getting it.

Also remove a method that is an exact duplicate of a base class (not sure whats with that).

This also coincidently fixes the issue with when a foreign file repo uses PagedTiffHandler
and the local one does not, and the builtin Tiff handler tries to treat the metadata as if
it was its own form.
Modified paths:
  • /trunk/phase3/includes/AutoLoader.php (modified) (history)
  • /trunk/phase3/includes/media/Bitmap.php (modified) (history)
  • /trunk/phase3/includes/media/Generic.php (modified) (history)
  • /trunk/phase3/includes/media/Jpeg.php (modified) (history)
  • /trunk/phase3/includes/media/JpegOrTiff.php (added) (history)
  • /trunk/phase3/includes/media/Tiff.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/media/Bitmap.php
——@@ -671,116 +671,6 @@
672672 }
673673
674674 /**
675 - * Its unclear if anything still uses this
676 - * as jpeg is now in its own subclass.
677 - *
678 - * And really each media handler should use a
679 - * different getMetadata, as the formats aren't
680 - * all that similar and usually have different
681 - * metadata needs.
682 - *
683 - * @deprecated
684 - */
685 - function getMetadata( $image, $filename ) {
686 - wfDeprecated( __METHOD__ );
687 - global $wgShowEXIF;
688 - if ( $wgShowEXIF && file_exists( $filename ) ) {
689 - $exif = new Exif( $filename );
690 - $data = $exif->getFilteredData();
691 - if ( $data ) {
692 - $data['MEDIAWIKI_EXIF_VERSION'] = Exif::version();
693 - return serialize( $data );
694 - } else {
695 - return '0';
696 - }
697 - } else {
698 - return '';
699 - }
700 - }
701 -
702 - function getMetadataType( $image ) {
703 - return 'exif';
704 - }
705 -
706 - function isMetadataValid( $image, $metadata ) {
707 - global $wgShowEXIF;
708 - if ( !$wgShowEXIF ) {
709 - # Metadata disabled and so an empty field is expected
710 - return true;
711 - }
712 - if ( $metadata === '0' ) {
713 - # Special value indicating that there is no EXIF data in the file
714 - return true;
715 - }
716 - wfSuppressWarnings();
717 - $exif = unserialize( $metadata );
718 - wfRestoreWarnings();
719 - if ( !isset( $exif['MEDIAWIKI_EXIF_VERSION'] ) ||
720 - $exif['MEDIAWIKI_EXIF_VERSION'] != Exif::version() )
721 - {
722 - # Wrong version
723 - wfDebug( __METHOD__ . ": wrong version\n" );
724 - return false;
725 - }
726 - return true;
727 - }
728 -
729 - /**
730 - * Get a list of EXIF metadata items which should be displayed when
731 - * the metadata table is collapsed.
732 - *
733 - * @return array of strings
734 - * @access private
735 - */
736 - function visibleMetadataFields() {
737 - $fields = array();
738 - $lines = explode( "\n", wfMsgForContent( 'metadata-fields' ) );
739 - foreach ( $lines as $line ) {
740 - $matches = array();
741 - if ( preg_match( '/^\\*\s*(.*?)\s*$/', $line, $matches ) ) {
742 - $fields[] = $matches[1];
743 - }
744 - }
745 - $fields = array_map( 'strtolower', $fields );
746 - return $fields;
747 - }
748 -
749 - /**
750 - * @param $image File
751 - * @return array|bool
752 - */
753 - function formatMetadata( $image ) {
754 - $result = array(
755 - 'visible' => array(),
756 - 'collapsed' => array()
757 - );
758 - $metadata = $image->getMetadata();
759 - if ( !$metadata ) {
760 - return false;
761 - }
762 - $exif = unserialize( $metadata );
763 - if ( !$exif ) {
764 - return false;
765 - }
766 - unset( $exif['MEDIAWIKI_EXIF_VERSION'] );
767 - $format = new FormatExif( $exif );
768 -
769 - $formatted = $format->getFormattedData();
770 - // Sort fields into visible and collapsed
771 - $visibleFields = $this->visibleMetadataFields();
772 - foreach ( $formatted as $name => $value ) {
773 - $tag = strtolower( $name );
774 - self::addMeta( $result,
775 - in_array( $tag, $visibleFields ) ? 'visible' : 'collapsed',
776 - 'exif',
777 - $tag,
778 - $value
779 - );
780 - }
781 - return $result;
782 - }
783 -
784 - /**
785675 * Try to read out the orientation of the file and return the angle that
786676 * the file needs to be rotated to be viewed
787677 *
Index: trunk/phase3/includes/media/Generic.php
——@@ -325,7 +325,7 @@
326326 * the metadata table is collapsed.
327327 *
328328 * @return array of strings
329 - * @access private
 329+ * @access protected
330330 */
331331 function visibleMetadataFields() {
332332 $fields = array();
Index: trunk/phase3/includes/media/Jpeg.php
——@@ -4,15 +4,16 @@
55 * @ingroup Media
66 */
77
8 -/** JPEG specific handler.
9 - * Inherits most stuff from BitmapHandler, just here to do the metadata handler differently
 8+/**
 9+ * JPEG specific handler.
 10+ * Inherits most stuff from BitmapHandler, just here to do the metadata handler differently.
 11+ *
 12+ * Metadata stuff common to Jpeg and built-in Tiff (not PagedTiffHandler) is in JpegOrTiffHandler.
 13+ *
1014 * @ingroup Media
1115 */
12 -class JpegHandler extends BitmapHandler {
 16+class JpegHandler extends JpegOrTiffHandler {
1317
14 - const BROKEN_FILE = '-1'; // error extracting metadata
15 - const OLD_BROKEN_FILE = '0'; // outdated error extracting metadata.
16 -
1718 function getMetadata ( $image, $filename ) {
1819 try {
1920 $meta = BitmapMetadataHandler::Jpeg( $filename );
——@@ -27,7 +28,7 @@
2829 // BitmapMetadataHandler throws an exception in certain exceptional cases like if file does not exist.
2930 wfDebug( __METHOD__ . ': ' . $e->getMessage() . "\n" );
3031
31 - /* This used to use 0 (self::OLD_BROKEN_FILE) for the cases
 32+ /* This used to use 0 (JpegOrTiffHandler::OLD_BROKEN_FILE) for the cases
3233 * * No metadata in the file
3334 * * Something is broken in the file.
3435 * However, if the metadata support gets expanded then you can't tell if the 0 is from
——@@ -36,103 +37,9 @@
3738 * Thus switch to using -1 to denote only a broken file, and use an array with only
3839 * MEDIAWIKI_EXIF_VERSION to denote no props.
3940 */
40 - return self::BROKEN_FILE;
 41+ return JpegOrTiffHandler::BROKEN_FILE;
4142 }
4243 }
4344
44 - function convertMetadataVersion( $metadata, $version = 1 ) {
45 - // basically flattens arrays.
46 - $version = explode(';', $version, 2);
47 - $version = intval($version[0]);
48 - if ( $version < 1 || $version >= 2 ) {
49 - return $metadata;
50 - }
51 -
52 - $avoidHtml = true;
53 -
54 - if ( !is_array( $metadata ) ) {
55 - $metadata = unserialize( $metadata );
56 - }
57 - if ( !isset( $metadata['MEDIAWIKI_EXIF_VERSION'] ) || $metadata['MEDIAWIKI_EXIF_VERSION'] != 2 ) {
58 - return $metadata;
59 - }
60 -
61 - // Treat Software as a special case because in can contain
62 - // an array of (SoftwareName, Version).
63 - if (isset( $metadata['Software'] )
64 - && is_array( $metadata['Software'] )
65 - && is_array( $metadata['Software'][0])
66 - && isset( $metadata['Software'][0][0] )
67 - && isset( $metadata['Software'][0][1])
68 - ) {
69 - $metadata['Software'] = $metadata['Software'][0][0] . ' (Version '
70 - . $metadata['Software'][0][1] . ')';
71 - }
72 -
73 - // ContactInfo also has to be dealt with specially
74 - if ( isset( $metadata['Contact'] ) ) {
75 - $metadata['Contact'] =
76 - FormatMetadata::collapseContactInfo(
77 - $metadata['Contact'] );
78 - }
79 -
80 - foreach ( $metadata as &$val ) {
81 - if ( is_array( $val ) ) {
82 - $val = FormatMetadata::flattenArray( $val, 'ul', $avoidHtml );
83 - }
84 - }
85 - $metadata['MEDIAWIKI_EXIF_VERSION'] = 1;
86 - return $metadata;
87 - }
88 -
89 - function isMetadataValid( $image, $metadata ) {
90 - global $wgShowEXIF;
91 - if ( !$wgShowEXIF ) {
92 - # Metadata disabled and so an empty field is expected
93 - return self::METADATA_GOOD;
94 - }
95 - if ( $metadata === self::OLD_BROKEN_FILE ) {
96 - # Old special value indicating that there is no EXIF data in the file.
97 - # or that there was an error well extracting the metadata.
98 - wfDebug( __METHOD__ . ": back-compat version\n");
99 - return self::METADATA_COMPATIBLE;
100 - }
101 - if ( $metadata === self::BROKEN_FILE ) {
102 - return self::METADATA_GOOD;
103 - }
104 - wfSuppressWarnings();
105 - $exif = unserialize( $metadata );
106 - wfRestoreWarnings();
107 - if ( !isset( $exif['MEDIAWIKI_EXIF_VERSION'] ) ||
108 - $exif['MEDIAWIKI_EXIF_VERSION'] != Exif::version() )
109 - {
110 - if ( isset( $exif['MEDIAWIKI_EXIF_VERSION'] ) &&
111 - $exif['MEDIAWIKI_EXIF_VERSION'] == 1 )
112 - {
113 - //back-compatible but old
114 - wfDebug( __METHOD__.": back-compat version\n" );
115 - return self::METADATA_COMPATIBLE;
116 - }
117 - # Wrong (non-compatible) version
118 - wfDebug( __METHOD__.": wrong version\n" );
119 - return self::METADATA_BAD;
120 - }
121 - return self::METADATA_GOOD;
122 - }
123 -
124 - function formatMetadata( $image ) {
125 - $metadata = $image->getMetadata();
126 - if ( !$metadata || $metadata == self::BROKEN_FILE ) {
127 - return false;
128 - }
129 - $exif = unserialize( $metadata );
130 - if ( !$exif ) {
131 - return false;
132 - }
133 - unset( $exif['MEDIAWIKI_EXIF_VERSION'] );
134 - if ( count( $exif ) == 0 ) {
135 - return false;
136 - }
137 - return $this->formatMetadataHelper( $exif );
138 - }
13945 }
 46+
Index: trunk/phase3/includes/media/Tiff.php
——@@ -11,7 +11,7 @@
1212 *
1313 * @ingroup Media
1414 */
15 -class TiffHandler extends BitmapHandler {
 15+class TiffHandler extends JpegOrTiffHandler {
1616
1717 /**
1818 * Conversion to PNG for inline display can be disabled here...
——@@ -34,4 +34,20 @@
3535 global $wgTiffThumbnailType;
3636 return $wgTiffThumbnailType;
3737 }
 38+
 39+ function getMetadata( $image, $filename ) {
 40+ global $wgShowEXIF;
 41+ if ( $wgShowEXIF && file_exists( $filename ) ) {
 42+ $exif = new Exif( $filename );
 43+ $data = $exif->getFilteredData();
 44+ if ( $data ) {
 45+ $data['MEDIAWIKI_EXIF_VERSION'] = Exif::version();
 46+ return serialize( $data );
 47+ } else {
 48+ return JpegOrTiffHandler::BROKEN_FILE;
 49+ }
 50+ } else {
 51+ return '';
 52+ }
 53+ }
3854 }
Index: trunk/phase3/includes/media/JpegOrTiff.php
——@@ -0,0 +1,123 @@
 2+<?php
 3+/**
 4+ * @file
 5+ * @ingroup Media
 6+ */
 7+
 8+/**
 9+ * Stuff specific to JPEG and (built-in) TIFF handler.
 10+ * All metadata related, since both JPEG and TIFF support Exif.
 11+ *
 12+ * @ingroup Media
 13+ */
 14+class JpegOrTiffHandler extends BitmapHandler {
 15+
 16+ const BROKEN_FILE = '-1'; // error extracting metadata
 17+ const OLD_BROKEN_FILE = '0'; // outdated error extracting metadata.
 18+
 19+ function convertMetadataVersion( $metadata, $version = 1 ) {
 20+ // basically flattens arrays.
 21+ $version = explode(';', $version, 2);
 22+ $version = intval($version[0]);
 23+ if ( $version < 1 || $version >= 2 ) {
 24+ return $metadata;
 25+ }
 26+
 27+ $avoidHtml = true;
 28+
 29+ if ( !is_array( $metadata ) ) {
 30+ $metadata = unserialize( $metadata );
 31+ }
 32+ if ( !isset( $metadata['MEDIAWIKI_EXIF_VERSION'] ) || $metadata['MEDIAWIKI_EXIF_VERSION'] != 2 ) {
 33+ return $metadata;
 34+ }
 35+
 36+ // Treat Software as a special case because in can contain
 37+ // an array of (SoftwareName, Version).
 38+ if (isset( $metadata['Software'] )
 39+ && is_array( $metadata['Software'] )
 40+ && is_array( $metadata['Software'][0])
 41+ && isset( $metadata['Software'][0][0] )
 42+ && isset( $metadata['Software'][0][1])
 43+ ) {
 44+ $metadata['Software'] = $metadata['Software'][0][0] . ' (Version '
 45+ . $metadata['Software'][0][1] . ')';
 46+ }
 47+
 48+ // ContactInfo also has to be dealt with specially
 49+ if ( isset( $metadata['Contact'] ) ) {
 50+ $metadata['Contact'] =
 51+ FormatMetadata::collapseContactInfo(
 52+ $metadata['Contact'] );
 53+ }
 54+
 55+ foreach ( $metadata as &$val ) {
 56+ if ( is_array( $val ) ) {
 57+ $val = FormatMetadata::flattenArray( $val, 'ul', $avoidHtml );
 58+ }
 59+ }
 60+ $metadata['MEDIAWIKI_EXIF_VERSION'] = 1;
 61+ return $metadata;
 62+ }
 63+
 64+ function isMetadataValid( $image, $metadata ) {
 65+ global $wgShowEXIF;
 66+ if ( !$wgShowEXIF ) {
 67+ # Metadata disabled and so an empty field is expected
 68+ return self::METADATA_GOOD;
 69+ }
 70+ if ( $metadata === self::OLD_BROKEN_FILE ) {
 71+ # Old special value indicating that there is no EXIF data in the file.
 72+ # or that there was an error well extracting the metadata.
 73+ wfDebug( __METHOD__ . ": back-compat version\n");
 74+ return self::METADATA_COMPATIBLE;
 75+ }
 76+ if ( $metadata === self::BROKEN_FILE ) {
 77+ return self::METADATA_GOOD;
 78+ }
 79+ wfSuppressWarnings();
 80+ $exif = unserialize( $metadata );
 81+ wfRestoreWarnings();
 82+ if ( !isset( $exif['MEDIAWIKI_EXIF_VERSION'] ) ||
 83+ $exif['MEDIAWIKI_EXIF_VERSION'] != Exif::version() )
 84+ {
 85+ if ( isset( $exif['MEDIAWIKI_EXIF_VERSION'] ) &&
 86+ $exif['MEDIAWIKI_EXIF_VERSION'] == 1 )
 87+ {
 88+ //back-compatible but old
 89+ wfDebug( __METHOD__.": back-compat version\n" );
 90+ return self::METADATA_COMPATIBLE;
 91+ }
 92+ # Wrong (non-compatible) version
 93+ wfDebug( __METHOD__.": wrong version\n" );
 94+ return self::METADATA_BAD;
 95+ }
 96+ return self::METADATA_GOOD;
 97+ }
 98+
 99+ function formatMetadata( $image ) {
 100+ $metadata = $image->getMetadata();
 101+ if ( !$metadata ||
 102+ $this->isMetadataValid( $image, $metadata ) === self::METADATA_BAD )
 103+ {
 104+ // So we don't try and display metadata from PagedTiffHandler
 105+ // for example when using InstantCommons.
 106+ return false;
 107+ }
 108+
 109+ $exif = unserialize( $metadata );
 110+ if ( !$exif ) {
 111+ return false;
 112+ }
 113+ unset( $exif['MEDIAWIKI_EXIF_VERSION'] );
 114+ if ( count( $exif ) == 0 ) {
 115+ return false;
 116+ }
 117+ return $this->formatMetadataHelper( $exif );
 118+ }
 119+
 120+ function getMetadataType( $image ) {
 121+ return 'exif';
 122+ }
 123+}
 124+
Property changes on: trunk/phase3/includes/media/JpegOrTiff.php
___________________________________________________________________
Added: svn:mergeinfo
1125 Merged /branches/REL1_15/phase3/includes/media/Jpeg.php:r51646
2126 Merged /branches/sqlite/includes/media/Jpeg.php:r58211-58321
3127 Merged /branches/new-installer/phase3/includes/media/Jpeg.php:r43664-66004
4128 Merged /branches/wmf-deployment/includes/media/Jpeg.php:r53381
Added: svn:eol-style
5129 + native
Index: trunk/phase3/includes/AutoLoader.php
——@@ -517,6 +517,7 @@
518518 'IPTC' => 'includes/media/IPTC.php',
519519 'JpegHandler' => 'includes/media/Jpeg.php',
520520 'JpegMetadataExtractor' => 'includes/media/JpegMetadataExtractor.php',
 521+ 'JpegOrTiffHandler' => 'includes/media/JpegOrTiff.php',
521522 'MediaHandler' => 'includes/media/Generic.php',
522523 'MediaTransformError' => 'includes/media/MediaTransformOutput.php',
523524 'MediaTransformOutput' => 'includes/media/MediaTransformOutput.php',

Follow-up revisions

RevisionCommit summaryAuthorDate
r90256(follow-up r86567) per CR rename the class JpegOrTiffHandler to ExifBitmapHan...bawolff03:37, 17 June 2011

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r86169Merge to trunk everything in img_metadata branch....bawolff01:23, 16 April 2011

Comments

#Comment by Brion VIBBER (talk | contribs)   00:47, 15 June 2011

'JpegOrTiffHandler' is kinda... odd, especially if it's got nothing JPEG or TIFF-specific. :) Perhaps ExifBitmapHandler would be more appropriate?

#Comment by Bawolff (talk | contribs)   03:41, 17 June 2011

yeah that is much better. Done in r90256.

resetting to new.

Status & tagging log