Thijs/wiki.techinc.nl
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Embed TinyRGB color profile when JPG EXIF Color Space = sRGB but no profile embedded

Browse source 
Existing srgb.jpg & tinyrgb.jpg have been replaced to be able to
easily compare a "fixed" missingprofile.jpg to tinyrgb.jpg.
With the existing files, when the tinyrgb profile was added to
missingprofile.jpg, it would end up basically the same as tinyrgb.jpg,
except that not all the exif data would be in the exact same order.
I've rebuilt srgb.jpg & tinyrgb.jpg by first removing their profile
(which is what missingprofile.jpg is), and then copying it over again:
    exiftool -tagsfromfile srgb.jpg -ICC_Profile new_srgb.jpg

Meanwhile also moved the profile-swapping code to JpegHandler, as it
was jpeg-specific.

Bug: T134498
Change-Id: I722dd6f66f6007182ad9a215e5eb382776983c05
This commit is contained in:
Matthias Mullie 2016-08-30 16:36:09 +02:00 committed by Matthias Mullie
parent d066d5d406
commit a428e3f067
8 changed files with 185 additions and 129 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

72
includes/media/ExifBitmap.php
View file

@ -30,7 +30,6 @@
class ExifBitmapHandler extends BitmapHandler {
const BROKEN_FILE = '-1'; // error extracting metadata
const OLD_BROKEN_FILE = '0'; // outdated error extracting metadata.
const SRGB_ICC_PROFILE_NAME = 'IEC 61966-2.1 Default RGB colour space - sRGB';
function convertMetadataVersion( $metadata, $version = 1 ) {
// basically flattens arrays.
@ -243,75 +242,4 @@ class ExifBitmapHandler extends BitmapHandler {
return 0;
}
protected function transformImageMagick( $image, $params ) {
global $wgUseTinyRGBForJPGThumbnails;
$ret = parent::transformImageMagick( $image, $params );
if ( $ret ) {
return $ret;
}
if ( $params['mimeType'] === 'image/jpeg' && $wgUseTinyRGBForJPGThumbnails ) {
// T100976 If the profile embedded in the JPG is sRGB, swap it for the smaller
// (and free) TinyRGB
$this->swapICCProfile(
$params['dstPath'],
self::SRGB_ICC_PROFILE_NAME,
realpath( __DIR__ ) . '/tinyrgb.icc'
);
}
return false;
}
/**
* Swaps an embedded ICC profile for another, if found.
* Depends on exiftool, no-op if not installed.
* @param string $filepath File to be manipulated (will be overwritten)
* @param string $oldProfileString Exact name of color profile to look for
* (the one that will be replaced)
* @param string $profileFilepath ICC profile file to apply to the file
* @since 1.26
* @return bool
*/
public function swapICCProfile( $filepath, $oldProfileString, $profileFilepath ) {
global $wgExiftool;
if ( !$wgExiftool || !is_executable( $wgExiftool ) ) {
return false;
}
$cmd = wfEscapeShellArg( $wgExiftool,
'-DeviceModelDesc',
'-S',
'-T',
$filepath
);
$output = wfShellExecWithStderr( $cmd, $retval );
if ( $retval !== 0 || strcasecmp( trim( $output ), $oldProfileString ) !== 0 ) {
// We can't establish that this file has the expected ICC profile, don't process it
return false;
}
$cmd = wfEscapeShellArg( $wgExiftool,
'-overwrite_original',
'-icc_profile<=' . $profileFilepath,
$filepath
);
$output = wfShellExecWithStderr( $cmd, $retval );
if ( $retval !== 0 ) {
$this->logErrorForExternalProcess( $retval, $output, $cmd );
return false;
}
return true;
}
}

116
includes/media/Jpeg.php
View file

@ -31,6 +31,8 @@
* @ingroup Media
*/
class JpegHandler extends ExifBitmapHandler {
const SRGB_EXIF_COLOR_SPACE = 'sRGB';
const SRGB_ICC_PROFILE_DESCRIPTION = 'sRGB IEC61966-2.1';
function normaliseParams( $image, &$params ) {
if ( !parent::normaliseParams( $image, $params ) ) {
@ -171,4 +173,118 @@ class JpegHandler extends ExifBitmapHandler {
return $params;
}
/**
* {@inheritdoc}
*/
protected function transformImageMagick( $image, $params ) {
global $wgUseTinyRGBForJPGThumbnails;
$ret = parent::transformImageMagick( $image, $params );
if ( $ret ) {
return $ret;
}
if ( $wgUseTinyRGBForJPGThumbnails ) {
// T100976 If the profile embedded in the JPG is sRGB, swap it for the smaller
// (and free) TinyRGB
/**
* We'll want to replace the color profile for JPGs:
* * in the sRGB color space, or with the sRGB profile
* (other profiles will be left untouched)
* * without color space or profile, in which case browsers
* should assume sRGB, but don't always do (e.g. on wide-gamut
* monitors (unless it's meant for low bandwith)
* @see https://phabricator.wikimedia.org/T134498
*/
$colorSpaces = [ self::SRGB_EXIF_COLOR_SPACE, '-' ];
$profiles = [ self::SRGB_ICC_PROFILE_DESCRIPTION ];
// we'll also add TinyRGB profile to images lacking a profile, but
// only if they're not low quality (which are meant to save bandwith
// and we don't want to increase the filesize by adding a profile)
if ( $params['quality'] > 30 ) {
$profiles[] = '-';
}
$this->swapICCProfile(
$params['dstPath'],
$colorSpaces,
$profiles,
realpath( __DIR__ ) . '/tinyrgb.icc'
);
}
return false;
}
/**
* Swaps an embedded ICC profile for another, if found.
* Depends on exiftool, no-op if not installed.
* @param string $filepath File to be manipulated (will be overwritten)
* @param array $colorSpaces Only process files with this/these Color Space(s)
* @param array $oldProfileStrings Exact name(s) of color profile to look for
* (the one that will be replaced)
* @param string $profileFilepath ICC profile file to apply to the file
* @since 1.26
* @return bool
*/
public function swapICCProfile( $filepath, array $colorSpaces,
array $oldProfileStrings, $profileFilepath
) {
global $wgExiftool;
if ( !$wgExiftool || !is_executable( $wgExiftool ) ) {
return false;
}
$cmd = wfEscapeShellArg( $wgExiftool,
'-EXIF:ColorSpace',
'-ICC_Profile:ProfileDescription',
'-S',
'-T',
$filepath
);
$output = wfShellExecWithStderr( $cmd, $retval );
// Explode EXIF data into an array with [0 => Color Space, 1 => Device Model Desc]
$data = explode( "\t", trim( $output ) );
if ( $retval !== 0 ) {
return false;
}
// Make a regex out of the source data to match it to an array of color
// spaces in a case-insensitive way
$colorSpaceRegex = '/'.preg_quote( $data[0], '/' ).'/i';
if ( empty( preg_grep( $colorSpaceRegex, $colorSpaces ) ) ) {
// We can't establish that this file matches the color space, don't process it
return false;
}
$profileRegex = '/'.preg_quote( $data[1], '/' ).'/i';
if ( empty( preg_grep( $profileRegex, $oldProfileStrings ) ) ) {
// We can't establish that this file has the expected ICC profile, don't process it
return false;
}
$cmd = wfEscapeShellArg( $wgExiftool,
'-overwrite_original',
'-icc_profile<=' . $profileFilepath,
$filepath
);
$output = wfShellExecWithStderr( $cmd, $retval );
if ( $retval !== 0 ) {
$this->logErrorForExternalProcess( $retval, $output, $cmd );
return false;
}
return true;
}
}

BIN
tests/phpunit/data/media/adobergb.jpg Normal file
View file

Binary file not shown.
Side by side

After

Width:  |  Height:  |  Size: 5.2 KiB

BIN
tests/phpunit/data/media/missingprofile.jpg Normal file
View file

Binary file not shown.
Side by side

After

Width:  |  Height:  |  Size: 4.5 KiB

BIN
tests/phpunit/data/media/srgb.jpg
View file

Binary file not shown.
Side by side Swipe Overlay

Before

Width:  |  Height:  |  Size: 7.6 KiB

After

Width:  |  Height:  |  Size: 7.6 KiB

BIN
tests/phpunit/data/media/tinyrgb.jpg
View file

Binary file not shown.
Side by side Swipe Overlay

Before

Width:  |  Height:  |  Size: 5 KiB

After

Width:  |  Height:  |  Size: 5 KiB

57
tests/phpunit/includes/media/ExifBitmapTest.php
View file

@ -142,61 +142,4 @@ class ExifBitmapTest extends MediaWikiMediaTestCase {
$res = $this->handler->convertMetadataVersion( $metadata, 1 );
$this->assertEquals( $expected, $res );
}
/**
* @dataProvider provideSwappingICCProfile
* @covers ExifBitmapHandler::swapICCProfile
*/
public function testSwappingICCProfile(
$sourceFilename, $controlFilename, $newProfileFilename, $oldProfileName
) {
global $wgExiftool;
if ( !$wgExiftool || !is_file( $wgExiftool ) ) {
$this->markTestSkipped( "Exiftool not installed, cannot test ICC profile swapping" );
}
$this->setMwGlobals( 'wgUseTinyRGBForJPGThumbnails', true );
$sourceFilepath = $this->filePath . $sourceFilename;
$controlFilepath = $this->filePath . $controlFilename;
$profileFilepath = $this->filePath . $newProfileFilename;
$filepath = $this->getNewTempFile();
copy( $sourceFilepath, $filepath );
$file = $this->dataFile( $sourceFilename, 'image/jpeg' );
$this->handler->swapICCProfile( $filepath, $oldProfileName, $profileFilepath );
$this->assertEquals(
sha1( file_get_contents( $filepath ) ),
sha1( file_get_contents( $controlFilepath ) )
);
}
public function provideSwappingICCProfile() {
return [
// File with sRGB should end up with TinyRGB
[
'srgb.jpg',
'tinyrgb.jpg',
'tinyrgb.icc',
'IEC 61966-2.1 Default RGB colour space - sRGB'
],
// File with TinyRGB should be left unchanged
[
'tinyrgb.jpg',
'tinyrgb.jpg',
'tinyrgb.icc',
'IEC 61966-2.1 Default RGB colour space - sRGB'
],
// File with no profile should be left unchanged
[
'test.jpg',
'test.jpg',
'tinyrgb.icc',
'IEC 61966-2.1 Default RGB colour space - sRGB'
]
];
}
}

69
tests/phpunit/includes/media/JpegTest.php
View file

@ -51,4 +51,73 @@ class JpegTest extends MediaWikiMediaTestCase {
$this->assertEquals( $res, $expected );
}
/**
* @dataProvider provideSwappingICCProfile
* @covers ExifBitmapHandler::swapICCProfile
*/
public function testSwappingICCProfile(
$sourceFilename, $controlFilename, $newProfileFilename, $oldProfileName
) {
global $wgExiftool;
if ( !$wgExiftool || !is_file( $wgExiftool ) ) {
$this->markTestSkipped( "Exiftool not installed, cannot test ICC profile swapping" );
}
$this->setMwGlobals( 'wgUseTinyRGBForJPGThumbnails', true );
$sourceFilepath = $this->filePath . $sourceFilename;
$controlFilepath = $this->filePath . $controlFilename;
$profileFilepath = $this->filePath . $newProfileFilename;
$filepath = $this->getNewTempFile();
copy( $sourceFilepath, $filepath );
$file = $this->dataFile( $sourceFilename, 'image/jpeg' );
$this->handler->swapICCProfile(
$filepath,
[ 'sRGB', '-' ],
[ $oldProfileName ],
$profileFilepath
);
$this->assertEquals(
sha1( file_get_contents( $filepath ) ),
sha1( file_get_contents( $controlFilepath ) )
);
}
public function provideSwappingICCProfile() {
return [
// File with sRGB should end up with TinyRGB
[
'srgb.jpg',
'tinyrgb.jpg',
'tinyrgb.icc',
'sRGB IEC61966-2.1'
],
// File with TinyRGB should be left unchanged
[
'tinyrgb.jpg',
'tinyrgb.jpg',
'tinyrgb.icc',
'sRGB IEC61966-2.1'
],
// File without profile should end up with TinyRGB
[
'missingprofile.jpg',
'tinyrgb.jpg',
'tinyrgb.icc',
'sRGB IEC61966-2.1'
],
// Non-sRGB file should be left untouched
[
'adobergb.jpg',
'adobergb.jpg',
'tinyrgb.icc',
'sRGB IEC61966-2.1'
]
];
}
}