Problem/Motivation

Image field plugin should allow comparing more elements / details about the image added. We might follow: http://www.lars-tesmer.com/blog/2010/09/20/git---how-to-get-better-diffs... to have an example of a better diff.
We will need mockups to see how this will be implemented.

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

CommentFileSizeAuthor
#14 enrich_image_diff_based-2825763-14.patch3.42 KBsilvi.addweb
#13 interdiff-2825763-11-13.txt2.57 KBjohnchque
#13 enrich_image_diff_based-2825763-13.patch5.05 KBjohnchque
#11 interdiff-2825763-9-11.txt648 bytesjohnchque
#11 enrich_image_diff_based-2825763-11.patch4.67 KBjohnchque
#9 interdiff-2825763-7-9.txt3.76 KBjohnchque
#9 enrich_image_diff_based-2825763-9.patch4.67 KBjohnchque
#7 interdiff.2825763-5-7.txt1.36 KBjohnchque
#7 enrich_image_diff_based-2825763-7.patch4.8 KBjohnchque
#5 Screenshot from 2016-11-10 15-28-11.png39.84 KBjohnchque
#5 enrich_image_diff_based-2825763-5.patch3.35 KBjohnchque

Issue fork diff-2825763

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

yongt9412 created an issue. See original summary.

miro_dietiker’s picture

miro_dietiker
Location Switzerland
commented
Title: Improve image diff » Enrich image diff based on file metadata
miro_dietiker’s picture

miro_dietiker
Location Switzerland
commented
Component: Code » Diff builder plugins
miro_dietiker’s picture

miro_dietiker
Location Switzerland
commented

Important note, for media_entity_image, there is an API to extract exif. We should also integrate with that - could be a separate issue.
We should also check what wrapper it uses and not use a different one to keep common dependencies low.

johnchque’s picture

johnchque
Primary language Spanish
Location Bolivia
commented
Status: Active » Needs review
StatusFileSize
new enrich_image_diff_based-2825763-5.patch3.35 KB
new Screenshot from 2016-11-10 15-28-11.png39.84 KB

First try, trying to get as much info as we can. :)

Status: Needs review » Needs work

The last submitted patch, 5: enrich_image_diff_based-2825763-5.patch, failed testing.

johnchque’s picture

johnchque
Primary language Spanish
Location Bolivia
commented
Status: Needs work » Needs review
StatusFileSize
new enrich_image_diff_based-2825763-7.patch4.8 KB
new interdiff.2825763-5-7.txt1.36 KB

IMHO is better to add a text like (No title available) or something similar when wasn't set. So the diff can keep the same data in both sides. Just an idea. :) Fixing tests.

miro_dietiker’s picture

miro_dietiker
Location Switzerland
commented
Status: Needs review » Needs work 
  1. +++ b/config/schema/diff.schema.yml
@@ -112,6 +112,9 @@ diff.plugin.settings.image_field_diff_builder:
+      label: 'Compare extra image data'

