[TIMOB-25585] Android: MediaModule creates temporary files that don't delete on app exit.

[ypbnv]

JIRA: https://jira.appcelerator.org/browse/TIMOB-25585

Description:
Files created as a result from camera activity are never deleted when saveToPhotoGallery = false is set for Ti.Media.showCamera.
I think we are OK to create a TiBlob for the response without keeping a reference to the file ( creating the blob from byte array and deleting the temp file under the hood) in order to not bloat up the space used by the app.
Also added a missing break;.

Note: I am not sure if we can have a unit test for that. What do you think?

Test case:
app.js

var window = Ti.UI.createWindow(),
    buttonTakePhoto = Ti.UI.createButton({title: 'Take photo', top: 50}),
    buttonDeletePhotos = Ti.UI.createButton({title: 'Delete', top: 100}),
    buttonList = Ti.UI.createButton({title: 'List pictures', top: 150}),
    buttonClose = Ti.UI.createButton({title: 'Close', top: 200}),
    imageView = Ti.UI.createImageView({top: 250});

function takePhoto() {
    var imageBinary, imageFileCam;
    if (!Ti.Media.hasCameraPermissions()) {
        Ti.Media.requestCameraPermissions(function(e) {
          if (e.success === true) {
            takePhoto();
          } else {
            Ti.API.error('Camera permission error.');
          }
        });
    } else {
        Titanium.Media.showCamera({
          success : function(e) {
            imageBinary = e.media;
            imageFileCam = Titanium.Filesystem.getFile(Titanium.Filesystem.applicationDataDirectory, String.format('MyAppPhoto_%s.jpg', Date.now()));
            imageFileCam.write(imageBinary);
            imageView.image = e.media;
          },
          allowEditing : false,
          saveToPhotoGallery : false,
          mediaTypes : [ Ti.Media.MEDIA_TYPE_PHOTO ]
        });
    }
};

function logTempFolder() {
  Ti.API.info('----------logTempFolder----------');
  var f = Ti.Filesystem.getFile(Ti.Filesystem.applicationDataDirectory);
  var dir = f.getDirectoryListing();
  if (dir && dir.length > 0) {
    Ti.API.info('| logTempFolder -> number of files: ' + dir.length);
    dir.forEach(function(file) {
        Ti.API.info('File : ' + file);
    });
  } else {
    Ti.API.info('| logTempFolder -> No files present');
  }
  Ti.API.info('----------/logTempFolder----------');
}

function clearTempFiles_click(e) {
  var f = Ti.Filesystem.getFile(Ti.Filesystem.applicationDataDirectory);
  var dir = f.getDirectoryListing();

  dir.forEach(function(file) {
    if (file.startsWith("MyAppPhoto_")) {
      var imageFile = Titanium.Filesystem.getFile(
          Ti.Filesystem.applicationDataDirectory, file);
      imageFile.deleteFile();
    }
  });
}

function closeApp() {
  window.close();
}

buttonTakePhoto.addEventListener('click', takePhoto);
buttonList.addEventListener('click', logTempFolder);
buttonDeletePhotos.addEventListener('click', clearTempFiles_click);
buttonClose.addEventListener('click', closeApp);

window.add([buttonTakePhoto, buttonList, buttonDeletePhotos, imageView]);
window.open();

[garymathews]

This works great, but maybe we could make use of TiBlob.getBytes()?

TiBlob blob = TiBlob.blobFromFile(file);

if (!saveToPhotoGallery) {
    blob = TiBlob.blobFromData(blob.getBytes());
    imageFile.delete();
}

[jquick-axway]

@ypbnv, switching to a byte array like this will introduce a JPEG EXIF orientation bug. There are many Android devices whose camera will not automatically save the photo to file in an upright position. Instead, they'll save the photo relative to the camera's upright position and will save the orientation the user is holding the device as EXIF metadata instead.

Currently, we fetch the EXIF orientation via our TiImageHelper.getOrientation() method, but it only supports reading from a file path. We'll need to modify the code to support reading EXIF data from an InputStream as well...

import android.support.media.ExifInterface;

/**
 * Fetches the orientation of the given image in case it is not displayed upright.
 * <p>
 * This typically needs to be done with JPEG files whose EXIF information provides
 * the photo's "orientation" (aka: rotation) relative to the camera's mounting orientation.
 * @param path Path to an image file or URL.
 * @return
 * Returns the orientation of the image in degrees, clockwise.
 * <p>
 * Will only return values 0, 90, 180, and 270.
 * <p>
 * A value of 0 indicates that the image is upright or if this method was unable to fetch
 * orientation information from the image.
 */
