diff --git a/lib/src/features/posting/providers/camera_provider.dart b/lib/src/features/posting/providers/camera_provider.dart index 6b33b07..ec0bf8e 100644 --- a/lib/src/features/posting/providers/camera_provider.dart +++ b/lib/src/features/posting/providers/camera_provider.dart @@ -14,8 +14,13 @@ part 'camera_provider.g.dart'; class Camera extends _$Camera { late final SparkLogger _logger; AppLifecycleListener? _lifecycleListener; + CameraController? _controller; bool _isFlippingCamera = false; + /// The controller is owned by this provider. Callers may borrow it for the + /// preview, but must not dispose it. + CameraController? get controller => _controller; + // Track if camera was disposed due to app lifecycle (not user navigation) bool _wasDisposedByLifecycle = false; @@ -23,10 +28,10 @@ class Camera extends _$Camera { FutureOr build(ResolutionPreset resolutionPreset) async { _logger = GetIt.instance().getLogger('Camera'); - // Only dispose lifecycle listener when provider is permanently disposed - // Don't use _disposeCamera here - we need to keep lifecycle listener - // active so we can detect when app returns to foreground - ref.onDispose(_disposeLifecycleListener); + ref.onDispose(() { + _disposeLifecycleListener(); + unawaited(_disposeOwnedCamera()); + }); // Listen to app lifecycle to pause/resume camera // Note: onHide is for app fully backgrounded (not transient inactive states) @@ -86,13 +91,9 @@ class Camera extends _$Camera { _logger.i('Found ${cameras.length} cameras'); - final controller = await _createCameraController(cameras.first); + await _createCameraController(cameras.first); - return CameraState( - controller: controller, - cameras: cameras, - isInitialized: true, - ); + return CameraState(cameras: cameras, isInitialized: true); } catch (e, stackTrace) { _logger.e( 'Camera initialization failed', @@ -113,15 +114,27 @@ class Camera extends _$Camera { resolutionPreset, imageFormatGroup: ImageFormatGroup.jpeg, ); + _controller = controller; - await controller.initialize(); + try { + await controller.initialize(); + + if (!ref.mounted) { + throw StateError('Camera provider disposed during initialization'); + } + if (controller.value.isInitialized) { + _logger.i('Camera controller successfully initialized'); + return controller; + } - if (controller.value.isInitialized) { - _logger.i('Camera controller successfully initialized'); - return controller; - } else { _logger.e('Camera controller initialization incomplete'); throw Exception('Camera controller initialized but camera not ready'); + } catch (_) { + if (identical(_controller, controller)) { + _controller = null; + } + await controller.dispose(); + rethrow; } } @@ -168,7 +181,7 @@ class Camera extends _$Camera { 0, currentState.cameras.length - 1, ); - final currentCamera = currentState.controller?.description; + final currentCamera = _controller?.description; final currentLensDirection = currentCamera?.lensDirection ?? currentState.cameras[fallbackIndex].lensDirection; @@ -199,7 +212,7 @@ class Camera extends _$Camera { _logger.d('Flipping camera'); final newCamera = currentState.cameras[newIndex]; - final controller = currentState.controller; + final controller = _controller; _isFlippingCamera = true; state = AsyncValue.data( currentState.copyWith(isFlipping: true, error: null), @@ -217,7 +230,6 @@ class Camera extends _$Camera { state = AsyncValue.data( currentState.copyWith( - controller: newController, selectedCameraIndex: newIndex, isInitialized: true, isFlipping: false, @@ -230,7 +242,6 @@ class Camera extends _$Camera { state = AsyncValue.data( currentState.copyWith( - controller: controller, selectedCameraIndex: newIndex, isInitialized: true, isFlipping: false, @@ -254,12 +265,13 @@ class Camera extends _$Camera { Future takePhoto() async { final currentState = state.value; + final controller = _controller; if (currentState == null) { _logger.w('Cannot take photo - no current state'); return null; } - if (currentState.controller == null || !currentState.isInitialized) { + if (controller == null || !currentState.isInitialized) { _logger.w('Cannot take photo - camera not initialized'); return null; } @@ -267,7 +279,7 @@ class Camera extends _$Camera { _logger.d('Taking photo'); try { - final file = await currentState.controller!.takePicture(); + final file = await controller.takePicture(); _logger.i('Photo taken successfully: ${file.path}'); return file; } catch (e, stackTrace) { @@ -279,12 +291,13 @@ class Camera extends _$Camera { Future startVideoRecording() async { final currentState = state.value; + final controller = _controller; if (currentState == null) { _logger.w('Cannot start recording - no current state'); return false; } - if (currentState.controller == null || + if (controller == null || !currentState.isInitialized || currentState.isRecording) { _logger.w( @@ -299,10 +312,8 @@ class Camera extends _$Camera { state = AsyncValue.data(currentState.copyWith(isRecording: true)); try { - await currentState.controller!.prepareForVideoRecording(); - await currentState.controller!.startVideoRecording( - enablePersistentRecording: true, - ); + await controller.prepareForVideoRecording(); + await controller.startVideoRecording(enablePersistentRecording: true); _logger.i('Video recording started successfully'); return true; } catch (e, stackTrace) { @@ -321,12 +332,13 @@ class Camera extends _$Camera { Future stopVideoRecording() async { final currentState = state.value; + final controller = _controller; if (currentState == null) { _logger.w('Cannot stop recording - no current state'); return null; } - if (currentState.controller == null || + if (controller == null || !currentState.isInitialized || !currentState.isRecording) { _logger.w('Cannot stop recording - not currently recording'); @@ -339,7 +351,7 @@ class Camera extends _$Camera { state = AsyncValue.data(currentState.copyWith(isRecording: false)); try { - final file = await currentState.controller!.stopVideoRecording(); + final file = await controller.stopVideoRecording(); _logger.i('Video recording stopped successfully: ${file.path}'); return file; } catch (e, stackTrace) { @@ -356,14 +368,18 @@ class Camera extends _$Camera { Future _disposeCamera() async { _logger.d('Disposing camera'); + if (!ref.mounted) { + await _disposeOwnedCamera(); + return; + } + try { final currentState = state.value; - final controller = currentState?.controller; - final wasRecording = currentState?.isRecording ?? false; + final controller = _controller; + _controller = null; state = AsyncValue.data( currentState?.copyWith( - controller: null, isInitialized: false, isRecording: false, isFlipping: false, @@ -373,32 +389,47 @@ class Camera extends _$Camera { if (controller != null) { await _waitForPreviewDetach(); + await _disposeController(controller); + } + } catch (e, stackTrace) { + _logger.e('Error disposing camera', error: e, stackTrace: stackTrace); + } + } - if (wasRecording) { - _logger.d('Stopping recording before disposal'); - try { - if (controller.value.isRecordingVideo) { - await controller.stopVideoRecording(); - } - } catch (e, stackTrace) { - _logger.e( - 'Error stopping recording during disposal', - error: e, - stackTrace: stackTrace, - ); - } - } + Future _disposeOwnedCamera() async { + final controller = _controller; + _controller = null; + if (controller != null) { + await _disposeController(controller); + } + } - await controller.dispose(); - _logger.i('Camera controller disposed successfully'); + Future _disposeController(CameraController controller) async { + if (controller.value.isRecordingVideo) { + _logger.d('Stopping recording before disposal'); + try { + await controller.stopVideoRecording(); + } catch (e, stackTrace) { + _logger.e( + 'Error stopping recording during disposal', + error: e, + stackTrace: stackTrace, + ); } + } + + try { + await controller.dispose(); + _logger.i('Camera controller disposed successfully'); } catch (e, stackTrace) { - _logger.e('Error disposing camera', error: e, stackTrace: stackTrace); + _logger.e( + 'Error disposing camera controller', + error: e, + stackTrace: stackTrace, + ); } } - /// Disposes the lifecycle listener. Should only be called when provider - /// is being permanently disposed (not for lifecycle pauses). void _disposeLifecycleListener() { _lifecycleListener?.dispose(); _lifecycleListener = null; diff --git a/lib/src/features/posting/providers/camera_state.dart b/lib/src/features/posting/providers/camera_state.dart index cb8b187..c93cca0 100644 --- a/lib/src/features/posting/providers/camera_state.dart +++ b/lib/src/features/posting/providers/camera_state.dart @@ -6,7 +6,6 @@ part 'camera_state.freezed.dart'; @freezed abstract class CameraState with _$CameraState { const factory CameraState({ - CameraController? controller, @Default([]) List cameras, @Default(0) int selectedCameraIndex, @Default(false) bool isInitialized, diff --git a/lib/src/features/posting/ui/pages/recording_page.dart b/lib/src/features/posting/ui/pages/recording_page.dart index 9282832..cf062b1 100644 --- a/lib/src/features/posting/ui/pages/recording_page.dart +++ b/lib/src/features/posting/ui/pages/recording_page.dart @@ -113,11 +113,12 @@ class _RecordingPageState extends ConsumerState { bool _isCameraReady() { final cameraAsync = ref.read(_cameraProvider); + final controller = ref.read(_cameraProvider.notifier).controller; if (cameraAsync.hasError) return false; final cameraState = cameraAsync.value; return cameraState != null && cameraState.isInitialized && - cameraState.controller != null && + controller != null && cameraState.cameras.isNotEmpty; } @@ -236,16 +237,15 @@ class _RecordingPageState extends ConsumerState { } try { + await ref.read(_cameraProvider.notifier).disposeCamera(); + if (!mounted) return; + await context.router.push( ImageReviewRoute(imageFiles: photos, storyMode: widget.storyMode), ); if (!mounted) return; - - setState(() { - _isProcessing = false; - }); - await ref.read(_cameraProvider.notifier).reinitializeCamera(); + await _resumeCameraAfterPhotoFlow(); } catch (e, stackTrace) { _logger.e( 'Error processing multiple photos', @@ -253,9 +253,8 @@ class _RecordingPageState extends ConsumerState { stackTrace: stackTrace, ); if (mounted) { - setState(() { - _isProcessing = false; - }); + await _resumeCameraAfterPhotoFlow(); + if (!mounted) return; ScaffoldMessenger.of(context).showSnackBar( SnackBar( content: Text( @@ -271,6 +270,9 @@ class _RecordingPageState extends ConsumerState { if (!mounted) return; try { + await ref.read(_cameraProvider.notifier).disposeCamera(); + if (!mounted) return; + // Open the story image editor final editedImage = await GetIt.I() .openStoryImageEditor(context, photoFile); @@ -323,20 +325,12 @@ class _RecordingPageState extends ConsumerState { } } - // Reset processing state and reinitialize camera - if (mounted) { - setState(() { - _isProcessing = false; - }); - // Reinitialize camera after returning from editor - ref.read(_cameraProvider.notifier).reinitializeCamera(); - } + await _resumeCameraAfterPhotoFlow(); } catch (e, stackTrace) { _logger.e('Error processing photo', error: e, stackTrace: stackTrace); if (mounted) { - setState(() { - _isProcessing = false; - }); + await _resumeCameraAfterPhotoFlow(); + if (!mounted) return; ScaffoldMessenger.of(context).showSnackBar( SnackBar( content: Text( @@ -348,6 +342,15 @@ class _RecordingPageState extends ConsumerState { } } + Future _resumeCameraAfterPhotoFlow() async { + if (!mounted) return; + + setState(() { + _isProcessing = false; + }); + await ref.read(_cameraProvider.notifier).reinitializeCamera(); + } + void _startRecording() { unawaited(_startRecordingAsync()); } @@ -872,6 +875,8 @@ class _RecordingPageState extends ConsumerState { return cameraAsync.when( data: (cameraState) { + final controller = ref.read(_cameraProvider.notifier).controller; + if (cameraState.error != null) { return Scaffold( backgroundColor: Colors.black, @@ -926,7 +931,7 @@ class _RecordingPageState extends ConsumerState { } // No cameras available - show placeholder with library picker - if (!hasCameras || cameraState.controller == null) { + if (!hasCameras || controller == null) { return RecordingPageTemplate( cameraPreview: Container( color: Colors.black, @@ -976,7 +981,7 @@ class _RecordingPageState extends ConsumerState { availableLensDirections.contains(CameraLensDirection.back) && !_isStartingRecording && !cameraState.isFlipping; - final aspectRatio = cameraState.controller!.value.aspectRatio; + final aspectRatio = controller.value.aspectRatio; final canFinalizeSession = recordingState.canFinalize && !_isProcessing && @@ -995,9 +1000,7 @@ class _RecordingPageState extends ConsumerState { : _handleTap; return RecordingPageTemplate( - cameraPreview: RepaintBoundary( - child: CameraPreview(cameraState.controller!), - ), + cameraPreview: RepaintBoundary(child: CameraPreview(controller)), aspectRatio: aspectRatio, isRecording: recordingState.isRecording, elapsedDuration: recordingState.elapsedDuration, diff --git a/pubspec.lock b/pubspec.lock index 8f9737c..46c6440 100644 --- a/pubspec.lock +++ b/pubspec.lock @@ -298,7 +298,7 @@ packages: source: hosted version: "0.10.1" camera_platform_interface: - dependency: transitive + dependency: "direct dev" description: name: camera_platform_interface sha256: "7ac852d77699acee79f0d438b793feee26721841e50973576419ff5c6d95e9b7" diff --git a/pubspec.yaml b/pubspec.yaml index 06d9555..c1ebef1 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -74,6 +74,7 @@ dependencies: dev_dependencies: auto_route_generator: ^10.4.0 build_runner: ^2.15.0 + camera_platform_interface: ^2.13.0 flutter_launcher_icons: ^0.14.4 flutter_lints: ^6.0.0 flutter_test: diff --git a/test/src/features/posting/providers/camera_provider_test.dart b/test/src/features/posting/providers/camera_provider_test.dart new file mode 100644 index 0000000..96fda54 --- /dev/null +++ b/test/src/features/posting/providers/camera_provider_test.dart @@ -0,0 +1,112 @@ +import 'dart:async'; + +import 'package:camera_platform_interface/camera_platform_interface.dart'; +import 'package:flutter/services.dart'; +import 'package:flutter_riverpod/flutter_riverpod.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:get_it/get_it.dart'; +import 'package:spark/src/core/utils/logging/logging.dart'; +import 'package:spark/src/features/posting/providers/camera_provider.dart'; + +void main() { + TestWidgetsFlutterBinding.ensureInitialized(); + + late CameraPlatform originalCameraPlatform; + late _FakeCameraPlatform cameraPlatform; + + setUpAll(() { + originalCameraPlatform = CameraPlatform.instance; + }); + + setUp(() async { + await GetIt.I.reset(); + GetIt.I.registerSingleton(LogService()); + cameraPlatform = _FakeCameraPlatform(); + CameraPlatform.instance = cameraPlatform; + }); + + tearDown(() async { + CameraPlatform.instance = originalCameraPlatform; + await GetIt.I.reset(); + }); + + test('auto-dispose releases the initialized camera', () async { + final container = ProviderContainer(); + addTearDown(container.dispose); + final provider = cameraProvider(ResolutionPreset.low); + final subscription = container.listen(provider, (previous, next) {}); + + await container.read(provider.future); + expect(container.read(provider.notifier).controller, isNotNull); + + subscription.close(); + await container.pump(); + await cameraPlatform.disposed; + + expect(cameraPlatform.disposedCameraIds, [_FakeCameraPlatform.cameraId]); + }); +} + +class _FakeCameraPlatform extends CameraPlatform { + static const cameraId = 13; + static const camera = CameraDescription( + name: 'back', + lensDirection: CameraLensDirection.back, + sensorOrientation: 90, + ); + + final Completer _disposed = Completer(); + final List disposedCameraIds = []; + + Future get disposed => _disposed.future; + + @override + Future> availableCameras() async => [camera]; + + @override + Future createCameraWithSettings( + CameraDescription cameraDescription, + MediaSettings mediaSettings, + ) async => cameraId; + + @override + Future initializeCamera( + int cameraId, { + ImageFormatGroup imageFormatGroup = ImageFormatGroup.unknown, + }) async {} + + @override + Stream onCameraInitialized(int cameraId) { + return Stream.value( + CameraInitializedEvent( + cameraId, + 1080, + 1920, + ExposureMode.auto, + true, + FocusMode.auto, + true, + ), + ); + } + + @override + Stream onCameraError(int cameraId) { + return Stream.value(CameraErrorEvent(cameraId, 'test error')); + } + + @override + Stream onDeviceOrientationChanged() { + return Stream.value( + const DeviceOrientationChangedEvent(DeviceOrientation.portraitUp), + ); + } + + @override + Future dispose(int cameraId) async { + disposedCameraIds.add(cameraId); + if (!_disposed.isCompleted) { + _disposed.complete(); + } + } +}