+++ b/src/Plugin/diff/Field/ImageFieldBuilder.php
@@ -97,6 +111,13 @@ class ImageFieldBuilder extends FieldDiffBuilderBase {
+      '#description' => $this->t('Compare image data such as file size, MIME type, width and height.'),

    "extra image data" => "image metadata"
    See
    https://en.wikipedia.org/wiki/Metadata#Photographs 

  2. +++ b/src/Plugin/diff/Field/ImageFieldBuilder.php
@@ -43,7 +43,7 @@ class ImageFieldBuilder extends FieldDiffBuilderBase {
-          if (!empty($values['title'])) {
+          if (isset($values['title'])) {

    Then this would apply to both title and alt. 

  3. +++ b/src/Plugin/diff/Field/ImageFieldBuilder.php
@@ -55,6 +55,20 @@ class ImageFieldBuilder extends FieldDiffBuilderBase {
+            $result[$field_key][] = $this->t('File size: @size', ['@size' => $image->getSize()]);
+            $result[$field_key][] = $this->t('MIME Type: @mime', ['@mime' => $image->getMimeType()]);
...
+              $result[$field_key][] = $this->t('Width: @width', ['@width' => $values['width']]);

    You now explicitly pick the trivial metadata dimensions defined by the fileManager.
    The File size should use the converter for human readability, see format_size().

    This would be an intermediate state acceptable to commit and then continue in a follow-up with more detail, see below...

    The link above contains a reference to exiftool on JPG.
    Medias shows that instead, in PHP this would mean exif_read_data() like \Drupal\media_entity_image\Plugin\MediaEntity\Type\Image::getExifField implements.
    It seems exif_read_data() is always present, while for Image type we should use the getExifField api (separate specific plugin).

    Here two examples what this additionally contains, a png screenshot:

    $ exiftool Screenshot\ from\ 2016-08-14\ 00-57-37.png 
ExifTool Version Number         : 10.10
File Name                       : Screenshot from 2016-08-14 00-57-37.png
Directory                       : .
File Size                       : 3.1 kB
File Modification Date/Time     : 2016:08:14 00:57:41+02:00
File Access Date/Time           : 2016:10:17 15:57:43+02:00
File Inode Change Date/Time     : 2016:08:14 00:57:41+02:00
File Permissions                : rw-rw-r--
File Type                       : PNG
File Type Extension             : png
MIME Type                       : image/png
Image Width                     : 572
Image Height                    : 79
Bit Depth                       : 8
Color Type                      : RGB
Compression                     : Deflate/Inflate
Filter                          : Adaptive
Interlace                       : Noninterlaced
Significant Bits                : 8 8 8
Software                        : gnome-screenshot
Image Size                      : 572x79
Megapixels                      : 0.045

    And a real JPG example from a cheap panasonic cam:

    $ exiftool P1040001.JPG 
ExifTool Version Number         : 10.10
File Name                       : P1040001.JPG
Directory                       : .
File Size                       : 4.2 MB
File Modification Date/Time     : 2016:07:20 19:56:04+02:00
File Access Date/Time           : 2016:10:17 15:45:26+02:00
File Inode Change Date/Time     : 2016:08:10 11:23:03+02:00
File Permissions                : rw-r--r--
File Type                       : JPEG
File Type Extension             : jpg
MIME Type                       : image/jpeg
Exif Byte Order                 : Little-endian (Intel, II)
Make                            : Panasonic
Camera Model Name               : DMC-FT1
Orientation                     : Rotate 270 CW
X Resolution                    : 180
Y Resolution                    : 180
Resolution Unit                 : inches
Software                        : Ver.1.2
Modify Date                     : 2016:07:20 17:56:05
Y Cb Cr Positioning             : Co-sited
Exposure Time                   : 1/30
F Number                        : 3.3
Exposure Program                : Program AE
ISO                             : 100
Exif Version                    : 0221
Date/Time Original              : 2016:07:20 17:56:05
Create Date                     : 2016:07:20 17:56:05
Components Configuration        : Y, Cb, Cr, -
Compressed Bits Per Pixel       : 4
Exposure Compensation           : -0.66
Max Aperture Value              : 3.3
Metering Mode                   : Multi-segment
Light Source                    : Flash
Flash                           : Auto, Fired
Focal Length                    : 4.9 mm
Image Quality                   : High
Firmware Version                : 0.1.2.0
White Balance                   : Auto
Focus Mode                      : Auto
AF Area Mode                    : 1-area
Image Stabilization             : On, Mode 2
Macro Mode                      : Off
Shooting Mode                   : Snow
Audio                           : No
Data Dump                       : (Binary data 8200 bytes, use -b option to extract)
White Balance Bias              : 0
Flash Bias                      : 0
Internal Serial Number          : (F36) 2009:10:22 no. 0110
Panasonic Exif Version          : 0290
Color Effect                    : Off
Time Since Power On             : 00:00:59.31
Burst Mode                      : Off
Sequence Number                 : 0
Contrast Mode                   : Normal
Noise Reduction                 : Standard
Self Timer                      : Off
Rotation                        : Rotate 270 CW
AF Assist Lamp                  : Enabled but Not Used
Color Mode                      : Normal
Optical Zoom Mode               : Standard
Conversion Lens                 : Off
Travel Day                      : n/a
World Time Location             : Home
Program ISO                     : n/a
Advanced Scene Type             : 1
Faces Detected                  : 0
AF Point Position               : 0.5 0.5
Num Face Positions              : 0
Faces Recognized                : 0
Flash Warning                   : No
Maker Note Version              : 0131
Scene Mode                      : Snow
WB Red Level                    : 1616
WB Green Level                  : 1054
WB Blue Level                   : 2177
Flash Fired                     : Yes
Text Stamp                      : Off
Baby Age                        : (not set)
Transform                       : Off
Flashpix Version                : 0100
Color Space                     : sRGB
Exif Image Width                : 3264
Exif Image Height               : 2448
Interoperability Index          : R98 - DCF basic file (sRGB)
Interoperability Version        : 0100
Sensing Method                  : One-chip color area
File Source                     : Digital Camera
Scene Type                      : Directly photographed
Custom Rendered                 : Normal
Exposure Mode                   : Auto
Digital Zoom Ratio              : 0
Focal Length In 35mm Format     : 28 mm
Scene Capture Type              : Standard
Gain Control                    : None
Contrast                        : Normal
Saturation                      : Normal
Sharpness                       : Normal
PrintIM Version                 : 0250
Compression                     : JPEG (old-style)
Thumbnail Offset                : 10752
Thumbnail Length                : 6064
MPF Version                     : 0100
Number Of Images                : 2
MP Image Flags                  : Dependent child image
MP Image Format                 : JPEG
MP Image Type                   : Large Thumbnail (full HD equivalent)
MP Image Length                 : 888074
MP Image Start                  : 3494400
Dependent Image 1 Entry Number  : 0
Dependent Image 2 Entry Number  : 0
Image Width                     : 3264
Image Height                    : 2448
Encoding Process                : Baseline DCT, Huffman coding
Bits Per Sample                 : 8
Color Components                : 3
Y Cb Cr Sub Sampling            : YCbCr4:2:2 (2 1)
Advanced Scene Mode             : Snow
Aperture                        : 3.3
Blue Balance                    : 2.065465
Image Size                      : 3264x2448
Preview Image                   : (Binary data 888074 bytes, use -b option to extract)
Megapixels                      : 8.0
Red Balance                     : 1.533207
Scale Factor To 35 mm Equivalent: 5.7
Shutter Speed                   : 1/30
Thumbnail Image                 : (Binary data 6064 bytes, use -b option to extract)
Circle Of Confusion             : 0.005 mm
Field Of View                   : 65.5 deg
Focal Length                    : 4.9 mm (35 mm equivalent: 28.0 mm)
Hyperfocal Distance             : 1.38 m
Light Value                     : 8.4

    More modern cams would also add things like GPS coordinates.

    But this leads to a huge amount of data. There's also overlap between exif and filemanager.
    When a new image is added, all these lines would show up as +.

    To avoid overload, i would ask, if we can put that extended information into a collapsible extended fieldset while keeping the field data itself minimalistic. This problem could apply to any embedded entity such as through entity reference:
    On top level the entity label (added, changed, removed) might be fine.
    On click, the user might want to see the target fields and its deltas resolved.
    That could be already output and hidden in a fieldset (recursion?), or only resolved on button click via AJAX.

johnchque’s picture

johnchque
Primary language Spanish
Location Bolivia
commented
Status: Needs work » Needs review
StatusFileSize
new enrich_image_diff_based-2825763-9.patch4.67 KB
new interdiff-2825763-7-9.txt3.76 KB

Changes made based on comment above, will open a followup for implementing exif output.

Status: Needs review » Needs work

The last submitted patch, 9: enrich_image_diff_based-2825763-9.patch, failed testing.

johnchque’s picture

johnchque
Primary language Spanish
Location Bolivia
commented
Status: Needs work » Needs review
StatusFileSize
new enrich_image_diff_based-2825763-11.patch4.67 KB
new interdiff-2825763-9-11.txt648 bytes

Sorry, small leftover.

miro_dietiker’s picture

miro_dietiker
Location Switzerland
commented
Status: Needs review » Needs work 
  1. +++ b/src/Plugin/diff/Field/ImageFieldBuilder.php
@@ -55,6 +55,20 @@ class ImageFieldBuilder extends FieldDiffBuilderBase {
+            $result[$field_key][] = $this->t('File size: @size', ['@size' => format_size($image->getSize())]);
+            $result[$field_key][] = $this->t('MIME Type: @mime', ['@mime' => $image->getMimeType()]);
...
+              $result[$field_key][] = $this->t('Width: @width', ['@width' => $values['width']]);
...
+              $result[$field_key][] = $this->t('Height: @height', ['@height' => $values['height']]);

    Shouldn't we use keys for the concepts such as size, type, width, height instead of simply []? 

  2. +++ b/src/Tests/DiffPluginFileTest.php
@@ -204,7 +204,7 @@ class DiffPluginFileTest extends DiffPluginTestBase {
@@ -223,7 +223,7 @@ class DiffPluginFileTest extends DiffPluginTestBase {

    No tests yet? :-)

johnchque’s picture

johnchque
Primary language Spanish
Location Bolivia
commented
Status: Needs work » Needs review
StatusFileSize
new enrich_image_diff_based-2825763-13.patch5.05 KB
new interdiff-2825763-11-13.txt2.57 KB

Changes made, some tests added.

silvi.addweb’s picture

silvi.addweb commented
StatusFileSize
new enrich_image_diff_based-2825763-14.patch3.42 KB

re-roll patch #13 without test file changes.

acbramley’s picture

acbramley commented
Status: Needs review » Needs work

Please update to use an MR