mirror of
https://github.com/commons-app/apps-android-commons.git
synced 2025-10-28 21:33:53 +01:00
Fix app stuck and memory issues while uploading images (#2287)
* Do not use an image array to store all bitmap pixels at once * Extract image preprocessing to a different service and use computation thread * Add java docs * Cleanup code to remove temp file logic * Add logs in upload flow * Fix tests * Fix more tests
This commit is contained in:
Vivek Maskara
neslihanturan
parent
21f82dd346
commit
559127dfa3
21 changed files with 320 additions and 891 deletions
|
|
@ -1,155 +0,0 @@
|
|||
package fr.free.nrw.commons.utils;
|
||||
|
||||
import android.content.Context;
|
||||
import android.net.Uri;
|
||||
|
||||
import java.io.File;
|
||||
import java.io.FileNotFoundException;
|
||||
import java.io.FileOutputStream;
|
||||
import java.io.IOException;
|
||||
import java.io.InputStream;
|
||||
import java.io.OutputStream;
|
||||
import java.util.Random;
|
||||
|
||||
import timber.log.Timber;
|
||||
|
||||
/**
|
||||
* This class includes utility methods for uploading process of images.
|
||||
*/
|
||||
|
||||
public class ContributionUtils {
|
||||
|
||||
private static String TEMP_EXTERNAL_DIRECTORY =
|
||||
android.os.Environment.getExternalStorageDirectory().getPath()+
|
||||
File.separatorChar+"UploadingByCommonsApp";
|
||||
|
||||
/**
|
||||
* Saves images temporarily to a fixed folder and use Uri of that file during upload process.
|
||||
* Otherwise, temporary Uri provided by content provider sometimes points to a null space and
|
||||
* consequently upload fails. See: issue #1400A and E.
|
||||
* Not: Saved image will be deleted, our directory will be empty after upload process.
|
||||
* @return URI of saved image
|
||||
*/
|
||||
public static Uri saveFileBeingUploadedTemporarily(Context context, Uri URIfromContentProvider) {
|
||||
// TODO add exceptions for Google Drive URİ is needed
|
||||
Uri result = null;
|
||||
|
||||
if (checkIfDirectoryExists(TEMP_EXTERNAL_DIRECTORY)) {
|
||||
String destinationFilename = decideTempDestinationFileName();
|
||||
result = saveFileFromURI(context, URIfromContentProvider, destinationFilename);
|
||||
} else { // If directory doesn't exist, create it and recursive call current method to check again
|
||||
|
||||
File file = new File(TEMP_EXTERNAL_DIRECTORY);
|
||||
if (file.mkdirs()) {
|
||||
Timber.d("saveFileBeingUploadedTemporarily() parameters: URI from Content Provider %s", URIfromContentProvider);
|
||||
result = saveFileBeingUploadedTemporarily(context, URIfromContentProvider); // If directory is created
|
||||
} else { //An error occurred to create directory
|
||||
Timber.e("saveFileBeingUploadedTemporarily() parameters: URI from Content Provider %s", URIfromContentProvider);
|
||||
}
|
||||
}
|
||||
return result;
|
||||
}
|
||||
|
||||
/**
|
||||
* Removes temp file created during upload
|
||||
* @param tempFileUri
|
||||
*/
|
||||
public static void removeTemporaryFile(Uri tempFileUri) {
|
||||
//TODO: do I have to notify file system about deletion?
|
||||
File tempFile = new File(tempFileUri.getPath());
|
||||
if (tempFile.exists()) {
|
||||
boolean isDeleted = tempFile.delete();
|
||||
Timber.e("removeTemporaryFile() parameters: URI tempFileUri %s, deleted status %b", tempFileUri, isDeleted);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Creates a temporary directory and returns pathname
|
||||
* @return
|
||||
*/
|
||||
private static String decideTempDestinationFileName() {
|
||||
int i = 0;
|
||||
while (new File(TEMP_EXTERNAL_DIRECTORY + File.separatorChar + i + "_tmp").exists()) {
|
||||
i++;
|
||||
}
|
||||
|
||||
// Use time stamp for file name, so that two temporary file never has same file name
|
||||
// to prevent previous file reference bug
|
||||
String timeStamp = String.valueOf(System.currentTimeMillis() / 1000);
|
||||
|
||||
// For multiple uploads, time randomisation should be combined with another random
|
||||
// parameter, since they created at same time
|
||||
int multipleUploadRandomParameter = new Random().nextInt(100);
|
||||
return TEMP_EXTERNAL_DIRECTORY + File.separatorChar + timeStamp + multipleUploadRandomParameter + "_tmp";
|
||||
}
|
||||
|
||||
/**
|
||||
* Empties files in Temporary Directory
|
||||
*/
|
||||
public static void emptyTemporaryDirectory() {
|
||||
File dir = new File(TEMP_EXTERNAL_DIRECTORY);
|
||||
if (dir.isDirectory()) {
|
||||
String[] children = dir.list();
|
||||
|
||||
if (children == null) return;
|
||||
|
||||
for (String child : children) {
|
||||
new File(dir, child).delete();
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Saves file from source URI to destination.
|
||||
* @param sourceUri Uri which points to file to be saved
|
||||
* @param destinationFilename where file will be located at
|
||||
* @return Uri points to file saved
|
||||
*/
|
||||
private static Uri saveFileFromURI(Context context, Uri sourceUri, String destinationFilename) {
|
||||
File file = new File(destinationFilename);
|
||||
if (file.exists()) {
|
||||
file.delete();
|
||||
}
|
||||
|
||||
InputStream in = null;
|
||||
OutputStream out = null;
|
||||
try {
|
||||
in = context.getContentResolver().openInputStream(sourceUri);
|
||||
out = new FileOutputStream(new File(destinationFilename));
|
||||
|
||||
byte[] buf = new byte[1024];
|
||||
int length;
|
||||
while ((length = in.read(buf)) > 0) {
|
||||
out.write(buf, 0, length);
|
||||
}
|
||||
} catch (FileNotFoundException e) {
|
||||
e.printStackTrace();
|
||||
} catch (IOException e) {
|
||||
e.printStackTrace();
|
||||
} finally {
|
||||
try {
|
||||
if (out != null) out.close();
|
||||
} catch (IOException e) {
|
||||
e.printStackTrace();
|
||||
}
|
||||
|
||||
try {
|
||||
if (in != null) in.close();
|
||||
} catch (IOException e) {
|
||||
e.printStackTrace();
|
||||
}
|
||||
}
|
||||
|
||||
return Uri.parse("file://" + destinationFilename);
|
||||
}
|
||||
|
||||
/**
|
||||
* Checks if directory exists
|
||||
* @param pathToCheck path of directory to check
|
||||
* @return true if directory exists, false otherwise
|
||||
*/
|
||||
private static boolean checkIfDirectoryExists(String pathToCheck) {
|
||||
File dir = new File(pathToCheck);
|
||||
return dir.exists() && dir.isDirectory();
|
||||
}
|
||||
}
|
||||
|
|
@ -128,44 +128,42 @@ public class ImageUtils {
|
|||
int bitmapHeight = bitmap.getHeight();
|
||||
|
||||
int allPixelsCount = bitmapWidth * bitmapHeight;
|
||||
int[] bitmapPixels = new int[allPixelsCount];
|
||||
Timber.d("total %s", Integer.toString(allPixelsCount));
|
||||
|
||||
bitmap.getPixels(bitmapPixels,0,bitmapWidth,0,0,bitmapWidth,bitmapHeight);
|
||||
int numberOfBrightPixels = 0;
|
||||
int numberOfMediumBrightnessPixels = 0;
|
||||
double brightPixelThreshold = 0.025*allPixelsCount;
|
||||
double mediumBrightPixelThreshold = 0.3*allPixelsCount;
|
||||
double brightPixelThreshold = 0.025 * allPixelsCount;
|
||||
double mediumBrightPixelThreshold = 0.3 * allPixelsCount;
|
||||
|
||||
for (int pixel : bitmapPixels) {
|
||||
int r = Color.red(pixel);
|
||||
int g = Color.green(pixel);
|
||||
int b = Color.blue(pixel);
|
||||
for (int x = 0; x < bitmapWidth; x++) {
|
||||
for (int y = 0; y < bitmapHeight; y++) {
|
||||
int pixel = bitmap.getPixel(x, y);
|
||||
int r = Color.red(pixel);
|
||||
int g = Color.green(pixel);
|
||||
int b = Color.blue(pixel);
|
||||
|
||||
int secondMax = r>g ? r:g;
|
||||
double max = (secondMax>b ? secondMax:b)/255.0;
|
||||
int secondMax = r > g ? r : g;
|
||||
double max = (secondMax > b ? secondMax : b) / 255.0;
|
||||
|
||||
int secondMin = r<g ? r:g;
|
||||
double min = (secondMin<b ? secondMin:b)/255.0;
|
||||
int secondMin = r < g ? r : g;
|
||||
double min = (secondMin < b ? secondMin : b) / 255.0;
|
||||
|
||||
double luminance = ((max+min)/2.0)*100;
|
||||
double luminance = ((max + min) / 2.0) * 100;
|
||||
|
||||
int highBrightnessLuminance = 40;
|
||||
int mediumBrightnessLuminance = 26;
|
||||
int highBrightnessLuminance = 40;
|
||||
int mediumBrightnessLuminance = 26;
|
||||
|
||||
if (luminance<highBrightnessLuminance){
|
||||
if (luminance>mediumBrightnessLuminance){
|
||||
numberOfMediumBrightnessPixels++;
|
||||
if (luminance < highBrightnessLuminance) {
|
||||
if (luminance > mediumBrightnessLuminance) {
|
||||
numberOfMediumBrightnessPixels++;
|
||||
}
|
||||
} else {
|
||||
numberOfBrightPixels++;
|
||||
}
|
||||
|
||||
if (numberOfBrightPixels >= brightPixelThreshold || numberOfMediumBrightnessPixels >= mediumBrightPixelThreshold) {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
else {
|
||||
numberOfBrightPixels++;
|
||||
}
|
||||
|
||||
if (numberOfBrightPixels>=brightPixelThreshold || numberOfMediumBrightnessPixels>=mediumBrightPixelThreshold){
|
||||
return false;
|
||||
}
|
||||
|
||||
}
|
||||
return true;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -5,6 +5,8 @@ import android.graphics.BitmapRegionDecoder;
|
|||
import javax.inject.Inject;
|
||||
import javax.inject.Singleton;
|
||||
|
||||
import io.reactivex.Single;
|
||||
import io.reactivex.schedulers.Schedulers;
|
||||
import fr.free.nrw.commons.location.LatLng;
|
||||
|
||||
import static fr.free.nrw.commons.utils.ImageUtils.*;
|
||||
|
|
@ -17,11 +19,18 @@ public class ImageUtilsWrapper {
|
|||
|
||||
}
|
||||
|
||||
public @Result int checkIfImageIsTooDark(BitmapRegionDecoder bitmapRegionDecoder) {
|
||||
return ImageUtils.checkIfImageIsTooDark(bitmapRegionDecoder);
|
||||
public Single<Integer> checkIfImageIsTooDark(BitmapRegionDecoder bitmapRegionDecoder) {
|
||||
int isImageDark = ImageUtils.checkIfImageIsTooDark(bitmapRegionDecoder);
|
||||
return Single.just(isImageDark)
|
||||
.subscribeOn(Schedulers.computation())
|
||||
.observeOn(Schedulers.computation());
|
||||
}
|
||||
|
||||
public boolean checkImageGeolocationIsDifferent(String geolocationOfFileString, LatLng latLng) {
|
||||
return ImageUtils.checkImageGeolocationIsDifferent(geolocationOfFileString, latLng);
|
||||
public Single<Integer> checkImageGeolocationIsDifferent(String geolocationOfFileString, LatLng latLng) {
|
||||
boolean isImageGeoLocationDifferent = ImageUtils.checkImageGeolocationIsDifferent(geolocationOfFileString, latLng);
|
||||
return Single.just(isImageGeoLocationDifferent)
|
||||
.subscribeOn(Schedulers.computation())
|
||||
.observeOn(Schedulers.computation())
|
||||
.map(isDifferent -> isDifferent ? ImageUtils.IMAGE_GEOLOCATION_DIFFERENT : ImageUtils.IMAGE_OK);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue