[2.9 ACRA] crash adding bitmap to card

[mikehardy]

https://couchdb.ankidroid.org/acralyzer/_design/acralyzer/index.html#/report-details/47c1cab0-f57c-4162-aa09-f09d416bd11b

java.lang.RuntimeException: Failure delivering result ResultInfo{who=null, request=2, result=-1, data=Intent { act=inline-data (has extras) }} to activity {com.ichi2.anki/com.ichi2.anki.multimediacard.activity.MultimediaEditFieldActivity}: java.lang.NullPointerException: Attempt to invoke virtual method 'boolean android.graphics.Bitmap.compress(android.graphics.Bitmap$CompressFormat, int, java.io.OutputStream)' on a null object reference
at android.app.ActivityThread.deliverResults(ActivityThread.java:4382)
at android.app.ActivityThread.handleSendResult(ActivityThread.java:4426)
at android.app.ActivityThread.-wrap20(Unknown Source:0)
at android.app.ActivityThread$H.handleMessage(ActivityThread.java:1685)
at android.os.Handler.dispatchMessage(Handler.java:106)
at android.os.Looper.loop(Looper.java:164)
at android.app.ActivityThread.main(ActivityThread.java:6626)
at java.lang.reflect.Method.invoke(Native Method)
at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:438)
at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:811)
Caused by: java.lang.NullPointerException: Attempt to invoke virtual method 'boolean android.graphics.Bitmap.compress(android.graphics.Bitmap$CompressFormat, int, java.io.OutputStream)' on a null object reference
at com.ichi2.anki.multimediacard.fields.BasicImageFieldController.rotateAndCompress(BasicImageFieldController.java:6)
at com.ichi2.anki.multimediacard.fields.BasicImageFieldController.onActivityResult(BasicImageFieldController.java:9)
at com.ichi2.anki.multimediacard.activity.MultimediaEditFieldActivity.onActivityResult(MultimediaEditFieldActivity.java:3)
at android.app.Activity.dispatchActivityResult(Activity.java:7305)
at android.app.ActivityThread.deliverResults(ActivityThread.java:4378)
... 9 more

[Arthur-Milchior]

Is there a standard process to show a user an error message ?
Because I believe that the only thing which can reasonably done for this error is to state to the user that «the image could not be read as a bitmap file» (too technical ? «as an image» maybe ?) and that it failed.

The original problem is BitmapUtil#decodeFile, which returns null when it gets IOException, and this null is not taken into account

[timrae]

There's no standard process, but see #5561 for a good example

[david-allison]

Cause:

Anki-Android/AnkiDroid/src/main/java/com/ichi2/anki/multimediacard/fields/BasicImageFieldController.java

Lines 222 to 227 in b3d6cb3

	Bitmap b = BitmapUtil.decodeFile(f, IMAGE_SAVE_MAX_WIDTH);
	FileOutputStream out = null;
	try {
	out = new FileOutputStream(outPath);
	b = ExifUtil.rotateFromCamera(f, b);
	b.compress(Bitmap.CompressFormat.PNG, 90, out);

On Exception, BitmapUtil.decodeFile returns null.

Anki-Android/AnkiDroid/src/main/java/com/ichi2/utils/BitmapUtil.java

Lines 37 to 67 in b4361f4

	public static Bitmap decodeFile(File theFile, int IMAGE_MAX_SIZE) {
	Bitmap bmp = null;
	try {
	// Decode image size
	BitmapFactory.Options o = new BitmapFactory.Options();
	o.inJustDecodeBounds = true;
	FileInputStream fis = new FileInputStream(theFile);
	BitmapFactory.decodeStream(fis, null, o);
	fis.close();
	int scale = 1;
	if (o.outHeight > IMAGE_MAX_SIZE || o.outWidth > IMAGE_MAX_SIZE) {
	scale = (int) Math.pow(
	2,
	(int) Math.round(Math.log(IMAGE_MAX_SIZE / (double) Math.max(o.outHeight, o.outWidth))
	/ Math.log(0.5)));
	}
	// Decode with inSampleSize
	BitmapFactory.Options o2 = new BitmapFactory.Options();
	o2.inSampleSize = scale;
	fis = new FileInputStream(theFile);
	bmp = BitmapFactory.decodeStream(fis, null, o2);
	fis.close();
	} catch (IOException e) {
	// do nothing
	}
	return bmp;
	}

We can rotate a null, but we can't compress a null.

[david-allison]

Our context is that we're in rotateAndCompress, we have a file path, and we want to decode, shrink, rotate and compress the image, and return a file path to the new image.

If we can't generate a Bitmap, then we can't do any of that.

Proposal:

• Display a Toast stating that file compression failed
• Add the filesize to the screen.

[mikehardy]

@david-allison-1 that sounds fine for user notification (the toast), when you say "add filesize to the screen" do you mean to the toast? As in "Failed to compress 0 bytes" or similar? Wording might be better if it is not technical also, e.g. "Failed to add image of 0 bytes" the card. There will be the problem of making the quantity human-friendly perhaps? Failed to add 345,456,432 bytes may not be meaningful, kilobytes might be the best tradeoff for length / precision? But implementor's choice in many ways - I'm just thinking through it out loud on the first cup of coffee

[david-allison]

@mikehardy

I'd like the toast to display "Failed to compress image" and the screen to appear as such:

This gives the user the choice about what to do with the image, rather than completely failing.

[david-allison]

I was just testing this, and the compression made my images bigger. 1.46-> 1.78 MB, so wasn't really sure how to handle that.

[mikehardy]

Okay - what's the decision tree for the user? They see the toast and they're thinking what to do - if I'm the user I am now nervous to hit any of the buttons. If I save it is the image busted? Do I retake?

Also just in general in the area cropping (or I should say - allowing the option to farm out an image edit via Intent) was really close to done #5301

[david-allison]

This is one that I'm not too happy with, I'll have a think and get back to it later.

• Allowing the user to take images is better than failing the process if they can't be compressed.
• We don't know why the images are failing to be compressed, and we should get this information to see if there's a common case that we can avoid.
• We likely want to inform the user that the image may be larger than normal
  • An advanced user will understand that
  • With a less advanced user, I'm unsure.

[mikehardy]

We may gather information via sendExceptionReport for non fatal things, and you can also record analytics events (though they have less data) if you're interested in a feature's usage in general. Let me see about getting you access to acralyzer (for crashes/exception reports) and analytics (for feature usage) so you have the ability to analyze those

[david-allison]

OK, had a think: frame it as a warning about increasing the collection size for now, and we can deal with this much better once opening the file in an external application is supported.

New plans:

• Get a fix for the crash out
• If we get a failure, warn the user that the image is large, and the impact of that in a TextView without mentioning the compression failure.
• Add a logcat & exception report for the failure
• Add a follow-up issue to deal with improving usability further.

[mikehardy]

Sounds good