public static int getOrientation(String path)
{
	// Validate argument.
	if ((path == null) || (path.length() <= 0)) {
		Log.e(TAG, "Path of image file could not determined. Could not create an exifInterface from an invalid path.");
		return 0;
	}

	// Attempt to fetch the EXIF orientation from the given file/url path.
	int orientation = 0;
	try (InputStream stream = TiFileHelper.getInstance().openInputStream(path, false)) {
		if (stream != null) {
			orientation = getOrientation(stream);
		}
	} catch (Exception ex) {
	}
	return orientation;
}

/**
 * Fetches the orientation of the given image in case it is not displayed upright.
 * <p>
 * This typically needs to be done with JPEG files whose EXIF information provides
 * the photo's "orientation" (aka: rotation) relative to the camera's upright orientation.
 * @param stream
 * An open input stream to an encoded image file, such as a JPEG.
 * <p>
 * This stream should not reference the raw decoded pixels of a bitmap since it would not
 * contain any EXIF orientation metadata.
 * <p>
 * This method will not close the given stream. That is the caller's responsibility.
 * @return
 * Returns the orientation of the image in degrees, clockwise.
 * <p>
 * Will only return values 0, 90, 180, and 270.
 * <p>
 * A value of 0 indicates that the image is upright or if this method was unable to fetch
 * orientation information from the image.
 */
public static int getOrientation(InputStream stream)
{
	int orientation = 0;
	try {
		if (stream != null) {
			ExifInterface exifInterface = new ExifInterface(stream);
			int exifOrientation = exifInterface.getAttributeInt(
					ExifInterface.TAG_ORIENTATION, ExifInterface.ORIENTATION_NORMAL);
			switch (exifOrientation) {
				case ExifInterface.ORIENTATION_ROTATE_270:
				case ExifInterface.ORIENTATION_TRANSVERSE:		// Rotated and mirrored.
					orientation = 270;
					break;
				case ExifInterface.ORIENTATION_ROTATE_180:
				case ExifInterface.ORIENTATION_FLIP_VERTICAL:
					orientation = 180;
					break;
				case ExifInterface.ORIENTATION_ROTATE_90:
				case ExifInterface.ORIENTATION_TRANSPOSE:		// Rotated and mirrored.
					orientation = 90;
					break;
			}
		}
	} catch (Exception ex) {
		Log.e(TAG, "Unable to find orientation " + ex.getMessage());
	}
	return orientation;
}

I've written and tested the above code a several months ago, but I've never had time to push it into a PR. The above requires you to add the "android-support-exifinterface.jar" support library which provides a back-ported version for Android 5.x and below.

[jquick-axway]

My only other concerns here are...

Memory Usage:
A high megapixel camera's JPEG photo with quality set to high can still consume a lot of memory. Even in encoded form. Like 2-3 megabytes. This also makes it a performance hit when reading in the bytes on the main UI thread. I'd prefer to keep photos saved to a temp directory and delete them later next time we restart the JavaScript runtime or terminate it. (Deleting files from storage is fast. It's the writes that are slow.)

Customer Expectations:
We also have customers who prefer that photos remain stored to file so that the app can process them later. For example, the JIRA comment here. This will make file cleanup a bit tricky if we do stick to saving photos to file. Personally, I would recommend that they call the photo file's move() method to move out of the temp directory so that it wouldn't get deleted later.

Thoughts?

[garymathews]

@jquick-axway I agree, we could just do this:

if (!saveToPhotoGallary) {
  TiFileHelper.getInstance().destroyOnExit(imageFile);
}

NOTE: I made a PR a while ago doing something similar here https://github.com/appcelerator/titanium_mobile/pull/9362/files

[ypbnv]

@jquick-axway, @garymathews In order to keep the file for referencing it later I changed the directory of the activity result file in the application cache instead the application data directory. The latter gets cleared after a dispose call which is not triggered if the user swipes the application from the task manager (common thing) or the system kills it to free up memory. We clear the cache after every new start of the application. I don't think we have a reliable callback for cleaning the cache just before losing the application/JS context.

[garymathews]

CR: PASS

[build]

	Messages
📖	💾 Here's the generated SDK zipfile.

Generated by 🚫 dangerJS

[ypbnv]

@jquick-axway I changed the approach towards the problem. Let me know if we are good to go for QE testing.

[jquick-axway]

CR: Pass

[ssjsamir]

FR Passed Was able to see that temporary files were no longer stored and did not require to be manually deleted.

Test Steps:

• Created a titanium project with the build form this PR
• Added the above code in to the project
• Ran the project
• Took a photo
• pressed "list pictures"
• saw tiaxxx.xxx temp files no longer being saved in the list (unlike inn 7.0.2.GA)

Test Environment
APPC Studio: 5.0.0.201711280737
APPC CLI: 7.0.1
Device: Nexus 6p (8.1.0)
Operating System Name: Mac OS High Sierra
Operating System Version: 10.13
Node.js Version: 8.9.1