Fix background microphone recording on Android #564
Conversation
|
WalkthroughThis update introduces background microphone recording functionality for Android using Changes
Uplevel your code reviews with Entelligence.AI ProEntelligence.AI ProIf you like this project, please support us by purchasing the Pro version. The Pro version has advanced context, superior noise reduction and several proprietary improvements compared to the open source version. Moreover, Entelligence.AI Pro is free for open source projects. |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Files selected (4)
- app/android/app/src/main/AndroidManifest.xml (2)
- app/lib/pages/capture/logic/mic_background_service.dart (1)
- app/lib/pages/capture/logic/phone_recorder_mixin.dart (2)
- app/lib/pages/capture/page.dart (2)
Files ignored due to filter (1)
- app/pubspec.yaml
Review comments generated (13)
- Review: 8
- LGTM: 5
Tips
Chat with Entelligence.AI Bot (@Entelligence.AI)
- Reply on review comments left by this bot to ask follow-up questions. A review comment is a comment on a diff or a file.
- Invite the bot into a review comment chain by tagging
@Entelligence.AIin a reply.
Code suggestions
- The bot may make code suggestions, but please review them carefully before committing since the line number ranges may be misaligned.
- You can edit the comment made by the bot and manually tweak the suggestion if it is slightly off.
Pausing incremental reviews
- Add
@Entelligence.AI: ignoreanywhere in the PR description to pause further reviews from the bot.
| <service | ||
| android:name="id.flutter.flutter_background_service.BackgroundService" | ||
| android:foregroundServiceType="microphone"/> --> | ||
| android:foregroundServiceType="microphone" | ||
| tools:node="merge" /> |
There was a problem hiding this comment.
The service declaration for the BackgroundService is missing some important attributes. It's recommended to add android:enabled="true" to ensure that the service can be instantiated by the system, and android:exported="false" to prevent other applications from starting the service. Also, consider adding android:stopWithTask="true" if you want the service to automatically stop when the user removes a task rooted in an activity owned by your application.
<service
android:name="id.flutter.flutter_background_service.BackgroundService"
android:foregroundServiceType="microphone"
+ android:enabled="true"
+ android:exported="false"
+ android:stopWithTask="true"
tools:node="merge" />| if (Platform.isAndroid) { | ||
| stopStreamRecordingOnAndroid(); | ||
| } else { | ||
| await stopStreamRecording(wsConnectionState, websocketChannel); | ||
| } |
There was a problem hiding this comment.
The stopStreamRecordingOnAndroid() and stopStreamRecording() functions are called based on the platform, but there's no error handling in case these functions fail. It would be better to wrap these calls in a try-catch block to handle any potential exceptions.
- if (Platform.isAndroid) {
- stopStreamRecordingOnAndroid();
- } else {
- await stopStreamRecording(wsConnectionState, websocketChannel);
- }
+ try {
+ if (Platform.isAndroid) {
+ stopStreamRecordingOnAndroid();
+ } else {
+ await stopStreamRecording(wsConnectionState, websocketChannel);
+ }
+ } catch (e) {
+ print('Error stopping stream recording: $e');
+ }| if (Platform.isAndroid) { | ||
| await streamRecordingOnAndroid(wsConnectionState, websocketChannel); | ||
| } else { | ||
| await startStreamRecording(wsConnectionState, websocketChannel); | ||
| } |
There was a problem hiding this comment.
Similar to the previous comment, the streamRecordingOnAndroid() and startStreamRecording() functions are called based on the platform, but there's no error handling in case these functions fail. It would be better to wrap these calls in a try-catch block to handle any potential exceptions.
- if (Platform.isAndroid) {
- await streamRecordingOnAndroid(wsConnectionState, websocketChannel);
- } else {
- await startStreamRecording(wsConnectionState, websocketChannel);
- }
+ try {
+ if (Platform.isAndroid) {
+ await streamRecordingOnAndroid(wsConnectionState, websocketChannel);
+ } else {
+ await startStreamRecording(wsConnectionState, websocketChannel);
+ }
+ } catch (e) {
+ print('Error starting stream recording: $e');
+ }| WebsocketConnectionStatus? wsConnectionState2; | ||
| IOWebSocketChannel? websocketChannel2; | ||
|
|
||
| @override | ||
| void initState() { | ||
| WidgetsBinding.instance.addPostFrameCallback((timeStamp) async { | ||
| await listenToBackgroundService(); | ||
| }); | ||
| super.initState(); | ||
| } | ||
|
|
||
| listenToBackgroundService() async { | ||
| if (await FlutterBackgroundService().isRunning()) { | ||
| FlutterBackgroundService().on('audioBytes').listen((event) { | ||
| Uint8List convertedList = Uint8List.fromList(event!['data'].cast<int>()); | ||
| if (wsConnectionState2 == WebsocketConnectionStatus.connected) websocketChannel2?.sink.add(convertedList); | ||
| }); | ||
| FlutterBackgroundService().on('stateUpdate').listen((event) { | ||
| if (event!['state'] == 'recording') { | ||
| setState(() => recordingState = RecordingState.record); | ||
| } else if (event['state'] == 'initializing') { | ||
| setState(() => recordingState = RecordingState.initialising); | ||
| } else if (event['state'] == 'stopped') { | ||
| setState(() => recordingState = RecordingState.stop); | ||
| } | ||
| }); | ||
| } | ||
| } |
There was a problem hiding this comment.
The listenToBackgroundService method is missing error handling. If the background service fails or if there's an issue with the event listeners, it could lead to unhandled exceptions. Consider wrapping the contents of this method in a try-catch block to handle potential errors.
listenToBackgroundService() async {
+ try {
if (await FlutterBackgroundService().isRunning()) {
FlutterBackgroundService().on('audioBytes').listen((event) {
Uint8List convertedList = Uint8List.fromList(event!['data'].cast<int>());
if (wsConnectionState2 == WebsocketConnectionStatus.connected) websocketChannel2?.sink.add(convertedList);
});
FlutterBackgroundService().on('stateUpdate').listen((event) {
if (event!['state'] == 'recording') {
setState(() => recordingState = RecordingState.record);
} else if (event['state'] == 'initializing') {
setState(() => recordingState = RecordingState.initialising);
} else if (event['state'] == 'stopped') {
setState(() => recordingState = RecordingState.stop);
}
});
}
+ } catch (e) {
+ // Handle or log the error here
+ }
}| streamRecordingOnAndroid(WebsocketConnectionStatus wsConnectionState, IOWebSocketChannel? websocketChannel) async { | ||
| setState(() { | ||
| wsConnectionState2 = wsConnectionState; | ||
| websocketChannel2 = websocketChannel; | ||
| }); | ||
| await Permission.microphone.request(); | ||
| setState(() => recordingState = RecordingState.initialising); | ||
| await initializeMicBackgroundService(); | ||
| startBackgroundService(); | ||
| await listenToBackgroundService(); | ||
| } | ||
|
|
||
| stopStreamRecordingOnAndroid() { | ||
| stopBackgroundService(); | ||
| if (wsConnectionState2 == WebsocketConnectionStatus.connected) websocketChannel2?.sink.close(); | ||
| } |
There was a problem hiding this comment.
In the streamRecordingOnAndroid and stopStreamRecordingOnAndroid methods, you're directly manipulating state variables (wsConnectionState2, websocketChannel2). This can lead to unexpected behavior and make the code harder to debug. Instead, consider creating setter methods for these variables that encapsulate the state changes.
| void startBackgroundService() { | ||
| final service = FlutterBackgroundService(); | ||
| service.startService(); | ||
| } |
There was a problem hiding this comment.
The startBackgroundService and stopBackgroundService methods are creating a new instance of FlutterBackgroundService each time they're called. This could lead to multiple instances of the service running concurrently, which might not be what you want. Consider using a singleton pattern for managing the background service instance.
+ FlutterBackgroundService _serviceInstance = FlutterBackgroundService();
- void startBackgroundService() {
- final service = FlutterBackgroundService();
- service.startService();
- }
+ void startBackgroundService() {
+ _serviceInstance.startService();
+ }
- void stopBackgroundService() {
- final service = FlutterBackgroundService();
- service.invoke("stop");
- }
+ void stopBackgroundService() {
+ _serviceInstance.invoke("stop");
+ }| Future<void> initializeMicBackgroundService() async { | ||
| final service = FlutterBackgroundService(); | ||
|
|
||
| await service.configure( | ||
| iosConfiguration: IosConfiguration( | ||
| autoStart: false, | ||
| onForeground: onStart, | ||
| onBackground: onIosBackground, | ||
| ), | ||
| androidConfiguration: AndroidConfiguration( | ||
| autoStart: true, | ||
| onStart: onStart, | ||
| isForegroundMode: true, | ||
| autoStartOnBoot: true, | ||
| foregroundServiceType: AndroidForegroundType.microphone, | ||
| ), | ||
| ); | ||
| } |
There was a problem hiding this comment.
In the initializeMicBackgroundService method, you're configuring the service to auto-start on Android but not on iOS (autoStart: true vs autoStart: false). If this is intentional, make sure it's documented somewhere so that future developers understand why there's a difference in behavior between the two platforms.
| @pragma('vm:entry-point') | ||
| Future onStart(ServiceInstance service) async { | ||
| FlutterSoundRecorder? _mRecorder = FlutterSoundRecorder(); | ||
| await _mRecorder.openRecorder(isBGService: true); | ||
| var recordingDataController = StreamController<Uint8List>(); | ||
|
|
||
| await _mRecorder.startRecorder( | ||
| toStream: recordingDataController.sink, | ||
| codec: Codec.pcm16, | ||
| numChannels: 1, | ||
| sampleRate: 16000, | ||
| bufferSize: 8192, | ||
| ); | ||
| service.invoke("stateUpdate", {"state": 'recording'}); | ||
| recordingDataController.stream.listen((buffer) { | ||
| Uint8List audioBytes = buffer; | ||
| List<dynamic> audioBytesList = audioBytes.toList(); | ||
| service.invoke("audioBytes", {"data": audioBytesList}); | ||
| }); | ||
|
|
||
| service.on('stop').listen((event) { | ||
| _mRecorder.stopRecorder(); | ||
| service.invoke("stateUpdate", {"state": 'stopped'}); | ||
| service.stopSelf(); | ||
| }); | ||
| } |
There was a problem hiding this comment.
In the onStart method, you're starting the recorder and setting up a stream listener, but there's no error handling. If an error occurs while opening the recorder or during recording, it could cause the app to crash. Consider adding try-catch blocks around these operations to handle potential errors gracefully.
@pragma('vm:entry-point')
Future onStart(ServiceInstance service) async {
FlutterSoundRecorder? _mRecorder = FlutterSoundRecorder();
+ try {
await _mRecorder.openRecorder(isBGService: true);
var recordingDataController = StreamController<Uint8List>();
await _mRecorder.startRecorder(
toStream: recordingDataController.sink,
codec: Codec.pcm16,
numChannels: 1,
sampleRate: 16000,
bufferSize: 8192,
);
+ } catch (e) {
+ // Handle the error appropriately
+ print('Error starting recorder: $e');
+ }
service.invoke("stateUpdate", {"state": 'recording'});
recordingDataController.stream.listen((buffer) {
Uint8List audioBytes = buffer;
List<dynamic> audioBytesList = audioBytes.toList();
service.invoke("audioBytes", {"data": audioBytesList});
});
service.on('stop').listen((event) {
+ try {
_mRecorder.stopRecorder();
+ } catch (e) {
+ // Handle the error appropriately
+ print('Error stopping recorder: $e');
+ }
service.invoke("stateUpdate", {"state": 'stopped'});
service.stopSelf();
});
}
There was a problem hiding this comment.
Files selected (1)
- app/android/app/proguard-rules.pro (1)
Files ignored due to filter (1)
- app/pubspec.yaml
Review comments generated (2)
- Review: 1
- LGTM: 1
Tips
Chat with Entelligence.AI Bot (@Entelligence.AI)
- Reply on review comments left by this bot to ask follow-up questions. A review comment is a comment on a diff or a file.
- Invite the bot into a review comment chain by tagging
@Entelligence.AIin a reply.
Code suggestions
- The bot may make code suggestions, but please review them carefully before committing since the line number ranges may be misaligned.
- You can edit the comment made by the bot and manually tweak the suggestion if it is slightly off.
Pausing incremental reviews
- Add
@Entelligence.AI: ignoreanywhere in the PR description to pause further reviews from the bot.
|
|
||
| # You might not be using firebase | ||
| -keep class com.google.firebase.** { *; } | ||
| -keep class com.builttoroam.devicecalendar.** { *; } No newline at end of file | ||
| -keep class com.builttoroam.devicecalendar.** { *; } | ||
|
|
||
| -keep class com.pravera.flutter_foreground_task.service.** { *; } |
There was a problem hiding this comment.
Note: This review was outside of the patch, but no patch was found that overlapped with it. Original lines [14-14]
It's good that you've added the ProGuard rule to prevent obfuscation of the com.pravera.flutter_foreground_task.service classes, as they are likely used in reflection or native calls. However, it's a best practice to add a newline at the end of files. This is because some Unix-based tools don't handle the lack of a newline character at the end of files very well.
- \ No newline at end of file
+ -keep class com.pravera.flutter_foreground_task.service.** { *; }
+
I've forked
flutter_soundand made some changes in it so that it can work in background as well. I will open a PR to theflutter_soundfor those changes to be merged. Once those changes are merged, we can use the package directly.Summary by Entelligence.AI
flutter_sound. This allows the app to record and stream audio data even when it's running in the background.com.pravera.flutter_foreground_task.service, enhancing overall app stability.