Skip to content
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

Jzecm wal local sync 958 #992

Merged
merged 4 commits into from
Oct 7, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 12 additions & 2 deletions app/lib/providers/capture_provider.dart
Original file line number Diff line number Diff line change
Expand Up @@ -152,11 +152,21 @@ class CaptureProvider extends ChangeNotifier
_bleBytesStream?.cancel();
_bleBytesStream = await _getBleAudioBytesListener(id, onAudioBytesReceived: (List<int> value) {
if (value.isEmpty) return;
wal.onByteStream(value);

// support: opus
var isWalSupported = codec == BleAudioCodec.opus;
if (isWalSupported) {
wal.onByteStream(value);
}
Comment on lines +156 to +160
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Image description Entelligence.AI

The check for isWalSupported is a good addition to ensure that the codec type is supported before invoking wal.onByteStream(value). However, it would be better to return early if the codec is not supported. This way, we avoid unnecessary operations and improve code readability.

- var isWalSupported = codec == BleAudioCodec.opus;
- if (isWalSupported) {
-   wal.onByteStream(value);
- }
+ if (codec != BleAudioCodec.opus) {
+   return;
+ }
+ wal.onByteStream(value);


if (_socket?.state == SocketServiceState.connected) {
final trimmedValue = value.sublist(3);
_socket?.send(trimmedValue);
wal.onBytesSync(value);

// synced
if (isWalSupported) {
wal.onBytesSync(value);
}
Comment on lines +166 to +169
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Image description Entelligence.AI

Similar to the previous comment, it's good to see a check for isWalSupported before invoking wal.onBytesSync(value). But again, consider returning early if the codec is not supported.

- if (isWalSupported) {
-   wal.onBytesSync(value);
- }
+ if (codec != BleAudioCodec.opus) {
+   return;
+ }
+ wal.onBytesSync(value);

Please note that these changes assume that there are no other operations after these checks that need to be executed regardless of the codec type. If there are, you might want to adjust the logic accordingly.

}
});
setAudioBytesConnected(true);
Expand Down
27 changes: 14 additions & 13 deletions app/lib/services/wals.dart
Original file line number Diff line number Diff line change
@@ -1,22 +1,19 @@
import 'dart:async';
import 'dart:collection';
import 'dart:ffi';
import 'dart:io';
import 'dart:math';
import 'dart:typed_data';

import 'package:flutter/foundation.dart';
import 'package:friend_private/backend/http/api/memories.dart';
import 'package:friend_private/backend/preferences.dart';
import 'package:friend_private/backend/schema/memory.dart';
import 'package:friend_private/backend/schema/message_event.dart';
import 'package:friend_private/providers/message_provider.dart';
import 'package:friend_private/services/sockets/transcription_connection.dart';
import 'package:friend_private/services/sockets/wal_connection.dart';
import 'package:path_provider/path_provider.dart';

const ChunkSizeInSeconds = 7; // 30
const FlushIntervalInSeconds = 15; //300
const chunkSizeInSeconds = 30;
const flushIntervalInSeconds = 60;
Comment on lines 3 to +16
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Image description Entelligence.AI

The removal of unused imports and constants is a good practice for code cleanliness and readability. The renaming of the constants ChunkSizeInSeconds and FlushIntervalInSeconds to chunkSizeInSeconds and flushIntervalInSeconds follows Dart's naming conventions for constants, which is also a good change.

- import 'dart:ffi';
- import 'dart:math';
- const ChunkSizeInSeconds = 7; // 30
- const FlushIntervalInSeconds = 15; //300
+ const chunkSizeInSeconds = 30;
+ const flushIntervalInSeconds = 60;


abstract class IWalService {
void start();
Expand Down Expand Up @@ -74,7 +71,7 @@ class Wal {
this.filePath,
this.data = const []});

get seconds => ChunkSizeInSeconds;
get seconds => chunkSizeInSeconds;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Image description Entelligence.AI

The seconds getter has been updated to use the new constant chunkSizeInSeconds. This change is consistent with the update in the constants section. However, it's important to ensure that this new value doesn't negatively impact the system's performance or functionality.

-  get seconds => ChunkSizeInSeconds;
+  get seconds => chunkSizeInSeconds;


