[image_picker] New feature : pick images with image quality #1896

AmolGangadhare · 2019-07-24T12:39:28Z

Description

Now, user can provide image quality (compression ratio) while picking an image.
The images will be compressed based on the input. 0 meaning compress for small size, 100 meaning compress for max quality.

Related Issues

flutter/flutter/issues/34523
flutter/flutter/issues/36072

Checklist

Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes ([x]). This will ensure a smooth and quick review process.

Breaking Change

Does your PR require plugin users to manually update their apps to accommodate your change?

Yes, this is a breaking change (please indicate a breaking change in CHANGELOG.md and increment major revision).
No, this is not a breaking change.

cyanglaz

Thank you so much for the contribution. This looks great overall! I left some comments below.

cyanglaz · 2019-07-24T16:10:46Z

packages/image_picker/ios/Classes/ImagePickerPlugin.m

    NSNumber *maxWidth = [_arguments objectForKey:@"maxWidth"];
    NSNumber *maxHeight = [_arguments objectForKey:@"maxHeight"];
+    NSNumber *imageQuality = [_arguments objectForKey:@"imageQuality"];
+


Maybe add

if (![ImageQuality isKindOfClass:[NSNumber class]]) { imageQuality = @1; }

This made the imageQuality an optional parameter and returns 1 to be default. Same on Android

cyanglaz · 2019-07-24T16:20:53Z

packages/image_picker/ios/Classes/FLTImagePickerMetaDataUtil.m

-                 userInfo:nil];
-  }
+
+  // Commenting as of now as JPEG conversion is done by default


We should still keep this check. However, we don't want to crash it anymore. Since the compressing on PNG with quality is not supported. We want to let the plugin users aware of it.
Something like

if (quality && type != FLTImagePickerMIMETypeJPEG) { NSLog(@"image_picker: compressing is not supported for type %@. Returning the image with original quality"); }

This is mainly to handle when plugin users choose to reduce quality but the user picked a PNG image.

cyanglaz · 2019-07-24T16:22:43Z

packages/image_picker/lib/image_picker.dart

  /// original width and height.
+  /// The [imageQuality] argument resize the image as per the input, ranging from 0-100 compression.
+  /// 0 meaning compress for small size, 100 meaning compress for max quality.
+  /// Some formats, like PNG which is loss less, will ignore the quality setting


Seems only apply to iOS? It seems Android can compress PNGs.

in android, for PNG(lossless formats) it will ignore the compression

ok, so you are saying Android behaves the same as iOS. Then we can also print a message from Android if user picked a PNG but the quality is set to lower than 100. I think it is a good info for developer when debugging or profiling.

yes, added a log in the Android part, with the same log as in iOS

cyanglaz · 2019-07-24T16:24:23Z

packages/image_picker/lib/image_picker.dart

      throw ArgumentError.value(maxHeight, 'maxHeight cannot be negative');
    }

