Add smart image cropping with Computer Vision #149

johnwatkins0 · 2019-10-14T01:44:30Z

This PR introduces smart image cropping provided by Computer Vision.

It would resolve #112. I think #135 could integrate smart cropping without much additional effort.

Description of the Change

Sites are opted into the service through a checkbox on the image processing settings page. As an image is uploaded, each intermediate image size with cropping generates a request to the external service. If a cropped image is successfully returned, that image is saved and added to the image metadata in place of the version generated by WordPress (which remains in the file system).

Alternate Designs

I considered deleting the original non-smart-cropped file when Computer Vision successfully returns a cropped image. This would save server space. I'm still on the fence about it.
I also considered adding a checkbox group on the settings page allowing the feature to be opted into on the basis of specific image sizes, but this felt too complex. I did introduce a couple of filters that could be used to turn off smart-cropping specific image sizes.

Benefits

Thumbnails are nicely cropped around faces and other focal points.

Possible Drawbacks

A separate request needs to be made for each cropped image size. Only one core image size is cropped by default, but themes/plugins can register any number of cropped sizes. I think at some point the external requests around image processing are going to need to be done asynchronously rather than during pageload execution.

Verification Process

This only works on a publicly accessible site.

On the "Image Processing" Classifai settings page, with credentials in place, check the "Enable smart cropping" checkbox.
Upload a file to the media library. Useful files for testing are large portraits of people or similar photos in which the main focal point is significantly off center. The 150x150 thumbnail size is good for testing.
Upload the same file with the "Enable smart cropping" checkbox unchecked, and note the difference.
You might need to try a few files before you see a big difference.

Checklist:

I have read the CONTRIBUTING document.
My code follows the code style of this project.
My change requires a change to the documentation.
I have updated the documentation accordingly.
I have added tests to cover my change.
All new and existing tests passed.

johnwatkins0 · 2019-10-14T01:45:22Z

includes/Classifai/Helpers.php