factory Wal.fromJson(Map<String, dynamic> json) {
return Wal(
Expand Down Expand Up @@ -150,10 +147,10 @@ class WalService implements IWalService, IWalSocketServiceListener {
void start() {
_wals = SharedPreferencesUtil().wals;
debugPrint("wal service start: ${_wals.length}");
_chunkingTimer = Timer.periodic(const Duration(seconds: ChunkSizeInSeconds), (t) async {
_chunkingTimer = Timer.periodic(const Duration(seconds: chunkSizeInSeconds), (t) async {
await _chunk();
});
_flushingTimer = Timer.periodic(const Duration(seconds: FlushIntervalInSeconds), (t) async {
_flushingTimer = Timer.periodic(const Duration(seconds: flushIntervalInSeconds), (t) async {
Comment on lines +150 to +153
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Image description Entelligence.AI

The timer intervals have been updated to use the renamed constants. This maintains consistency in the codebase.

- _chunkingTimer = Timer.periodic(const Duration(seconds: ChunkSizeInSeconds), (t) async {
- _flushingTimer = Timer.periodic(const Duration(seconds: FlushIntervalInSeconds), (t) async {
+ _chunkingTimer = Timer.periodic(const Duration(seconds: chunkSizeInSeconds), (t) async {
+ _flushingTimer = Timer.periodic(const Duration(seconds: flushIntervalInSeconds), (t) async {

await _flush();
});
_status = WalServiceStatus.ready;
Expand All @@ -166,6 +163,7 @@ class WalService implements IWalService, IWalSocketServiceListener {
}

var framesPerSeconds = 100;
var lossesThreshold = 1 * framesPerSeconds; // 1s
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Image description Entelligence.AI

A new variable lossesThreshold has been introduced. It seems to be used for determining a threshold for frame losses. Please ensure that this new logic does not introduce any unexpected behavior in the frame processing logic.

+    var lossesThreshold = 1 * framesPerSeconds; // 1s

var newFrameSyncDelaySeconds = 5; // wait 5s for new frame synced
var timerEnd = DateTime.now().millisecondsSinceEpoch ~/ 1000 - newFrameSyncDelaySeconds;
var pivot = _frames.length - newFrameSyncDelaySeconds * framesPerSeconds;
Expand All @@ -176,18 +174,22 @@ class WalService implements IWalService, IWalSocketServiceListener {
// Scan backward
var high = pivot;
while (high > 0) {
var low = high - framesPerSeconds * ChunkSizeInSeconds;
var low = high - framesPerSeconds * chunkSizeInSeconds;
if (low < 0) {
low = 0;
}
var synced = true;
var losses = 0;
var chunk = _frames.sublist(low, high);
for (var f in chunk) {
var head = f.sublist(0, 3);
var seq = Uint8List.fromList(head..add(0)).buffer.asByteData().getInt32(0);
if (!_syncFrameSeq.contains(seq)) {
synced = false;
break;
losses++;
if (losses >= lossesThreshold) {
synced = false;
break;
}
Comment on lines +177 to +192
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Image description Entelligence.AI

The logic for frame synchronization has been adjusted. Now, instead of marking a chunk as unsynced as soon as a single frame is not found in _syncFrameSeq, it allows for up to lossesThreshold missing frames before marking the chunk as unsynced. This could potentially improve the robustness of the synchronization process against minor data losses.

- if (!_syncFrameSeq.contains(seq)) {
- synced = false;
- break;
+ if (!_syncFrameSeq.contains(seq)) {
+ losses++;
+ if (losses >= lossesThreshold) {
+ synced = false;
+ break;

}
}
var timerStart = timerEnd - (high - low) ~/ framesPerSeconds;
Expand Down Expand Up @@ -218,7 +220,7 @@ class WalService implements IWalService, IWalSocketServiceListener {
}

// next
timerEnd -= ChunkSizeInSeconds;
timerEnd -= chunkSizeInSeconds;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Image description Entelligence.AI

The timer end calculation has been updated to use the renamed constant, maintaining consistency in the codebase.

- timerEnd -= ChunkSizeInSeconds;
+ timerEnd -= chunkSizeInSeconds;

high = low;
}

Expand Down Expand Up @@ -278,7 +280,6 @@ class WalService implements IWalService, IWalSocketServiceListener {
Future stop() async {
_socket?.stop();

debugPrint("wal service stop");
_chunkingTimer?.cancel();
_flushingTimer?.cancel();

Comment on lines 280 to 285
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Image description Entelligence.AI

The removal of the debug print statement improves the cleanliness of the code. The rest of the changes are consistent with the previous state of the code.

- debugPrint("wal service stop");

Expand Down
Loading