Brion VIBBER has submitted this change and it was merged. Change subject: Allow mobile to reduce image quality ......................................................................
Allow mobile to reduce image quality http://www.mediawiki.org/wiki/Requests_for_comment/Reducing_image_quality_for_mobile Per above RFC, this patch implements the core changes required to specify quality reduction of JPEG via URL. To test, make sure your setup uses 404-based thumb generation. Vagrant supports it by adding "multimedia" role: $ vagrant add-role multimedia && vagrant provision * Pick any thumbnail jpeg URL that exists on the test wiki, e.g. http://.../images/thumb/4/49/Img.jpg/400px-Img.jpg * check that basic scaling works by altering 400 * add quality parameter qlow- right before px: http://.../images/thumb/4/49/Img.jpg/qlow-400px-Img.jpg Change-Id: I930ea06be6d302ffc8832d12b251422a9f1b3e75 --- M includes/Linker.php M includes/media/Bitmap.php M includes/media/Jpeg.php 3 files changed, 101 insertions(+), 18 deletions(-) Approvals: Brion VIBBER: Verified; Looks good to me, approved diff --git a/includes/Linker.php b/includes/Linker.php index cfa0158..9ec7944 100644 --- a/includes/Linker.php +++ b/includes/Linker.php @@ -530,7 +530,7 @@ * * @param array $handlerParams Associative array of media handler parameters, to be passed * to transform(). Typical keys are "width" and "page". - * @param string $time Timestamp of the file, set as false for current + * @param string|bool $time Timestamp of the file, set as false for current * @param string $query Query params for desc url * @param int|null $widthOption Used by the parser to remember the user preference thumbnailsize * @since 1.20 diff --git a/includes/media/Bitmap.php b/includes/media/Bitmap.php index 607c4e5..44be178 100644 --- a/includes/media/Bitmap.php +++ b/includes/media/Bitmap.php @@ -138,6 +138,10 @@ 'dstUrl' => $dstUrl, ); + if ( isset( $params['quality'] ) && $params['quality'] === 'low' ) { + $scalerParams['quality'] = 30; + } + # Determine scaler type $scaler = self::getScalerType( $dstPath ); @@ -147,6 +151,7 @@ if ( !$image->mustRender() && $scalerParams['physicalWidth'] == $scalerParams['srcWidth'] && $scalerParams['physicalHeight'] == $scalerParams['srcHeight'] + && !isset( $scalerParams['quality'] ) ) { # normaliseParams (or the user) wants us to return the unscaled image @@ -163,12 +168,14 @@ if ( $flags & self::TRANSFORM_LATER ) { wfDebug( __METHOD__ . ": Transforming later per flags.\n" ); - $params = array( + $newParams = array( 'width' => $scalerParams['clientWidth'], 'height' => $scalerParams['clientHeight'] ); - - return new ThumbnailImage( $image, $dstUrl, false, $params ); + if ( isset( $params['quality'] ) ) { + $newParams['quality'] = $params['quality']; + } + return new ThumbnailImage( $image, $dstUrl, false, $newParams ); } # Try to make a target path for the thumbnail @@ -235,12 +242,14 @@ } elseif ( $mto ) { return $mto; } else { - $params = array( + $newParams = array( 'width' => $scalerParams['clientWidth'], 'height' => $scalerParams['clientHeight'] ); - - return new ThumbnailImage( $image, $dstUrl, $dstPath, $params ); + if ( isset( $params['quality'] ) ) { + $newParams['quality'] = $params['quality']; + } + return new ThumbnailImage( $image, $dstUrl, $dstPath, $newParams ); } } @@ -314,7 +323,8 @@ $animation_post = array(); $decoderHint = array(); if ( $params['mimeType'] == 'image/jpeg' ) { - $quality = array( '-quality', '80' ); // 80% + $qualityVal = isset( $params['quality'] ) ? (string) $params['quality'] : null; + $quality = array( '-quality', $qualityVal ?: '80' ); // 80% # Sharpening, see bug 6193 if ( ( $params['physicalWidth'] + $params['physicalHeight'] ) / ( $params['srcWidth'] + $params['srcHeight'] ) @@ -419,7 +429,8 @@ list( $radius, $sigma ) = explode( 'x', $wgSharpenParameter ); $im->sharpenImage( $radius, $sigma ); } - $im->setCompressionQuality( 80 ); + $qualityVal = isset( $params['quality'] ) ? (string) $params['quality'] : null; + $im->setCompressionQuality( $qualityVal ?: 80 ); } elseif ( $params['mimeType'] == 'image/png' ) { $im->setCompressionQuality( 95 ); } elseif ( $params['mimeType'] == 'image/gif' ) { @@ -531,13 +542,14 @@ # input routine for this. $typemap = array( - 'image/gif' => array( 'imagecreatefromgif', 'palette', 'imagegif' ), - 'image/jpeg' => array( 'imagecreatefromjpeg', 'truecolor', + 'image/gif' => array( 'imagecreatefromgif', 'palette', false, 'imagegif' ), + 'image/jpeg' => array( 'imagecreatefromjpeg', 'truecolor', true, array( __CLASS__, 'imageJpegWrapper' ) ), - 'image/png' => array( 'imagecreatefrompng', 'bits', 'imagepng' ), - 'image/vnd.wap.wbmp' => array( 'imagecreatefromwbmp', 'palette', 'imagewbmp' ), - 'image/xbm' => array( 'imagecreatefromxbm', 'palette', 'imagexbm' ), + 'image/png' => array( 'imagecreatefrompng', 'bits', false, 'imagepng' ), + 'image/vnd.wap.wbmp' => array( 'imagecreatefromwbmp', 'palette', false, 'imagewbmp' ), + 'image/xbm' => array( 'imagecreatefromxbm', 'palette', false, 'imagexbm' ), ); + if ( !isset( $typemap[$params['mimeType']] ) ) { $err = 'Image type not supported'; wfDebug( "$err\n" ); @@ -545,7 +557,7 @@ return $this->getMediaTransformError( $params, $errMsg ); } - list( $loader, $colorStyle, $saveType ) = $typemap[$params['mimeType']]; + list( $loader, $colorStyle, $useQuality, $saveType ) = $typemap[$params['mimeType']]; if ( !function_exists( $loader ) ) { $err = "Incomplete GD library configuration: missing function $loader"; @@ -597,7 +609,12 @@ imagesavealpha( $dst_image, true ); - call_user_func( $saveType, $dst_image, $params['dstPath'] ); + $funcParams = array( $dst_image, $params['dstPath'] ); + if ( $useQuality && isset( $params['quality'] ) ) { + $funcParams[] = $params['quality']; + } + call_user_func_array( $saveType, $funcParams ); + imagedestroy( $dst_image ); imagedestroy( $src_image ); @@ -730,9 +747,10 @@ return $cache; } - static function imageJpegWrapper( $dst_image, $thumbPath ) { + // FIXME: transformImageMagick() & transformImageMagickExt() uses JPEG quality 80, here it's 95? + static function imageJpegWrapper( $dst_image, $thumbPath, $quality = 95 ) { imageinterlace( $dst_image ); - imagejpeg( $dst_image, $thumbPath, 95 ); + imagejpeg( $dst_image, $thumbPath, $quality ); } /** diff --git a/includes/media/Jpeg.php b/includes/media/Jpeg.php index 9ed626f..a0f7acb 100644 --- a/includes/media/Jpeg.php +++ b/includes/media/Jpeg.php @@ -31,6 +31,71 @@ * @ingroup Media */ class JpegHandler extends ExifBitmapHandler { + + function normaliseParams( $image, &$params ) { + if ( !parent::normaliseParams( $image, $params ) ) { + return false; + } + if ( isset( $params['quality'] ) && !self::validateQuality( $params['quality'] ) ) { + return false; + } + return true; + } + + function validateParam( $name, $value ) { + if ( $name === 'quality' ) { + return self::validateQuality( $value ); + } else { + return parent::validateParam( $name, $value ); + } + } + + /** Validate and normalize quality value to be between 1 and 100 (inclusive). + * @param int $value quality value, will be converted to integer or 0 if invalid + * @return bool true if the value is valid + */ + private static function validateQuality( $value ) { + return $value === 'low'; + } + + function makeParamString( $params ) { + // Prepend quality as "qValue-". This has to match parseParamString() below + $res = parent::makeParamString( $params ); + if ( $res && isset( $params['quality'] ) ) { + $res = "q{$params['quality']}-$res"; + } + return $res; + } + + function parseParamString( $str ) { + // $str contains "qlow-200px" or "200px" strings because thumb.php would strip the filename + // first - check if the string begins with "qlow-", and if so, treat it as quality. + // Pass the first portion, or the whole string if "qlow-" not found, to the parent + // The parsing must match the makeParamString() above + $res = false; + $m = false; + if ( preg_match( '/q([^-]+)-(.*)$/', $str, $m ) ) { + $v = $m[1]; + if ( self::validateQuality( $v ) ) { + $res = parent::parseParamString( $m[2] ); + if ( $res ) { + $res['quality'] = $v; + } + } + } else { + $res = parent::parseParamString( $str ); + } + return $res; + } + + function getScriptParams( $params ) { + $res = parent::getScriptParams( $params ); + if ( isset( $params['quality'] ) ) { + $res['quality'] = $params['quality']; + } + return $res; + } + function getMetadata( $image, $filename ) { try { $meta = BitmapMetadataHandler::Jpeg( $filename ); -- To view, visit https://gerrit.wikimedia.org/r/119661 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: merged Gerrit-Change-Id: I930ea06be6d302ffc8832d12b251422a9f1b3e75 Gerrit-PatchSet: 12 Gerrit-Project: mediawiki/core Gerrit-Branch: master Gerrit-Owner: Yurik <yu...@wikimedia.org> Gerrit-Reviewer: Aaron Schulz <asch...@wikimedia.org> Gerrit-Reviewer: Brian Wolff <bawolff...@gmail.com> Gerrit-Reviewer: Brion VIBBER <br...@wikimedia.org> Gerrit-Reviewer: Daniel Friesen <dan...@nadir-seen-fire.com> Gerrit-Reviewer: Dfoy <d...@wikimedia.org> Gerrit-Reviewer: Dr0ptp4kt <ab...@wikimedia.org> Gerrit-Reviewer: Eloquence <e...@wikimedia.org> Gerrit-Reviewer: MaxSem <maxsem.w...@gmail.com> Gerrit-Reviewer: Tim Starling <tstarl...@wikimedia.org> Gerrit-Reviewer: Yurik <yu...@wikimedia.org> Gerrit-Reviewer: jenkins-bot <> _______________________________________________ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