+    if (null == imageQuality) {


let's make imageQuality optional to reduce unwanted computations.

assert(imageQuality == null || (imageQuality >= 0 && imageQuality <= 100));

do we need to make imageQuality mandatory? now if imageQuality gets null from a user, replacing it with 100 does not have any impact on image quality, it will return the image as it is.

No, we don't need to make the imageQuality mandatory. However, I'd like to keep the parameters sent to the platform consistent with what user has passed in the method call, and let the underlying platform handle the case when imageQuality is null.
So the method channel params will look like
{
source:"camera",
imageQuality:null,
}

And from the platform (iOS for example) .

NSNumber *quality = @(1); if ([argument[@"imageQuality"] isKindOfClass[NSNumber class]]) { quality = [argument[@"imageQuality] floatValue]/100; }

It is just my personal opinion. Your code works perfectly fine as it is now!

done, added checks in Android and iOS to check for null

cyanglaz · 2019-07-24T16:24:37Z

packages/image_picker/lib/image_picker.dart

+      imageQuality = 100;
+    }
+
+    if (imageQuality != null && (imageQuality < 0 || imageQuality > 100)) {


remove this "if" and use the assert I mentioned above.

cyanglaz · 2019-07-24T16:25:34Z

packages/image_picker/test/image_picker_test.dart

              'source': 0,
              'maxWidth': null,
              'maxHeight': null,
+              'imageQuality': 100


also add a test for imageQuality is null after making imageQuality an optional param

i've added the test for null condition

cyanglaz

Thanks for the quick turn around. Left some more comments.

cyanglaz · 2019-07-24T19:37:07Z

packages/image_picker/ios/Classes/FLTImagePickerMetaDataUtil.m

-  //                 userInfo:nil];
-  //  }
+  if (quality && type != FLTImagePickerMIMETypeJPEG) {
+    NSLog(@"image_picker: compressing is not supported for type %@. Returning the image with original quality");


add argument for type.

NSLog(@"image_picker: compressing is not supported for type %@. Returning the image with original quality", [FLTImagePickerMetaDataUtil imageTypeSuffixFromType:type]});

I typed this code on Github site so it might fail compilation

cyanglaz · 2019-07-24T19:38:02Z

packages/image_picker/ios/Classes/ImagePickerPlugin.m

+      imageQuality = @1;
+    }
+
    if (imageQuality.intValue < 0 || imageQuality.intValue > 100) {


make it else if. in iOS nil.intValue will return 0 which is not what we wanted.

cyanglaz

LGTM! CI seems to be complaining about the formatting. Also, please update pubspec and changelog with a new version! Thanks again!

cyanglaz

LGTM. Thank you! Version needs to be updated.

cyanglaz · 2019-07-25T15:56:13Z

packages/image_picker/CHANGELOG.md

@@ -1,3 +1,8 @@
+## 0.6.0+18


Should be 0.6.1, same in pubspec
https://dart.dev/tools/pub/versioning#semantic-versions

I've updated pubspec and changelog with a new version.

cyanglaz · 2019-07-29T21:25:43Z

It seems some java tests are failing.

AmolGangadhare · 2019-07-30T10:24:34Z

@cyanglaz I've merged code and updated test cases

cyanglaz

LGTM! I edited the doc a little bit hope you don't mind :)

AmolGangadhare · 2019-07-30T17:35:59Z

LGTM! I edited the doc a little bit hope you don't mind :)

Not at all. :)

…1896)

New feature : pick images with image quality

191991d

googlebot added the cla: yes label Jul 24, 2019

AmolGangadhare added 5 commits July 24, 2019 18:43

Updated test cases

cf66f1f

Code formmated

9d14063

Code format

064bcdf

Code format

9682397

Code format

3b63af5

AmolGangadhare mentioned this pull request Jul 24, 2019

image picker should be able to return a lower resolution JPEG flutter/flutter#36072

Closed

cyanglaz suggested changes Jul 24, 2019

View reviewed changes

cyanglaz self-assigned this Jul 24, 2019

cyanglaz added the submit queue The Flutter team is in the process of landing this PR. label Jul 24, 2019

Review changes

7dc8e96

cyanglaz reviewed Jul 24, 2019

View reviewed changes

Review changes

53c39dd

cyanglaz approved these changes Jul 24, 2019

View reviewed changes

Updated pubspec and changelog, test cases

aae056e

cyanglaz approved these changes Jul 25, 2019

View reviewed changes

Updated pubspec and changelog

56a0744

cyanglaz added in review and removed submit queue The Flutter team is in the process of landing this PR. labels Jul 29, 2019

AmolGangadhare added 5 commits July 30, 2019 13:47

Updated test cases, resolved conflicts with latest commit

983e38b

Merge branch 'master' into master

86d6304

Added checks in ImagePickerCache for imageQuality

bc6ce98

Merge branch 'master' of github.com:AmolGangadhare/plugins

b365806

Code format

9efaf38

Update image_picker.dart

4f09e62

cyanglaz approved these changes Jul 30, 2019

View reviewed changes

cyanglaz added the submit queue The Flutter team is in the process of landing this PR. label Jul 30, 2019

cyanglaz removed the in review label Jul 30, 2019

cyanglaz merged commit 153e5b6 into flutter:master Jul 30, 2019

This was referenced Jul 30, 2019

[image_picker] configurable JPEG compression flutter/flutter#34523

Closed

image_picker plugin should allow generation of thumbnail images flutter/flutter#26191

Open

[image_picker] add resize quality #855

Closed

mithun-mondal pushed a commit to bKash-developer/archived_plugins that referenced this pull request Aug 6, 2019

[image_picker] New feature : pick images with image quality (flutter#…

dfe0683

…1896)

cyanglaz mentioned this pull request Oct 17, 2019

[image_picker] GIF loses animation flutter/flutter#41568

Closed

[image_picker] New feature : pick images with image quality #1896

[image_picker] New feature : pick images with image quality #1896

Uh oh!

Conversation

AmolGangadhare commented Jul 24, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Related Issues

Checklist

Breaking Change

Uh oh!

cyanglaz left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cyanglaz left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cyanglaz left a comment

Choose a reason for hiding this comment

Uh oh!

cyanglaz left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cyanglaz commented Jul 29, 2019

Uh oh!

AmolGangadhare commented Jul 30, 2019

Uh oh!

cyanglaz left a comment

Choose a reason for hiding this comment

Uh oh!

AmolGangadhare commented Jul 30, 2019

Uh oh!

Reviewers

Assignees

Labels

AmolGangadhare commented Jul 24, 2019 •

edited

Loading

cyanglaz left a comment •

edited

Loading