+ *
+ * @return int
+ */
+function computer_vision_max_filesize() {


Moved these functions out of the ComputerVision class because they're now called from multiple places and don't use ComputerVision class state.

johnwatkins0 · 2019-10-14T02:10:11Z

includes/Classifai/Providers/Azure/SmartCropping.php

+		 * @param string The request URL with query args added.
+		 * @param array  Array containing the image height and width.
+		 */
+		do_action( 'classifai_smart_cropping_unsuccessful_response', $response, $url, $data );


Can use the failed response for #143

jeffpaul · 2019-10-14T14:55:32Z

In regards to Alternate Designs 1: "I considered deleting the original non-smart-cropped file when Computer Vision successfully returns a cropped image. This would save server space. I'm still on the fence about it.", I think I'm on the side of the fence of deleting the original non-smart-cropped file as I'm not sure there would be an easy way for a site owner to swap that back into place of the smart-cropped file and its not as though we're deleting the main source image itself.

jeffpaul · 2019-10-14T14:56:42Z

In regards to Alternate Designs 2: "I also considered adding a checkbox group on the settings page allowing the feature to be opted into on the basis of specific image sizes, but this felt too complex. I did introduce a couple of filters that could be used to turn off smart-cropping specific image sizes.", I agree with the lesser complexity of the single checkbox on the settings page and that providing filters seems the better route for flexibility.

rickalee · 2019-10-15T15:11:25Z

Hi team,

Did some initial tests with Azure Blob Storage to ensure that images could be smart cropped even when offloaded to external service.

Good news: Initial images under 4MB get smart cropped with Azure Blob Storage

If the initial image is over 4MB and needs to go through @filesize to check server side file size it does not get smart cropped.

cc @johnwatkins0 @jeffpaul

jeffpaul · 2019-10-15T15:13:17Z

@rickalee thanks for the testing and feedback... any immediate advice for @johnwatkins0 on tweaks to ensure that images that go through the 4mb file size checks properly get through the smart cropping?

rickalee · 2019-10-15T15:36:50Z

Add a check for file_exists when using get_attached_file() if not try wp_get_attachment_url()

Note: With Azure Blob Storage we sometimes proxy example.com/wp-content/uploads/ to Azure Blob so URLs returned from wp_get_attachment_url() may not obviously be Azure Blob. One alternative would be to add a filter to response headers to determine source e.g. x-ms-blob-type: AppendBlob

johnwatkins0 · 2019-10-15T15:59:04Z

Thanks, @rickalee . For testing purposes, do you use https://github.com/10up/windows-azure-storage to integrate Azure storage, or something else?

rickalee · 2019-10-15T16:03:21Z

@johnwatkins0 That plugin is correct.

johnwatkins0 · 2019-10-16T04:50:35Z

Hi, @rickalee, after taking a closer look, I think this can be solved easily. I am weighing a few options, though, and would welcome your thoughts (also CC the reviewers on this PR).

This issue is not with cropping in particular but rather the get_largest_acceptable_image_url function that is now used for two different Computer Vision features (and incidentally was copied into another 10up plugin recently). The function checks the full image size and, if it's too large, walks through the intermediate images and uses the biggest one under the limit.

For cases of remote storage, we need an alternative. Here's what I'm considering:

Don't provide the alternative functionality in this plugin, but provide a filter before get_largest_acceptable_image_url runs. Document the 4MB limit and point plugin/theme developers to the filter (e.g., in a README section specifically addressing remote storage). If the filter returns a file, run the remote request, and if it fails, it fails.
Create an alternative function handling the remote storage case, and provide a filter allowing other theme/plugin developers to specifically opt in at the code level. Opting in would be required.
Do Integrate Watson API for image recognition #2, but with a settings checkbox instead of a filter.
Create the alternative function and find a way to detect remote storage automatically. This could get complicated, because there are so many possible configurations, and multiple popular plugins handling remote storage. If we go with this option, I would suggest (cc @jeffpaul ) that this be a separate issue tackled later.

Thanks!

rickalee · 2019-10-16T14:31:38Z

@johnwatkins0 I think option #1 with filter before get_largest_acceptable_image_url is path of least resistance. Although I'm curious if we should look into 5.3 behavior with Big Images and try and add our hooks after the -2560 crop is generated. Anecdotally big image as is in 5.3 cut original 6.1MB image down to 589kb. https://make.wordpress.org/core/2019/10/09/introducing-handling-of-big-images-in-wordpress-5-3/

johnwatkins0 · 2019-10-16T14:47:34Z

Good idea, @rickalee . I'll see if that can be the solution for 5.3+, with the filter for pre-5.3.

…p/classifai into feature/smart-image-cropping

johnwatkins0 · 2020-01-17T21:57:24Z

@jeffpaul The need for smart cropping in WordPress has come up in some client conversations lately, including in one fast-moving project with a late-winter launch date. Is there anything I can do to help move this along and maybe get it into the plugin in time?

jeffpaul · 2020-01-21T18:27:56Z

@johnwatkins0 I'm hoping that we can finish up some other work here in January so that we shift focus to getting the ClassifAI 1.5 release out in February. Is that timing too slow for what you're hoping for?

@rickalee have you had a chance to review John's latest updates to see if they play well with Azure blob storage such that you can 👍 this PR?

johnwatkins0 · 2020-01-21T21:33:38Z

Thanks, @jeffpaul . February would give us plenty of time.

Ritesh-patel · 2020-01-22T08:18:57Z

includes/Classifai/Providers/Azure/SmartCropping.php

+			return false;
+		}
+
+		$data = array(


We should change it to php 5 array.

dinhtungdu · 2020-01-22T13:38:13Z

In my test, the PR works great without Azure Storage. But when I use Azure storage, there are still some issues:

The thumbnail (150x150) doesn't get smart cropped.
The alt doesn't generate with Keep local file unchecked.

dinhtungdu · 2020-01-22T14:35:37Z

includes/Classifai/Providers/Azure/ComputerVision.php

 		return true;
 	}

 	/**
 	 * Register the functionality.
 	 */
 	public function register() {
-		add_filter( 'wp_generate_attachment_metadata', [ $this, 'process_image' ], 10, 2 );
+		add_filter( 'wp_generate_attachment_metadata', [ $this, 'smart_crop_image' ], 10, 2 );
+		add_filter( 'wp_generate_attachment_metadata', [ $this, 'generate_image_alt_tags' ], 10, 2 );


In my test, changing these filters priority to 8 get image tagging and smart cropping work with Azure Storage which hooks to the same filter with priority set to 9.

jeffpaul · 2020-01-22T15:34:13Z

@johnwatkins0 are you able to work through the feedback from @Ritesh-patel and @dinhtungdu? If not, I'll see if someone else can help wrap up these so the PR is ready for the v1.5 release.

johnwatkins0 · 2020-01-22T16:04:10Z

@jeffpaul Yes, I can work on it over the next few days.

johnwatkins0 · 2020-01-26T21:20:16Z

Thanks, @dinhtungdu . I have pushed the suggested changes. In my test environment using Azure storage I have uploaded an image and verified that the 150x150 thumb is smart-cropped and that the alt text is generated. Let me know if you find any other issues.

dinhtungdu

The cropping function works flawlessly locally.

This PR doesn't work with Azure storage because of get_largest_acceptable_image_url(). Because that function exists before this request, we should work on the Azure storage issue in other PR.

dinhtungdu · 2020-01-28T03:29:32Z

includes/Classifai/Providers/Azure/ComputerVision.php

@@ -84,18 +115,19 @@ public function get_max_filesize() {
 	 *
 	 * @return mixed
 	 */
-	public function process_image( $metadata, $attachment_id ) {
+	public function generate_image_alt_tags( $metadata, $attachment_id ) {


This function is doing both alt and tag generation. IMO, we should

Use better name

Or even better, split the public method to do only one task.

@dinhtungdu just to add some context here. This function handles both items because the results of scan_image return both the captions and tags. The actual creation for tags and alts tags are done by separate functions called by this function. This is a long-winded way of saying that we save another call to the service by doing this all at once.

I'd be for a name change to the function to better reflect what it does though :)

dinhtungdu · 2020-01-28T04:04:03Z

includes/Classifai/Helpers.php

+ * @param int    $max        The maximum acceptable size.
+ * @return string|null The image URL, or null if no acceptable image found.
+ */
+function get_largest_acceptable_image_url( $full_image, $full_url, $sizes, $max = MB_IN_BYTES ) {


This function return NULL with Azure storage enabled and configured. Tested with publicly accessible WP install and newly uploaded images after configuring Azure.

ryanwelcher · 2020-02-18T12:30:47Z

tests/Classifai/HelpersTest.php

+		);
+		$this->assertNull( $url );
+
+		remove_filter( 'classifai_computer_vision_max_filesize', $set_1kb_max_filesize );


Small nitpick here, if the test fails, this filter won't be removed. I'd recommend moving it up before the $this->assertNull call.

ryanwelcher

LGTM!

ryanwelcher · 2020-02-18T12:37:58Z

@jeffpaul @dinhtungdu I think we can land this feature now and then focus on the Azure Storage integration as an immediate followup. Any concerns with that approach?

dinhtungdu · 2020-02-18T12:40:54Z

@ryanwelcher No concern, I agree!

johnwatkins0 added 6 commits October 11, 2019 19:49

Merge branch 'develop' of https://github.com/10up/classifai into develop

204c4c4

Add smart image cropping with Computer Vision

79e9754

Remove redundant hooks

07b1f97

Add unit test for image usort callback

e4eb5e0

Comment spacing fix

872a64b

Documentation updates

6928a93

johnwatkins0 commented Oct 14, 2019

View reviewed changes

johnwatkins0 added this to the 1.5.0 milestone Oct 14, 2019

johnwatkins0 added the type:enhancement New feature or request. label Oct 14, 2019

johnwatkins0 self-assigned this Oct 14, 2019

johnwatkins0 added 3 commits October 13, 2019 21:12

Typo fix

011e319

Remove unnecessary string interpolation

23a8ec2

Add filter to opt in/out of cropping for a specific image size

1dceeed

jeffpaul requested review from helen, ryanwelcher and adamsilverstein October 14, 2019 14:56

johnwatkins0 added 3 commits October 16, 2019 22:34

Add filter to override image URL to send for smart cropping

48eed55

Overwrite original thumbnail with smart-cropped copy

b899486

Merge branch 'feature/smart-image-cropping' of https://github.com/10u…

aff74ea

…p/classifai into feature/smart-image-cropping

johnwatkins0 added 6 commits December 11, 2019 20:47

Merge branch 'develop' of https://github.com/10up/classifai into develop

dce1113

Merge branch 'develop' into feature/smart-image-cropping

61c0eaf

Fix failing image size unit test

8395c81

Merge branch 'develop' of https://github.com/10up/classifai into develop

d1dadb2

Merge branch 'develop' into feature/smart-image-cropping

7d8f45a

Remove obsolete test

167d58c

Ritesh-patel reviewed Jan 22, 2020

View reviewed changes

dinhtungdu reviewed Jan 22, 2020

View reviewed changes

johnwatkins0 mentioned this pull request Jan 23, 2020

CLI Script to Smart-Crop existing images #176

Closed

johnwatkins0 added 2 commits January 26, 2020 14:25

Update array syntax

bb79263

Run smart crop filters earlier

43244ab

dinhtungdu requested review from dinhtungdu and removed request for adamsilverstein January 26, 2020 22:27

dinhtungdu reviewed Jan 28, 2020

View reviewed changes

Merge branch 'develop' into feature/smart-image-cropping

2e577b3

johnwatkins0 mentioned this pull request Feb 7, 2020

Add: service provider request response info to Site Health Info screen #180

Merged

6 tasks

ryanwelcher reviewed Feb 18, 2020

View reviewed changes

ryanwelcher approved these changes Feb 18, 2020

View reviewed changes

jeffpaul merged commit 0bf9489 into develop Feb 18, 2020

jeffpaul deleted the feature/smart-image-cropping branch March 5, 2020 01:05

jeffpaul mentioned this pull request Mar 9, 2020

Resolve compatibility issue with Microsoft Azure Storage #201

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add smart image cropping with Computer Vision #149

Add smart image cropping with Computer Vision #149

johnwatkins0 commented Oct 14, 2019

johnwatkins0 Oct 14, 2019 •

edited

Loading

johnwatkins0 Oct 14, 2019

jeffpaul commented Oct 14, 2019

jeffpaul commented Oct 14, 2019

rickalee commented Oct 15, 2019

jeffpaul commented Oct 15, 2019

rickalee commented Oct 15, 2019

johnwatkins0 commented Oct 15, 2019 •

edited

Loading

rickalee commented Oct 15, 2019

johnwatkins0 commented Oct 16, 2019 •

edited

Loading

rickalee commented Oct 16, 2019 •

edited

Loading

johnwatkins0 commented Oct 16, 2019

johnwatkins0 commented Jan 17, 2020 •

edited

Loading

jeffpaul commented Jan 21, 2020

johnwatkins0 commented Jan 21, 2020

Ritesh-patel Jan 22, 2020

dinhtungdu commented Jan 22, 2020

dinhtungdu Jan 22, 2020

jeffpaul commented Jan 22, 2020

johnwatkins0 commented Jan 22, 2020

johnwatkins0 commented Jan 26, 2020

dinhtungdu left a comment •

edited

Loading

dinhtungdu Jan 28, 2020

ryanwelcher Feb 10, 2020

dinhtungdu Jan 28, 2020

ryanwelcher Feb 18, 2020

ryanwelcher left a comment

ryanwelcher commented Feb 18, 2020

dinhtungdu commented Feb 18, 2020

Add smart image cropping with Computer Vision #149

Add smart image cropping with Computer Vision #149

Conversation

johnwatkins0 commented Oct 14, 2019

Description of the Change

Alternate Designs

Benefits

Possible Drawbacks

Verification Process

Checklist:

johnwatkins0 Oct 14, 2019 • edited Loading

Choose a reason for hiding this comment

johnwatkins0 Oct 14, 2019

Choose a reason for hiding this comment

jeffpaul commented Oct 14, 2019

jeffpaul commented Oct 14, 2019

rickalee commented Oct 15, 2019

jeffpaul commented Oct 15, 2019

rickalee commented Oct 15, 2019

johnwatkins0 commented Oct 15, 2019 • edited Loading

rickalee commented Oct 15, 2019

johnwatkins0 commented Oct 16, 2019 • edited Loading

rickalee commented Oct 16, 2019 • edited Loading

johnwatkins0 commented Oct 16, 2019

johnwatkins0 commented Jan 17, 2020 • edited Loading

jeffpaul commented Jan 21, 2020

johnwatkins0 commented Jan 21, 2020

Ritesh-patel Jan 22, 2020

Choose a reason for hiding this comment

dinhtungdu commented Jan 22, 2020

dinhtungdu Jan 22, 2020

Choose a reason for hiding this comment

jeffpaul commented Jan 22, 2020

johnwatkins0 commented Jan 22, 2020

johnwatkins0 commented Jan 26, 2020

dinhtungdu left a comment • edited Loading

Choose a reason for hiding this comment

dinhtungdu Jan 28, 2020

Choose a reason for hiding this comment

ryanwelcher Feb 10, 2020

Choose a reason for hiding this comment

dinhtungdu Jan 28, 2020

Choose a reason for hiding this comment

ryanwelcher Feb 18, 2020

Choose a reason for hiding this comment

ryanwelcher left a comment

Choose a reason for hiding this comment

ryanwelcher commented Feb 18, 2020

dinhtungdu commented Feb 18, 2020

johnwatkins0 Oct 14, 2019 •

edited

Loading

johnwatkins0 commented Oct 15, 2019 •

edited

Loading

johnwatkins0 commented Oct 16, 2019 •

edited

Loading

rickalee commented Oct 16, 2019 •

edited

Loading

johnwatkins0 commented Jan 17, 2020 •

edited

Loading

dinhtungdu left a comment •

edited

Loading