-
Notifications
You must be signed in to change notification settings - Fork 85
Fix camera capture crash on devices with lower RAM #4635
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -27,6 +27,8 @@ | |
| import android.app.Activity; | ||
| import android.content.Context; | ||
| import android.content.Intent; | ||
| import android.content.pm.PackageManager; | ||
| import android.content.pm.ResolveInfo; | ||
| import android.util.Log; | ||
| import android.provider.MediaStore; | ||
| import android.graphics.Bitmap; | ||
|
|
@@ -40,6 +42,8 @@ | |
| public class CameraActivity extends Activity { | ||
| private static final String TAG = "Camera Activity"; | ||
| private static final int CAMERA_CODE = 102; | ||
| private static final String KEY_TARGET_PATH = "targetPath"; | ||
| private static final String KEY_CAMERA_FILE_PATH = "cameraFilePath"; | ||
|
|
||
| private String targetPath; | ||
| private File cameraFile; | ||
|
|
@@ -58,6 +62,15 @@ protected void onCreate(Bundle savedInstanceState) { | |
| orientationSensor = new OrientationSensor(mSensorManager, null); | ||
| orientationSensor.Register(this, SensorManager.SENSOR_DELAY_NORMAL); | ||
|
|
||
| if (savedInstanceState != null && savedInstanceState.containsKey(KEY_CAMERA_FILE_PATH)) { | ||
| // Process was killed and recreated while the camera app held the foreground. | ||
| // The capture is already in flight -- resume instead of relaunching it. | ||
| targetPath = savedInstanceState.getString(KEY_TARGET_PATH); | ||
| cameraFile = new File(savedInstanceState.getString(KEY_CAMERA_FILE_PATH)); | ||
| Log.d(TAG, "Resumed after process recreation, cameraFile: " + cameraFile.getAbsolutePath()); | ||
| return; | ||
| } | ||
|
|
||
| targetPath = getIntent().getExtras().getString("targetPath"); | ||
| Log.d(TAG, "targetPath: " + targetPath); | ||
|
|
||
|
|
@@ -77,7 +90,22 @@ protected void onCreate(Bundle savedInstanceState) { | |
| photoFile); | ||
|
|
||
| takePictureIntent.putExtra(MediaStore.EXTRA_OUTPUT, photoURI); | ||
| takePictureIntent.setFlags(Intent.FLAG_GRANT_WRITE_URI_PERMISSION | ||
| | Intent.FLAG_GRANT_READ_URI_PERMISSION); | ||
|
|
||
| // Explicitly grant URI permission to every app that can resolve this intent. | ||
| // Required because the URI is passed via EXTRA_OUTPUT rather than setData(), | ||
| // and some OEM camera apps (confirmed: Motorola) don't reliably honor the | ||
| // FLAG_GRANT_* flags in that case. | ||
| List<ResolveInfo> resolvedActivities = getPackageManager() | ||
| .queryIntentActivities(takePictureIntent, PackageManager.MATCH_DEFAULT_ONLY); | ||
| for (ResolveInfo resolveInfo : resolvedActivities) { | ||
| grantUriPermission(resolveInfo.activityInfo.packageName, photoURI, | ||
| Intent.FLAG_GRANT_WRITE_URI_PERMISSION | Intent.FLAG_GRANT_READ_URI_PERMISSION); | ||
| } | ||
|
|
||
| takePictureIntent.putExtra("__RESULT__", "takePictureIntent__RESULT__"); | ||
| startForegroundService(new Intent(this, CameraForegroundService.class)); | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shouldn't the |
||
| startActivityForResult(takePictureIntent, CAMERA_CODE); | ||
| } else { | ||
| Intent activityIntent = getIntent(); | ||
|
|
@@ -90,6 +118,15 @@ protected void onCreate(Bundle savedInstanceState) { | |
| return; | ||
| } | ||
|
|
||
| @Override | ||
| protected void onSaveInstanceState(Bundle outState) { | ||
| super.onSaveInstanceState(outState); | ||
| outState.putString(KEY_TARGET_PATH, targetPath); | ||
| if (cameraFile != null) { | ||
| outState.putString(KEY_CAMERA_FILE_PATH, cameraFile.getAbsolutePath()); | ||
| } | ||
| } | ||
|
|
||
| private File createImageFile(String targetPath) throws IOException { | ||
| // Create an image file name | ||
| String currentPhotoPath; | ||
|
|
@@ -115,7 +152,20 @@ protected void onActivityResult(int requestCode, int resultCode, Intent data) { | |
| Log.d(TAG, "resultCode: " + resultCode); | ||
| orientationSensor.Unregister(); | ||
|
|
||
| if (requestCode == CAMERA_CODE) { | ||
| // no-op if it isn't running (e.g. process was killed and recreated), stops it either way | ||
| stopService(new Intent(this, CameraForegroundService.class)); | ||
| } | ||
|
|
||
| if (requestCode == CAMERA_CODE && resultCode == Activity.RESULT_OK) { | ||
| if (cameraFile == null) { | ||
| Log.e(TAG, "cameraFile is null in onActivityResult - lost capture state."); | ||
| Intent resultData = getIntent(); | ||
| resultData.putExtra("__RESULT__", "Lost photo capture state."); | ||
| setResult(Activity.RESULT_CANCELED, resultData); | ||
| finish(); | ||
| return; | ||
| } | ||
| Log.d(TAG, "tmp exists: " + cameraFile.exists()); | ||
| Log.d(TAG, "tmp path: " + cameraFile.getAbsolutePath()); | ||
|
|
||
|
|
@@ -142,6 +192,23 @@ protected void onActivityResult(int requestCode, int resultCode, Intent data) { | |
| finish(); | ||
| } | ||
|
|
||
| @Override | ||
| protected void onDestroy() { | ||
| super.onDestroy(); | ||
| // no-op if it was already stopped in onActivityResult() or never started | ||
| stopService(new Intent(this, CameraForegroundService.class)); | ||
| if (cameraFile != null) { | ||
| try { | ||
| Uri photoURI = FileProvider.getUriForFile(this, | ||
| "uk.co.lutraconsulting.fileprovider", cameraFile); | ||
| revokeUriPermission(photoURI, | ||
| Intent.FLAG_GRANT_WRITE_URI_PERMISSION | Intent.FLAG_GRANT_READ_URI_PERMISSION); | ||
| } catch (IllegalArgumentException e) { | ||
| // cameraFile isn't covered by file_paths.xml -- nothing was granted, nothing to revoke | ||
| } | ||
| } | ||
|
Comment on lines
+200
to
+209
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would remove this as well |
||
| } | ||
|
|
||
| private void extendGPSExifData(long captureTime) { | ||
| int direction = getValueByTime(orientationSensor.m_azimuth_data, captureTime); | ||
| if (direction < 0) { | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,73 @@ | ||||||||||||||||||||||||
| /*************************************************************************** | ||||||||||||||||||||||||
| * * | ||||||||||||||||||||||||
| * This program is free software; you can redistribute it and/or modify * | ||||||||||||||||||||||||
| * it under the terms of the GNU General Public License as published by * | ||||||||||||||||||||||||
| * the Free Software Foundation; either version 2 of the License, or * | ||||||||||||||||||||||||
| * (at your option) any later version. * | ||||||||||||||||||||||||
| * * | ||||||||||||||||||||||||
| ***************************************************************************/ | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| package uk.co.lutraconsulting; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| import android.os.Build; | ||||||||||||||||||||||||
| import android.os.IBinder; | ||||||||||||||||||||||||
| import android.app.Service; | ||||||||||||||||||||||||
| import android.app.PendingIntent; | ||||||||||||||||||||||||
| import android.content.Intent; | ||||||||||||||||||||||||
| import android.content.pm.ServiceInfo; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| import android.app.Notification; | ||||||||||||||||||||||||
| import android.app.NotificationChannel; | ||||||||||||||||||||||||
| import android.app.NotificationManager; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||
| * Runs in the app's default process (unlike PositionTrackingService, which deliberately | ||||||||||||||||||||||||
| * runs in its own :trackingThread) for as long as CameraActivity is waiting on the external | ||||||||||||||||||||||||
| * camera app, to raise this process's priority and make it much less likely to be picked by | ||||||||||||||||||||||||
| * the OS's low-memory killer while the (often heavy) OEM camera stack is in the foreground. | ||||||||||||||||||||||||
| * This is a mitigation, not a guarantee -- under severe enough memory pressure the process can | ||||||||||||||||||||||||
| * still be killed, which is what CameraActivity's saved-instance-state resume handles. | ||||||||||||||||||||||||
|
Comment on lines
+24
to
+29
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||
| public class CameraForegroundService extends Service { | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| private static final String CHANNEL_ID = "CameraForegroundServiceChannel"; | ||||||||||||||||||||||||
| private static final int SERVICE_ID = 1011; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| @Override | ||||||||||||||||||||||||
| public IBinder onBind(Intent intent) { | ||||||||||||||||||||||||
| return null; | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| @Override | ||||||||||||||||||||||||
| public int onStartCommand(Intent intent, int flags, int startId) { | ||||||||||||||||||||||||
| NotificationChannel serviceChannel = new NotificationChannel( | ||||||||||||||||||||||||
| CHANNEL_ID, | ||||||||||||||||||||||||
| "Camera Foreground Service Channel", | ||||||||||||||||||||||||
| NotificationManager.IMPORTANCE_LOW | ||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| NotificationManager manager = getSystemService(NotificationManager.class); | ||||||||||||||||||||||||
| manager.createNotificationChannel(serviceChannel); | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| Intent notificationIntent = new Intent(this, MMActivity.class); | ||||||||||||||||||||||||
| PendingIntent pendingIntent = PendingIntent.getActivity(this, 0, notificationIntent, PendingIntent.FLAG_IMMUTABLE); | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| Notification notification = new Notification.Builder(this, CHANNEL_ID) | ||||||||||||||||||||||||
| .setSmallIcon(R.drawable.ic_notification) | ||||||||||||||||||||||||
| .setContentTitle("Waiting for photo") | ||||||||||||||||||||||||
| .setColor(getResources().getColor(R.color.grassColor)) | ||||||||||||||||||||||||
| .setContentIntent(pendingIntent) | ||||||||||||||||||||||||
| .build(); | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| if (Build.VERSION.SDK_INT >= 35) { // Android 15 (Vanilla Ice Cream) -- FOREGROUND_SERVICE_TYPE_SHORT_SERVICE | ||||||||||||||||||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||||||||
| startForeground(SERVICE_ID, notification, ServiceInfo.FOREGROUND_SERVICE_TYPE_SHORT_SERVICE); | ||||||||||||||||||||||||
| } else { | ||||||||||||||||||||||||
| startForeground(SERVICE_ID, notification); | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| // Not START_STICKY: if this service alone gets killed there is nothing useful to resume -- | ||||||||||||||||||||||||
| // it holds no state, it only exists to keep the process's priority elevated while | ||||||||||||||||||||||||
| // CameraActivity waits on startActivityForResult(). | ||||||||||||||||||||||||
| return START_NOT_STICKY; | ||||||||||||||||||||||||
|
Comment on lines
+68
to
+71
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have mixed feelings about this and it feels sloppy