diff --git a/App/serval_app/lib/data/camera_record.dart b/App/serval_app/lib/data/camera_record.dart index 732fe1e..8223092 100644 --- a/App/serval_app/lib/data/camera_record.dart +++ b/App/serval_app/lib/data/camera_record.dart @@ -241,6 +241,7 @@ class DetectionMaskSettings { /// and a hallway where they never do. class DetectionTuningSettings { const DetectionTuningSettings({ + this.enabled, this.classes, this.describeClasses, this.scoreThreshold, @@ -258,6 +259,7 @@ class DetectionTuningSettings { factory DetectionTuningSettings.fromJson(Map json) => DetectionTuningSettings( + enabled: json['enabled'] as bool?, classes: _strings(json['classes']), describeClasses: _strings(json['describeClasses']), scoreThreshold: (json['scoreThreshold'] as num?)?.toDouble(), @@ -278,6 +280,10 @@ class DetectionTuningSettings { noveltySeconds: (json['noveltySeconds'] as num?)?.toDouble(), ); + /// Whether the detector looks at this camera at all. Null follows the Server, which is what makes + /// its switch mean "every camera that does not say otherwise". + final bool? enabled; + /// Which of the model's classes this camera records at all. final List? classes; @@ -320,7 +326,12 @@ class DetectionTuningSettings { /// Regions of the view to ignore. Carried but not edited — see [DetectionMaskSettings]. final List? masks; + // The switch counts. A bag holding only `enabled: false` is a real instruction, and collapsing it + // to null on the way out — which is what every caller does with an empty one — would send the + // camera back to following the Server and make the toggle spring on again on the next load. The + // Server's own collapse rule counts it for the same reason. bool get isEmpty => + enabled == null && classes == null && describeClasses == null && scoreThreshold == null && @@ -336,6 +347,7 @@ class DetectionTuningSettings { noveltySeconds == null; Map toJson() => { + if (enabled != null) 'enabled': enabled, if (classes != null) 'classes': classes, if (describeClasses != null) 'describeClasses': describeClasses, if (scoreThreshold != null) 'scoreThreshold': scoreThreshold, @@ -353,6 +365,7 @@ class DetectionTuningSettings { /// Omit an argument to keep it; pass null to fall back to the Server's default. See [_keep]. DetectionTuningSettings copyWith({ + Object? enabled = _keep, Object? classes = _keep, Object? describeClasses = _keep, Object? scoreThreshold = _keep, @@ -367,6 +380,7 @@ class DetectionTuningSettings { Object? absenceSeconds = _keep, Object? noveltySeconds = _keep, }) => DetectionTuningSettings( + enabled: _pick(enabled, this.enabled), classes: _pick(classes, this.classes), describeClasses: _pick(describeClasses, this.describeClasses), scoreThreshold: _pick(scoreThreshold, this.scoreThreshold), @@ -385,6 +399,7 @@ class DetectionTuningSettings { @override bool operator ==(Object other) => other is DetectionTuningSettings && + other.enabled == enabled && _sameStrings(other.classes, classes) && _sameStrings(other.describeClasses, describeClasses) && _sameStrings(other.alertClasses, alertClasses) && @@ -401,6 +416,7 @@ class DetectionTuningSettings { @override int get hashCode => Object.hash( + enabled, classes == null ? null : Object.hashAll(classes!), describeClasses == null ? null : Object.hashAll(describeClasses!), alertClasses == null ? null : Object.hashAll(alertClasses!), diff --git a/App/serval_app/lib/models/server_camera_defaults.dart b/App/serval_app/lib/models/server_camera_defaults.dart index c5031ad..fc3299e 100644 --- a/App/serval_app/lib/models/server_camera_defaults.dart +++ b/App/serval_app/lib/models/server_camera_defaults.dart @@ -20,7 +20,15 @@ import 'server_settings.dart'; /// wins whenever it is there. A range here that disagrees with the Server's is a bug in this table, /// not a second opinion. enum CameraSetting { - // What it looks for — `CameraDetectionTuning`. + // Whether it looks at all, then what it looks for — `CameraDetectionTuning`. + detectionEnabled( + 'Serval:Ai:Detection:Enabled', + label: 'Look for objects', + help: + 'Runs the detector on this camera, so it can say what is there rather than only that ' + 'something moved. Left alone it follows the Server.', + kind: SettingKind.boolean, + ), detectionClasses( 'Serval:Ai:Detection:Classes', label: 'Record these objects', diff --git a/App/serval_app/lib/widgets/camera_settings_form.dart b/App/serval_app/lib/widgets/camera_settings_form.dart index c45d4c5..15e7716 100644 --- a/App/serval_app/lib/widgets/camera_settings_form.dart +++ b/App/serval_app/lib/widgets/camera_settings_form.dart @@ -12,6 +12,7 @@ import '../data/camera_record.dart'; import '../models/ptz.dart'; import '../playback/playback_volume.dart'; import '../models/server_camera_defaults.dart'; +import '../models/server_settings.dart' show SettingSource; import '../models/system_stats.dart'; import '../screens/cameras_screen.dart' show SaveFailureNote; import '../theme/nocturne.dart'; @@ -70,7 +71,7 @@ enum CameraSection { ), analysis( 'Analysis', - 'Which of the Server’s two analysers run on this camera. Each one’s own settings are in the ' + 'Which of the Server’s three analysers run on this camera. Each one’s own settings are in the ' 'section named after it.', ), objects( @@ -80,7 +81,7 @@ enum CameraSection { ), motion( 'Motion detection', - 'Used when this Server is not looking for objects — it compares each frame to the last ' + 'Used when this camera is not looking for objects — it compares each frame to the last ' 'instead. A camera facing a tree needs a higher setting than one facing a hallway.', ), speech( @@ -256,10 +257,12 @@ class CameraSettingsForm extends StatefulWidget { CameraSection.analysis: [ if (before.aiVision != after.aiVision) 'scene descriptions', if (before.aiAudio != after.aiAudio) 'audio analysis', + if (before.detectionTuning?.enabled != after.detectionTuning?.enabled) + 'looking for objects', ], CameraSection.objects: [ - if (_withoutMasks(before.detectionTuning) != - _withoutMasks(after.detectionTuning)) + if (_tuningOnly(before.detectionTuning) != + _tuningOnly(after.detectionTuning)) 'what it looks for', ], CameraSection.motion: [ @@ -292,11 +295,13 @@ class CameraSettingsForm extends StatefulWidget { static List changesBetween(CameraRecord before, CameraRecord after) => [for (final named in changesBySection(before, after).values) ...named]; - /// The tuning with its masks taken out, collapsed to null when nothing else is set — so a camera - /// whose only override is a mask compares equal to one with no overrides at all. - static DetectionTuningSettings? _withoutMasks(DetectionTuningSettings? it) { + /// The tuning with the two fields that are edited elsewhere taken out — the masks, on their own + /// screen, and the switch, in *Analysis* — collapsed to null when nothing else is set. So a camera + /// whose only override is one of those compares equal to one with no overrides at all, and neither + /// drawing a polygon nor flipping the switch lights up *Objects & alerts*. + static DetectionTuningSettings? _tuningOnly(DetectionTuningSettings? it) { if (it == null) return null; - final stripped = it.copyWith(masks: null); + final stripped = it.copyWith(masks: null, enabled: null); return stripped.isEmpty ? null : stripped; } @@ -695,7 +700,11 @@ class _CameraSettingsFormState extends State { 'Control profile', ], CameraSection.masks => const ['Masks'], - CameraSection.analysis => const ['Scene descriptions', 'Audio analysis'], + CameraSection.analysis => [ + 'Scene descriptions', + widget.defaults[CameraSetting.detectionEnabled].label, + 'Audio analysis', + ], CameraSection.objects => _labelsFor(_objectFields), CameraSection.motion => _labelsFor(_motionFields), CameraSection.speech => _labelsFor(_speechFields), @@ -768,7 +777,21 @@ class _CameraSettingsFormState extends State { CameraSection.recording => _keepingFootage, CameraSection.cameraControl => _reachingTheCamera, CameraSection.masks => _masksSection, - CameraSection.analysis => _senses, + // The one note in this section is about the Server rather than the camera, and it is drawn + // whatever this camera says: *Look for objects* on a camera is a choice within a Server that + // loaded a detector, and it cannot conjure one that was never opened. Without this, "off + // globally, on for the drive" looks like it works and silently does nothing. + CameraSection.analysis => Column( + crossAxisAlignment: CrossAxisAlignment.stretch, + children: [ + _senses, + if (widget.defaults.valueOf(CameraSetting.detectionEnabled) == + false) ...[ + const SizedBox(height: 12), + const TuningNote(_detectionOffOnServer, warning: true), + ], + ], + ), CameraSection.objects => _cards( cameraDetectionCards( tuning: _edited.detectionTuning, @@ -778,8 +801,13 @@ class _CameraSettingsFormState extends State { _update((r) => r.copyWith(detectionTuning: _keepMasks(tuning))), ), paired: paired, + // Two notes, not one, because the cards in this section answer to two different switches: + // all of them wait on *Look for objects*, and *Describe these objects* additionally waits + // on *Scene descriptions*. One note naming a single switch was wrong for whichever half of + // the section the reader was actually looking at. notes: [ - if (!_edited.aiVision) const TuningNote(_needsSceneDescriptions), + if (!_detectionOn) const TuningNote(_needsObjectDetection), + if (!_edited.aiVision) const TuningNote(_describeNeedsDescriptions), ], ), CameraSection.motion => _cards( @@ -795,6 +823,7 @@ class _CameraSettingsFormState extends State { if (_edited.motionTuning?.problem case final problem?) TuningNote(problem, warning: true), if (!_edited.aiVision) const TuningNote(_needsSceneDescriptions), + if (_detectionOn) const TuningNote(_replacedByObjects), ], ), CameraSection.speech => _speech, @@ -848,6 +877,31 @@ class _CameraSettingsFormState extends State { static const _needsSceneDescriptions = 'Nothing reads these until “Scene descriptions” is on in Analysis. They are kept either way.'; + /// The object equivalent. Almost every card in *Objects & alerts* answers to this switch rather + /// than to *Scene descriptions*, which is what the two used to share and no longer do. + static const _needsObjectDetection = + 'Nothing reads these until “Look for objects” is on in Analysis. They are kept either way.'; + + /// The one card in that section that is still about descriptions. Said apart from the note above + /// because the two can be true separately: a camera can look for objects and describe none of + /// them, which is now a configuration rather than a contradiction. + static const _describeNeedsDescriptions = + '“Describe these objects” does nothing until “Scene descriptions” is on in Analysis. ' + 'Everything else here still applies.'; + + /// Said under *Movement* while the object gate is the one running. The two are alternatives, not + /// a chain — see the vision pipeline — so these are stored and unread rather than half-applied. + static const _replacedByObjects = + 'This camera looks for objects, which replaces watching for movement rather than sitting ' + 'behind it. These are kept, and read again if it is turned off.'; + + /// The Server half of the same story, and the one warning in *Analysis*. Switching a camera on + /// cannot load a model the Server never opened, which is the one thing about these two settings + /// that does not read the way it works. + static const _detectionOffOnServer = + '“Look for objects” is off on the Server, so no detector is loaded and no camera looks for ' + 'objects whatever it is set to here. Turn it on in Server settings first.'; + /// The audio equivalent — and it names *Audio analysis* rather than speech, because that one /// switch gates the sound recogniser too. It was called “Write down speech” while doing both, /// which made turning it off look like it only stopped transcripts. @@ -1423,16 +1477,39 @@ class _CameraSettingsFormState extends State { ); } - /// The two switches the tuning sections are a refinement of, each named after the Server group it - /// turns on for this camera. + /// What this camera is actually looking for, which is its own answer or the Server's behind it. + bool get _detectionOn => + _edited.detectionTuning?.enabled ?? + widget.defaults.valueOf(CameraSetting.detectionEnabled) == true; + + /// Writes the switch without disturbing the rest of the bag — the thresholds and the masks are + /// carried through `copyWith`, so turning detection off never throws away the tuning that would + /// be wanted again the moment it came back on. + void _setDetection(bool? enabled) => _update((r) { + final tuning = (r.detectionTuning ?? const DetectionTuningSettings()) + .copyWith(enabled: enabled); + return r.copyWith(detectionTuning: tuning.isEmpty ? null : tuning); + }); + + /// The three switches the tuning sections are a refinement of, each named after the Server group + /// it turns on for this camera. /// - /// **The second one was called *Write down speech*, and that was a lie by omission.** `aiAudio` + /// **The audio one was called *Write down speech*, and that was a lie by omission.** `aiAudio` /// gates the sound recogniser as well as the transcriber — they run on the same audio — so /// turning off what read as a transcript setting also stopped glass-break and smoke-alarm /// detection, with nothing on screen saying so. *Audio analysis* is what it actually is, and the /// description now names both halves. + /// + /// **The third one is the odd one, and deliberately so.** *Look for objects* overrides a + /// Server-wide switch rather than being a fact the camera holds alone, so it draws the chip and + /// the reset link every other overridable camera setting draws — its switch is *what this camera + /// runs on*, and the chip below says whether the camera chose that or is only following. Widget get _senses => LayoutBuilder( builder: (context, constraints) { + final overridden = _edited.detectionTuning?.enabled != null; + final serverDetects = + widget.defaults.valueOf(CameraSetting.detectionEnabled) == true; + final tiles = [ CapabilityCard( icon: PhosphorIconsFill.sparkle, @@ -1441,6 +1518,18 @@ class _CameraSettingsFormState extends State { value: _edited.aiVision, onChanged: (value) => _update((r) => r.copyWith(aiVision: value)), ), + CapabilityCard( + icon: PhosphorIconsFill.boundingBox, + title: widget.defaults[CameraSetting.detectionEnabled].label, + description: 'Records what is there, not only that something moved.', + value: _detectionOn, + onChanged: _setDetection, + source: overridden ? SettingSource.user : SettingSource.builtIn, + resetLabel: overridden + ? 'Use the default · ${serverDetects ? 'on' : 'off'}' + : null, + onReset: overridden ? () => _setDetection(null) : null, + ), CapabilityCard( icon: PhosphorIconsFill.waveform, title: 'Audio analysis', @@ -1450,9 +1539,11 @@ class _CameraSettingsFormState extends State { ), ]; - // A card narrower than about 140 reads one word to a line, which is worse than a stack — - // so the cards only share a row when there is that much for each of them. - if (constraints.maxWidth < 280) { + // A card narrower than about 135 reads one word to a line, which is worse than a stack — so + // the cards only share a row when there is that much for each of them. Per card rather than + // a flat number, because the third one arrived and a flat 280 would have squeezed three into + // the width that was measured for two. + if (constraints.maxWidth < tiles.length * 135 + (tiles.length - 1) * 10) { return Column( crossAxisAlignment: CrossAxisAlignment.stretch, children: [ diff --git a/App/serval_app/lib/widgets/camera_tuning_sections.dart b/App/serval_app/lib/widgets/camera_tuning_sections.dart index 3ce4ed4..4adbeb5 100644 --- a/App/serval_app/lib/widgets/camera_tuning_sections.dart +++ b/App/serval_app/lib/widgets/camera_tuning_sections.dart @@ -30,6 +30,7 @@ import '../theme/app_theme.dart'; import '../theme/nocturne.dart'; import '../theme/serval_tokens.dart'; import 'label_chips.dart'; +import 'nocturne_toggle.dart'; import 'paired_rows.dart'; import 'settings_cards.dart'; @@ -91,6 +92,9 @@ class CameraSettingCard extends StatelessWidget { if (fallback case final num number) { return 'Use the default · ${settingFigure(number)}'; } + if (fallback case final bool on) { + return 'Use the default · ${on ? 'on' : 'off'}'; + } return 'Use the default'; } @@ -131,6 +135,18 @@ class CameraSettingCard extends StatelessWidget { } Widget get _control { + // Drawn exactly as the Server page draws a bool, for the reason the class comment gives about + // the rest of this card: a camera setting and a Server setting are the same kind of thing. + if (_descriptor.kind == SettingKind.boolean) { + return Align( + alignment: Alignment.centerLeft, + child: NocturneToggle( + value: _effective == true, + onChanged: (picked) => onChanged(picked), + ), + ); + } + if (_isList) { // A list is the one control that must not show the Server's value as its own. Real chips // read as "this camera names these", so a camera following the Server draws no chips and diff --git a/App/serval_app/lib/widgets/nocturne_toggle.dart b/App/serval_app/lib/widgets/nocturne_toggle.dart index e33629d..09e13eb 100644 --- a/App/serval_app/lib/widgets/nocturne_toggle.dart +++ b/App/serval_app/lib/widgets/nocturne_toggle.dart @@ -1,7 +1,9 @@ import 'package:flutter/widgets.dart'; import 'package:phosphor_icons/phosphor_icons.dart'; +import '../models/server_settings.dart'; import '../theme/nocturne.dart'; +import 'settings_cards.dart'; /// The 38x22 switch the settings screen uses everywhere a camera has a capability on or off. /// @@ -151,6 +153,13 @@ class ToggleRow extends StatelessWidget { /// /// The card tints when it is on, which is what makes the three readable as a group at a glance — /// you can see which of a camera's senses are awake without reading any of the labels. +/// +/// **One of the three follows the Server, and says so.** A capability the camera holds itself is a +/// plain bool with two states; one that overrides a Server-wide switch has a third, *unset*, and a +/// two-state control cannot show the difference between a camera that chose *on* and one that is +/// only following. So [source] and [onReset] are optional: given them, the card grows the same chip +/// and reset link a `SettingCard` has, and [value] is the *effective* value — what the camera is +/// actually running on. Without them it is the flat switch it was, which is what the other two want. class CapabilityCard extends StatelessWidget { const CapabilityCard({ super.key, @@ -159,14 +168,28 @@ class CapabilityCard extends StatelessWidget { required this.description, required this.value, this.onChanged, + this.source, + this.resetLabel, + this.onReset, }); final PhosphorIconData icon; final String title; final String description; + + /// What the camera is running on — its own choice, or the Server's behind it when [source] is + /// [SettingSource.builtIn]. final bool value; final ValueChanged? onChanged; + /// Whether this camera set the value itself. Null draws no chip, for a capability with no Server + /// switch behind it to inherit from. + final SettingSource? source; + + /// *Use the default*, naming what it restores. Drawn only alongside [onReset]. + final String? resetLabel; + final VoidCallback? onReset; + @override Widget build(BuildContext context) => Container( padding: const EdgeInsets.all(11), @@ -226,6 +249,25 @@ class CapabilityCard extends StatelessWidget { color: Nocturne.mix(Nocturne.text, 60), ), ), + + // Below the description rather than up beside the switch, which is where the Server page + // puts it. Three of these share a row at about 165px each, and the top line has already + // spent 53 of that on the glyph and the switch — *using the default* does not fit in what + // is left, and moving the switch down to make room would break the alignment across the + // three that makes them readable as a group. + if (source case final source?) ...[ + const SizedBox(height: 9), + Wrap( + spacing: 8, + runSpacing: 4, + crossAxisAlignment: WrapCrossAlignment.center, + children: [ + SettingSourceChip(source: source), + if (resetLabel != null && onReset != null) + SettingsLinkText(resetLabel!, onTap: onReset!), + ], + ), + ], ], ), ); diff --git a/App/serval_app/lib/widgets/settings_cards.dart b/App/serval_app/lib/widgets/settings_cards.dart index 9b2f810..1bc178b 100644 --- a/App/serval_app/lib/widgets/settings_cards.dart +++ b/App/serval_app/lib/widgets/settings_cards.dart @@ -500,7 +500,7 @@ class SettingCard extends StatelessWidget { if (pending) const SettingBadge('not saved', accent: true) else if (source case final source?) - _SettingSourceChip(source: source), + SettingSourceChip(source: source), if (restartRequired) const SettingBadge('needs a restart'), ], ), @@ -819,8 +819,8 @@ class _SettingTextControlState extends State { /// A camera field uses the same three states, because it has the same three: a value set on this /// camera is [SettingSource.user], one that falls through to the Server is [SettingSource.builtIn], /// and [SettingSource.deployment] simply never occurs on a camera. -class _SettingSourceChip extends StatelessWidget { - const _SettingSourceChip({required this.source}); +class SettingSourceChip extends StatelessWidget { + const SettingSourceChip({super.key, required this.source}); final SettingSource source; diff --git a/App/serval_app/test/camera_tuning_form_test.dart b/App/serval_app/test/camera_tuning_form_test.dart index ee408a2..991d26c 100644 --- a/App/serval_app/test/camera_tuning_form_test.dart +++ b/App/serval_app/test/camera_tuning_form_test.dart @@ -147,6 +147,95 @@ void main() { 'how it listens for sounds', ); }); + + /// The switch lives inside `detectionTuning` on the wire and in *Analysis* on screen, which is + /// the same split the masks have — and the same way it would go unsaveable. + testWidgets('looking for objects', (tester) async { + await expectNamed( + tester, + subject().copyWith( + detectionTuning: const DetectionTuningSettings(enabled: false), + ), + 'looking for objects', + ); + }); + }); + + /// The one that would break the feature silently. Every writer of a detection bag does + /// `onChanged(updated.isEmpty ? null : updated)` — the Server collapses an all-null override the + /// same way — so a bag holding only `enabled: false` reading as empty would send the camera back + /// to following the Server on save, and the switch would spring on again on the next load. + group('the switch alone is a real override', () { + test('a bag holding only the switch is not empty', () { + expect(const DetectionTuningSettings(enabled: false).isEmpty, isFalse); + expect(const DetectionTuningSettings(enabled: true).isEmpty, isFalse); + }); + + test('an untouched bag still is', () { + expect(const DetectionTuningSettings().isEmpty, isTrue); + }); + + test('the switch survives a round trip through JSON', () { + final restored = DetectionTuningSettings.fromJson( + const DetectionTuningSettings(enabled: false, maxFps: 2).toJson(), + ); + + expect(restored.enabled, isFalse); + expect(restored.maxFps, 2); + }); + + test('an absent switch reads as following the Server', () { + expect( + DetectionTuningSettings.fromJson(const {'maxFps': 2}).enabled, + isNull, + ); + }); + }); + + group('the object switch is its own section', () { + CameraRecord detecting(bool? enabled) => subject().copyWith( + detectionTuning: DetectionTuningSettings(enabled: enabled), + ); + + test( + 'switching detection off is not also a change to what it looks for', + () { + final changes = CameraSettingsForm.changesBySection( + subject(), + detecting(false), + ); + + expect( + changes[CameraSection.analysis], + contains('looking for objects'), + ); + expect(changes[CameraSection.objects], isEmpty); + }, + ); + + test('a threshold change is not also a change to the switch', () { + final changes = CameraSettingsForm.changesBySection( + detecting(false), + detecting(false).copyWith( + detectionTuning: const DetectionTuningSettings( + enabled: false, + scoreThreshold: 0.4, + ), + ), + ); + + expect(changes[CameraSection.objects], contains('what it looks for')); + expect(changes[CameraSection.analysis], isEmpty); + }); + + test('going back to following the Server is a change', () { + // The reset link writes null, which is a third state rather than a value — and one that has + // to survive `isEmpty` collapsing an otherwise-untouched bag on the way out. + expect( + CameraSettingsForm.changesBetween(detecting(false), subject()), + contains('looking for objects'), + ); + }); }); group('clearing an override is also a change', () { diff --git a/App/serval_app/test/detection_switch_test.dart b/App/serval_app/test/detection_switch_test.dart new file mode 100644 index 0000000..d463f57 --- /dev/null +++ b/App/serval_app/test/detection_switch_test.dart @@ -0,0 +1,176 @@ +import 'package:flutter/material.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:serval_app/data/camera_record.dart'; +import 'package:serval_app/models/server_camera_defaults.dart'; +import 'package:serval_app/models/server_settings.dart'; +import 'package:serval_app/theme/app_theme.dart'; +import 'package:serval_app/widgets/camera_settings_form.dart'; +import 'package:serval_app/widgets/nocturne_toggle.dart'; +import 'package:serval_app/widgets/status_indicators.dart'; + +/// *Look for objects* in *Analysis*: the one capability card that overrides a Server-wide switch +/// rather than being a fact the camera holds alone. +/// +/// The other two cards are plain bools with two states. This one has three — on, off, and *not +/// said*, which follows the Server — and a two-state switch cannot show the difference between a +/// camera that chose *on* and one that is only following. So it draws the chip and the reset link +/// every other overridable camera setting draws, and its switch shows the *effective* value. +/// +/// The failure this pins is quiet: a card that always drew *changed here*, or one whose switch read +/// its own null as *off*, would look right on a Server that detects and lie about every camera on +/// one that does not. +void main() { + ServerCameraDefaults defaultsWith({required bool serverDetects}) => + ServerCameraDefaults.from( + ServerSettings( + groups: const ['AI'], + restartRequired: false, + settings: [ + ServerSetting( + key: 'Serval:Ai:Detection:Enabled', + group: 'AI', + label: 'Look for objects', + help: 'Runs the detector on this camera.', + kind: SettingKind.boolean, + source: SettingSource.builtIn, + restartRequired: true, + value: serverDetects, + ), + ], + ), + ); + + CameraRecord subject({bool? detection}) => CameraRecord.blank().copyWith( + id: 'testcam', + name: 'Test', + aiVision: true, + aiAudio: true, + detectionTuning: detection == null + ? null + : DetectionTuningSettings(enabled: detection), + streams: [ + CameraRecord.blank().streams.single.copyWith( + url: 'rtsp://127.0.0.1:1/main', + ), + ], + ); + + Future pump( + WidgetTester tester, { + required CameraRecord record, + required bool serverDetects, + }) async { + final view = TestWidgetsFlutterBinding.ensureInitialized() + .platformDispatcher + .views + .first; + view.devicePixelRatio = 1.0; + view.physicalSize = const Size(1440, 4000); + addTearDown(() { + view.resetPhysicalSize(); + view.resetDevicePixelRatio(); + }); + + CameraRecord? saved; + await tester.pumpWidget( + MaterialApp( + debugShowCheckedModeBanner: false, + theme: buildServalTheme(), + home: Scaffold( + body: CameraSettingsForm( + record: record, + creating: false, + health: CameraHealth.healthy, + knownLocations: const [], + existingIds: const {}, + defaults: defaultsWith(serverDetects: serverDetects), + onSave: (edited) async => saved = edited, + ), + ), + ), + ); + await tester.pumpAndSettle(); + + // One section is on screen at a time and the index opens on the first, so every assertion below + // has to walk to *Analysis* first. + await tester.tap(find.text('Analysis').first); + await tester.pumpAndSettle(); + + return saved; + } + + testWidgets('Analysis draws three capability cards', (tester) async { + await pump(tester, record: subject(), serverDetects: true); + + expect(find.byType(CapabilityCard), findsNWidgets(3)); + expect(find.text('Look for objects'), findsOneWidget); + }); + + testWidgets('an untouched camera is following the Server', (tester) async { + await pump(tester, record: subject(), serverDetects: true); + + expect(find.text('using the default'), findsWidgets); + // Nothing to restore while nothing is overridden. + expect(find.textContaining('Use the default ·'), findsNothing); + }); + + testWidgets('the switch shows what the camera is running on', (tester) async { + // The card's own value is null in both of these; what differs is the Server behind it, and the + // switch has to show that rather than its own absence of an answer. + await pump(tester, record: subject(), serverDetects: true); + expect(_detectionSwitch(tester).value, isTrue); + + await pump(tester, record: subject(), serverDetects: false); + expect(_detectionSwitch(tester).value, isFalse); + }); + + testWidgets('switching it off says the camera chose that', (tester) async { + await pump(tester, record: subject(), serverDetects: true); + + await tester.tap(find.byWidget(_detectionSwitch(tester))); + await tester.pumpAndSettle(); + + expect(_detectionSwitch(tester).value, isFalse); + expect(find.text('changed here'), findsOneWidget); + expect(find.text('Use the default · on'), findsOneWidget); + }); + + testWidgets('the reset link goes back to following the Server', ( + tester, + ) async { + await pump(tester, record: subject(detection: false), serverDetects: true); + + expect(find.text('changed here'), findsOneWidget); + + await tester.tap(find.text('Use the default · on')); + await tester.pumpAndSettle(); + + // Back to the Server's answer, and saying so. + expect(_detectionSwitch(tester).value, isTrue); + expect(find.textContaining('Use the default ·'), findsNothing); + }); + + testWidgets('a Server that is not detecting warns rather than pretends', ( + tester, + ) async { + // Switching a camera on cannot load a model the Server never opened, which is the one thing + // about these two settings that does not read the way it works. + await pump(tester, record: subject(), serverDetects: false); + + expect( + find.textContaining('off on the Server', findRichText: true), + findsOneWidget, + ); + }); +} + +/// The middle card's switch — the three sit in Analysis in the order descriptions, objects, audio. +NocturneToggle _detectionSwitch(WidgetTester tester) => tester.widget( + find.descendant( + of: find.ancestor( + of: find.text('Look for objects'), + matching: find.byType(CapabilityCard), + ), + matching: find.byType(NocturneToggle), + ), +); diff --git a/Docs/architecture.md b/Docs/architecture.md index 1966a3b..02c8887 100644 --- a/Docs/architecture.md +++ b/Docs/architecture.md @@ -51,8 +51,10 @@ which normalizes it to one codec — the single decode path the whole front end dashboard, WebRTC and **AI** all run off it. The AI is the *same detection library* the edge CameraModule runs, hosted inside the Server, storing the **same telemetry** to MongoDB. A camera gets AI whether or not it has an edge device — edge cameras run it locally, module-less cameras - have the Server run it for them. It is opt-in per camera (`AiVision` / `AiAudio`), because one - vision model is shared across every camera in the process. + have the Server run it for them. Each of its three capabilities is chosen per camera and they are + independent: `AiVision` for scene descriptions and `AiAudio` for speech and sounds are both + opt-in, because one vision model is shared across every camera in the process; object detection is + `DetectionTuning.Enabled`, which follows the Server's own switch unless a camera says otherwise. - **Module camera:** a camera attached to an edge device (Orange Pi / RK3588) running the **CameraModule**. The module captures the camera locally, runs AI in-process — who's speaking, what's said, emotion, audio events, scene descriptions — and POSTs **telemetry** to the Server. diff --git a/Docs/detection.md b/Docs/detection.md index 18a03e1..92ac52a 100644 --- a/Docs/detection.md +++ b/Docs/detection.md @@ -22,8 +22,20 @@ many cameras: | **Motion** | the same, on hosts with no detection model | Frame differencing on a downscaled luma plane. A near-total change is *rejected* rather than reported — that is the IR-cut filter or a light switching on, not movement. | | **Sound level** | the VAD (an ONNX pass on every 512-sample window) | RMS threshold with pre-roll and hangover, so it opens before speech starts and closes well after it ends, and never cuts an utterance. | -**The two vision gates are alternatives, not a chain.** `Serval:Ai:Detection:Enabled` chooses; -off — the default — leaves the motion gate exactly as it was. +**The two vision gates are alternatives, not a chain, and which one runs is per camera.** +`Serval:Ai:Detection:Enabled` decides whether a detection model is loaded at all and is the default +for every camera; each camera then overrides it with `DetectionTuning.Enabled`, where unset — the +usual state — means "follow the Server". Off leaves that camera on the motion gate exactly as it was. + +The two directions are not symmetric, and the asymmetry is worth knowing before you reach for it: +the server key decides whether a model is opened, so switching a *camera* on under a server switch +that is off is stored, shown in the App, and inert. To run detection on two cameras out of six, turn +the server key on and switch the other four off — not the reverse. A camera in that state says so as +a startup advisory, and the App draws a warning under *Analysis*. + +**Object detection and scene descriptions are separate capabilities.** `AiVision` asks for prose; +`DetectionTuning.Enabled` asks for objects. A camera can do either alone — record what is there and +never be described, or be described off the motion gate with no detector spending inference on it. Where a detector *is* loaded, motion does not disappear — it changes job. It stops deciding **whether** to look and starts proposing **where**, which is the only use for it that survives having @@ -596,9 +608,9 @@ the same options instance and writing to it would retune all of them at once. | Bag | What a camera overrides | Why it is local | |---|---|---| | `AudioTuning` | speech gate, VAD threshold, sound gate | How loud the room is. See [the sound gate](#the-sound-gates-threshold-is-per-camera-and-it-matters-more-than-it-looks). | -| `DetectionTuning` | the three class lists, score and alert thresholds, `MinObjectFraction`, `TrackConfirmSeconds`, `TrackCoastSeconds`, `MaxFps`, `MinMovementFraction`, `AbsenceSeconds`, `NoveltySeconds`, masks | What is in the view, how far away it is, and whether things park in it. `NoveltySeconds` especially: a driveway and a hallway disagree completely about what counts as furniture. | +| `DetectionTuning` | `Enabled`, the three class lists, score and alert thresholds, `MinObjectFraction`, `TrackConfirmSeconds`, `TrackCoastSeconds`, `MaxFps`, `MinMovementFraction`, `AbsenceSeconds`, `NoveltySeconds`, masks | What is in the view, how far away it is, and whether things park in it. `NoveltySeconds` especially: a driveway and a hallway disagree completely about what counts as furniture. `Enabled` because inference is a shared budget and not every view is worth a share of it — a camera pointed at a private room can be recorded without being looked at, on a server that looks at everything else. | | `SoundTuning` | alert and ignored labels, both confidence floors, both cooldowns | Which sounds matter is a property of the room. A drive wants vehicles and glass; a nursery wants crying and the smoke alarm and emphatically not every passing car. | -| `MotionTuning` | `PixelDelta`, `MinChangedFraction`, `MaxChangedFraction` | Only reached when object detection is off — but that is every deployment without a detection model, where it is the *only* thing deciding whether the description model runs. | +| `MotionTuning` | `PixelDelta`, `MinChangedFraction`, `MaxChangedFraction` | Only reached when *this camera* is not looking for objects — every deployment without a detection model, and any camera that has switched detection off, where it is the *only* thing deciding whether the description model runs. | Two rules these all follow. An all-null bag is collapsed to no bag on save, so "this camera is tuned" means the same thing in the document, the API and the App. And an empty **list** is refused @@ -825,7 +837,25 @@ off the `vision` field is simply absent — never fabricated. Cameras with no edge module can still have AI, run inside the Server on their behalf, using the same shared library. It is off by default (`Serval:ServerAi:Enabled`), because enabling it loads -real models into that process; individual cameras then opt in with `AiVision` / `AiAudio`. +real models into that process. Individual cameras then choose among three capabilities, and they are +independent of each other: + +| Capability | Per-camera setting | Default | +|---|---|---| +| Scene descriptions | `AiVision` | Off. A flat opt-in the camera holds itself. | +| Object detection | `DetectionTuning.Enabled` | Unset, which follows `Serval:Ai:Detection:Enabled`. | +| Audio analysis | `AiAudio` | Off. Transcription, speaker labelling and sound tagging together. | + +A camera is watched when something it asked for has a model to run on — descriptions pair with the +vision model, objects with the detector — so a host with the small detector and no 2.3 GB vision +model still detects, and a camera that only wants prose is not quietly charged for inference. + +> **Upgrading.** Detection used to be gated by `AiVision` along with descriptions. It is now +> inherited, so on a server with `Serval:Ai:Detection:Enabled` on, cameras that had descriptions +> switched off will **start detecting** — a share of the inference budget, detection records, and +> object alerts on cameras that have never produced one. That is what "follow the Server" means and +> there is no migration; the App shows *Look for objects — using the default* on exactly those +> cameras. To keep the old behaviour, switch *Look for objects* off on them after upgrading. The two halves have deliberately different shapes, because their inputs do: diff --git a/Server/Serval.Server.Tests/CameraAiOptionsTests.cs b/Server/Serval.Server.Tests/CameraAiOptionsTests.cs index d94370f..5974248 100644 --- a/Server/Serval.Server.Tests/CameraAiOptionsTests.cs +++ b/Server/Serval.Server.Tests/CameraAiOptionsTests.cs @@ -380,4 +380,35 @@ public void An_empty_sound_or_movement_object_still_shares_the_server_options() CameraAiOptions.For( global, null, null, new CameraSoundTuning(), new CameraMotionTuning())); } + + /// + /// The switch is the first bool to travel this route, and the one where a write reaching + /// the shared instance would be loudest: one camera opting out would stop every other camera + /// detecting, with the logs showing exactly the setting the operator asked for. + /// + [Fact] + public void Switching_detection_off_for_one_camera_never_reaches_the_shared_defaults() + { + var global = new AiOptions(); + global.Detection.Enabled = true; + + AiOptions resolved = CameraAiOptions.For( + global, null, new CameraDetectionTuning { Enabled = false }); + + Assert.False(resolved.Detection.Enabled); + Assert.True(global.Detection.Enabled); + Assert.NotSame(global.Detection, resolved.Detection); + } + + [Fact] + public void A_camera_that_only_tunes_thresholds_still_inherits_the_switch() + { + var global = new AiOptions(); + global.Detection.Enabled = true; + + AiOptions resolved = CameraAiOptions.For( + global, null, new CameraDetectionTuning { MaxFps = 2 }); + + Assert.True(resolved.Detection.Enabled); + } } diff --git a/Server/Serval.Server.Tests/CameraAiSignatureTests.cs b/Server/Serval.Server.Tests/CameraAiSignatureTests.cs index 5a95d37..d7eb57b 100644 --- a/Server/Serval.Server.Tests/CameraAiSignatureTests.cs +++ b/Server/Serval.Server.Tests/CameraAiSignatureTests.cs @@ -508,4 +508,43 @@ public void Changing_a_cameras_examination_rate_changes_the_signature() => Assert.NotEqual( Signature(Detect(new CameraDetectionTuning { MaxFps = 1.0 })), Signature(Detect(new CameraDetectionTuning { MaxFps = 0.2 }))); + + /// The signature against a server that is looking for objects, which is the only state + /// in which a camera's own switch has anything to disagree with. + private static string SignatureWhileDetecting(Camera camera) + { + var ai = new AiOptions(); + ai.Detection.Enabled = true; + return CameraAiCoordinator.Signature(camera, ai); + } + + /// + /// The per-camera detection switch is not a term of + /// on its own — it reaches the digest through the effective options, like every other override. + /// If it ever stops doing so the toggle saves, the App reports it, and the running session keeps + /// detecting until something unrelated restarts it. + /// + [Fact] + public void Switching_a_cameras_object_detection_off_changes_the_signature() => + Assert.NotEqual( + SignatureWhileDetecting(Detect(null)), + SignatureWhileDetecting(Detect(new CameraDetectionTuning { Enabled = false }))); + + [Fact] + public void Switching_the_server_off_changes_every_inheriting_cameras_signature() => + Assert.NotEqual( + SignatureWhileDetecting(Detect(null)), + Signature(Detect(null))); + + /// + /// Pinning a camera to what the server already says is not a restart, because the digest is over + /// the effective settings rather than over the list of fields a camera overrides — the two + /// resolve to the same thing and a session cannot tell them apart. Recorded here because this is + /// the one place that design is visible from the outside. + /// + [Fact] + public void An_override_equal_to_the_server_value_is_not_a_restart() => + Assert.Equal( + SignatureWhileDetecting(Detect(null)), + SignatureWhileDetecting(Detect(new CameraDetectionTuning { Enabled = true }))); } diff --git a/Server/Serval.Server.Tests/CameraBsonSerializationTests.cs b/Server/Serval.Server.Tests/CameraBsonSerializationTests.cs index 671409b..cdf42c0 100644 --- a/Server/Serval.Server.Tests/CameraBsonSerializationTests.cs +++ b/Server/Serval.Server.Tests/CameraBsonSerializationTests.cs @@ -237,6 +237,26 @@ public void Detection_tuning_round_trips_including_its_masks() Assert.Equal(["car", "truck"], mask.Classes!); } + [Fact] + public void The_detection_switch_round_trips_and_an_absent_one_reads_as_inherit() + { + Camera camera = Camera(); + camera.DetectionTuning = new CameraDetectionTuning { Enabled = false }; + + var restored = BsonSerializer.Deserialize(camera.ToBsonDocument()); + + Assert.False(restored.DetectionTuning!.Enabled); + + // Every camera written before this field existed has no element for it, and must come back + // following the server rather than switched off. + Camera older = Camera(); + older.DetectionTuning = new CameraDetectionTuning { MaxFps = 2 }; + BsonDocument document = older.ToBsonDocument(); + document["DetectionTuning"].AsBsonDocument.Remove("Enabled"); + + Assert.Null(BsonSerializer.Deserialize(document).DetectionTuning!.Enabled); + } + [Fact] public void An_untuned_camera_stores_no_detection_field_at_all() { diff --git a/Server/Serval.Server.Tests/CameraRegistryCheckTests.cs b/Server/Serval.Server.Tests/CameraRegistryCheckTests.cs index cdd60e8..47aba3f 100644 --- a/Server/Serval.Server.Tests/CameraRegistryCheckTests.cs +++ b/Server/Serval.Server.Tests/CameraRegistryCheckTests.cs @@ -278,6 +278,83 @@ public void Ai_enabled_on_a_camera_but_not_on_the_server_earns_an_advisory() Assert.Contains(advisories, a => a.Contains("AiAudio", StringComparison.Ordinal)); } + [Fact] + public void Asking_to_look_for_objects_on_a_server_with_no_detector_earns_an_advisory() + { + // The asymmetry that reads backwards: the server key decides whether a model is opened at + // all, so "off globally, on for the drive" looks configured and detects nothing. + Camera camera = Valid(); + camera.DetectionTuning = new CameraDetectionTuning { Enabled = true }; + + IReadOnlyList advisories = + CameraRegistryCheck.Advisories(camera, ServerAiOn()); + + Assert.Contains( + advisories, + a => a.Contains("Serval:Ai:Detection:Enabled", StringComparison.Ordinal)); + } + + [Fact] + public void Asking_to_look_for_objects_with_the_server_ai_off_earns_an_advisory() + { + Camera camera = Valid(); + camera.DetectionTuning = new CameraDetectionTuning { Enabled = true }; + + IReadOnlyList advisories = + CameraRegistryCheck.Advisories(camera, new ServerOptions()); + + Assert.Contains( + advisories, + a => a.Contains("Serval:ServerAi:Enabled", StringComparison.Ordinal)); + } + + [Fact] + public void Detection_thresholds_kept_on_a_camera_that_is_not_detecting_earn_an_advisory() + { + // The mirror of the audio trap: tuned against a real view, then switched off, and the next + // person reads the numbers as being in force. + Camera camera = Valid(); + camera.DetectionTuning = new CameraDetectionTuning + { + Enabled = false, + ScoreThreshold = 0.6, + }; + + IReadOnlyList advisories = + CameraRegistryCheck.Advisories(camera, DetectionOn()); + + Assert.Contains( + advisories, + a => a.Contains("object-detection thresholds", StringComparison.Ordinal)); + } + + /// + /// The state that must stay quiet. A camera that writes no descriptions and inherits detection + /// is a legitimate steady configuration — record what is there, say nothing about it — and these + /// are logged at warning level once per startup, so a permanent line about a deliberate setup is + /// what teaches people to skip the rest of them. + /// + [Fact] + public void A_camera_that_detects_without_describing_earns_no_advisory() + { + Camera camera = Valid(); + camera.AiVision = false; + + IReadOnlyList advisories = + CameraRegistryCheck.Advisories(camera, DetectionOn()); + + Assert.DoesNotContain( + advisories, + a => a.Contains("look for objects", StringComparison.OrdinalIgnoreCase)); + } + + private static ServerOptions DetectionOn() + { + ServerOptions options = ServerAiOn(); + options.Ai.Detection.Enabled = true; + return options; + } + private static ServerOptions ServerAiOn() { var options = new ServerOptions(); diff --git a/Server/Serval.Server.Tests/CameraSettingFallbackParityTests.cs b/Server/Serval.Server.Tests/CameraSettingFallbackParityTests.cs index 61d2bc4..99e3f74 100644 --- a/Server/Serval.Server.Tests/CameraSettingFallbackParityTests.cs +++ b/Server/Serval.Server.Tests/CameraSettingFallbackParityTests.cs @@ -130,6 +130,7 @@ public void The_parse_finds_every_overridable_field() string[] expected = [ // CameraDetectionTuning, less Masks — polygons have no catalogue entry to fall back to. + "Serval:Ai:Detection:Enabled", "Serval:Ai:Detection:Classes", "Serval:Ai:Detection:DescribeClasses", "Serval:Ai:Detection:AlertClasses", "Serval:Ai:Detection:ScoreThreshold", "Serval:Ai:Detection:MinObjectFraction", "Serval:Ai:Detection:AlertMinConfidence", diff --git a/Server/Serval.Server.Tests/CameraValidationTests.cs b/Server/Serval.Server.Tests/CameraValidationTests.cs index b8b1ed9..2db26f0 100644 --- a/Server/Serval.Server.Tests/CameraValidationTests.cs +++ b/Server/Serval.Server.Tests/CameraValidationTests.cs @@ -478,6 +478,20 @@ public void An_empty_detection_override_is_collapsed_to_none() Assert.Null(camera.DetectionTuning); } + [Fact] + public void A_detection_override_holding_only_the_switch_survives_the_collapse() + { + // The counterpart to the test above, and the one that would break the feature silently: + // "this camera does not detect" is a real instruction with nothing else beside it, and + // collapsing it to null would send the camera back to following the server on every save. + Camera camera = WithDetection(new CameraDetectionTuning { Enabled = false }); + + CameraRepository.Validate(camera); + + Assert.NotNull(camera.DetectionTuning); + Assert.False(camera.DetectionTuning.Enabled); + } + [Fact] public void An_empty_class_list_is_rejected_rather_than_interpreted() { diff --git a/Server/Serval.Server.Tests/CameraVisionCapabilityTests.cs b/Server/Serval.Server.Tests/CameraVisionCapabilityTests.cs index 829956f..d0f48d4 100644 --- a/Server/Serval.Server.Tests/CameraVisionCapabilityTests.cs +++ b/Server/Serval.Server.Tests/CameraVisionCapabilityTests.cs @@ -12,18 +12,29 @@ namespace Serval.Server.Tests; public class CameraVisionCapabilityTests { [Theory] - // The case that was wrong: the scene-description worker is only registered when a 2.3 GB + // The case that was wrong first: the scene-description worker is only registered when a 2.3 GB // vision model is on disk, while the detector needs one a couple of hundred times smaller. // Requiring the worker meant a host with the detector and no vision model — by far the likelier // first deployment — silently never looked at a single frame. - [InlineData(true, false, true, true)] - [InlineData(true, true, false, true)] - [InlineData(true, true, true, true)] - [InlineData(true, false, false, false)] - [InlineData(false, true, true, false)] - public void Either_capability_alone_is_enough_to_watch_a_camera( - bool aiVision, bool hasVisionModel, bool hasDetector, bool expected) => - Assert.Equal(expected, CameraAiCoordinator.WantsVision(aiVision, hasVisionModel, hasDetector)); + [InlineData(true, true, false, true, true)] + [InlineData(true, true, true, false, true)] + [InlineData(true, true, true, true, true)] + [InlineData(true, true, false, false, false)] + [InlineData(false, false, true, true, false)] + + // Each capability pairs with its own model, which is the whole of the split. Describing scenes + // and looking for objects are asked for separately, so a camera that wants prose and has no + // vision model to write it is not watched just because a detector happens to be loaded — the + // old rule reached either model from either flag and ran the detector on it. + [InlineData(true, false, false, true, false)] + [InlineData(false, true, false, true, true)] + [InlineData(false, true, true, true, true)] + [InlineData(false, true, true, false, false)] + public void Each_capability_pairs_with_its_own_model( + bool describes, bool detects, bool hasVisionModel, bool hasDetector, bool expected) => + Assert.Equal( + expected, + CameraAiCoordinator.WantsVision(describes, detects, hasVisionModel, hasDetector)); private static CameraVisionPipeline Pipeline(AiOptions ai, IObjectDetector? detector) => new( @@ -80,6 +91,53 @@ public void A_loaded_and_enabled_detector_replaces_the_motion_gate_rather_than_j Assert.True(pipeline.UsesDetection); } + [Fact] + public void A_camera_that_switches_detection_off_keeps_the_motion_gate() + { + // The server is detecting and a model is loaded; this one camera has opted out. It must + // land on frame differencing rather than on nothing, so its scene descriptions carry on. + var global = new AiOptions(); + global.Detection.Enabled = true; + + AiOptions ai = CameraAiOptions.For( + global, tuning: null, detection: new CameraDetectionTuning { Enabled = false }); + + using CameraVisionPipeline pipeline = Pipeline(ai, new FakeDetector()); + + Assert.False(pipeline.UsesDetection); + Assert.True(global.Detection.Enabled); + } + + [Fact] + public void A_camera_that_switches_detection_on_under_a_server_that_did_not_load_one_is_inert() + { + // The asymmetry worth pinning: the server key decides whether a model is opened at all, so + // asking for detection on a camera cannot conjure one. The advisory says so; this proves it. + var global = new AiOptions(); + global.Detection.Enabled = false; + + AiOptions ai = CameraAiOptions.For( + global, tuning: null, detection: new CameraDetectionTuning { Enabled = true }); + + using CameraVisionPipeline pipeline = Pipeline(ai, detector: null); + + Assert.False(pipeline.UsesDetection); + } + + [Fact] + public void An_unset_switch_follows_the_server() + { + var global = new AiOptions(); + global.Detection.Enabled = true; + + AiOptions ai = CameraAiOptions.For( + global, tuning: null, detection: new CameraDetectionTuning { MaxFps = 2 }); + + using CameraVisionPipeline pipeline = Pipeline(ai, new FakeDetector()); + + Assert.True(pipeline.UsesDetection); + } + private sealed class FakeDetector : IObjectDetector { public string Description => "fake"; diff --git a/Server/Serval.Server/Ai/CameraAiCoordinator.cs b/Server/Serval.Server/Ai/CameraAiCoordinator.cs index 3dafc0f..692cdd2 100644 --- a/Server/Serval.Server/Ai/CameraAiCoordinator.cs +++ b/Server/Serval.Server/Ai/CameraAiCoordinator.cs @@ -34,6 +34,7 @@ public sealed class CameraAiCoordinator : BackgroundService private readonly CameraRepository _cameras; private readonly SnapshotBroadcaster _snapshots; private readonly DetectFrameBroadcaster _detectFrames; + private readonly Ingest.DetectSessionRestarts _detectRestarts; private readonly DetectionLoad _load; private readonly SceneDescriptionWorker? _vision; private readonly IObjectDetector? _detector; @@ -69,6 +70,7 @@ public CameraAiCoordinator( CameraRepository cameras, SnapshotBroadcaster snapshots, DetectFrameBroadcaster detectFrames, + Ingest.DetectSessionRestarts detectRestarts, DetectionLoad load, TelemetryRepository repository, EventBroadcaster events, @@ -85,6 +87,7 @@ public CameraAiCoordinator( _cameras = cameras; _snapshots = snapshots; _detectFrames = detectFrames; + _detectRestarts = detectRestarts; _load = load; _repository = repository; _events = events; @@ -289,18 +292,25 @@ private async Task ReconcileAsync(CancellationToken cancellationToken) /// /// Whether to watch this camera's frames at all. /// - /// Either capability is enough on its own, and that matters: - /// is only registered when a 2.3 GB vision model is on disk, while the object detector needs one - /// a couple of hundred times smaller. Requiring the worker would mean a host that has the - /// detector and not the model — by far the more likely first deployment — silently never looked - /// at anything. + /// Each capability pairs with its own model. Describing scenes and looking for + /// objects are two things a camera can ask for separately: a drive worth detecting on is not + /// necessarily worth seconds of a 2.3 GB model's time, and a camera watching a quiet room can be + /// worth a description without being worth a share of the inference budget. So a camera is + /// watched when something it asked for has a model to run on, and the pairing is what stops a + /// host that has the detector and not the vision model — by far the more likely first + /// deployment — from either silently never looking at anything or running the detector on a + /// camera that only ever wanted prose. /// - private bool WantsVision(Camera camera) => - WantsVision(camera.AiVision, _vision is not null, _detector is not null); + private bool WantsVision(Camera camera) => WantsVision( + camera.AiVision, + Effective(camera).Detection.Enabled, + _vision is not null, + _detector is not null); /// The rule on its own, so it can be pinned without standing up a host. - internal static bool WantsVision(bool aiVision, bool hasVisionModel, bool hasDetector) => - aiVision && (hasVisionModel || hasDetector); + internal static bool WantsVision( + bool describes, bool detects, bool hasVisionModel, bool hasDetector) => + (describes && hasVisionModel) || (detects && hasDetector); private bool WantsAudio(Camera camera) => camera.AiAudio && _analyzer is not null && camera.DetectStream is not null; @@ -336,7 +346,12 @@ internal static AiOptions Effective(AiOptions global, Camera camera) => CameraAi /// full. /// /// Static, and taking the server-wide settings as an argument, so it can be pinned - /// without standing up a host — the same reason is. + /// without standing up a host — the same reason + /// is. + /// + /// The per-camera object-detection switch is deliberately not a term of its own: it lives + /// in and so already reaches the digest through the + /// effective settings. Naming it here as well would digest it twice. /// internal static string Signature(Camera camera, AiOptions global) => $"{camera.DetectStream?.Url}|{camera.AiVision}|{camera.AiAudio}" @@ -345,15 +360,23 @@ internal static string Signature(Camera camera, AiOptions global) => private void Start(Camera camera) { var cts = new CancellationTokenSource(); - var session = new Session( - Signature(camera), cts, WantsVision(camera) && _detector is not null); + AiOptions ai = Effective(camera); + bool detects = CameraVisionPipeline.Detects(ai, _detector is not null); + + // Asked the same way the pipeline will ask it, rather than inferred from WantsVision — that + // is now true for a camera wanting only descriptions, and counting one of those against the + // inference budget would report every detecting camera a smaller share than it really gets. + var session = new Session(Signature(camera), cts, detects); session.Task = RunSupervisedAsync(camera, cts.Token); _sessions[camera.Id] = session; + // The two vision halves are logged apart because they are now separately switchable, and + // one "vision=True" no longer says which of them a camera actually got. _logger.LogInformation( - "Started AI for camera {CameraId} (vision={Vision}, audio={Audio}).", - camera.Id, WantsVision(camera), WantsAudio(camera)); + "Started AI for camera {CameraId} (descriptions={Describes}, objects={Detects}, " + + "audio={Audio}).", + camera.Id, camera.AiVision && _vision is not null, detects, WantsAudio(camera)); } /// @@ -400,12 +423,13 @@ private async Task RunOnceAsync(Camera camera, CancellationToken cancellationTok { var jobs = new List>(); - // Derived once, here, and handed to both halves. Vision has no per-camera overrides today, - // but having a single point where a camera's settings are resolved means adding one later - // is a change inside CameraAiOptions rather than a new read of the globals out here. + // Derived once, here, and handed to both halves — including to the capability question + // below, which reads this camera's detection switch out of it rather than resolving the + // camera's settings a second time to ask. AiOptions ai = Effective(camera); - if (WantsVision(camera)) + if (WantsVision( + camera.AiVision, ai.Detection.Enabled, _vision is not null, _detector is not null)) { jobs.Add(token => RunVisionAsync(camera, ai, token)); } @@ -445,9 +469,18 @@ private async Task RunOnceAsync(Camera camera, CancellationToken cancellationTok /// private async Task RunVisionAsync(Camera camera, AiOptions ai, CancellationToken cancellationToken) { + // The worker is withheld from a camera that detects without wanting prose, which is the + // whole of what "descriptions off, objects on" costs. Registering it anyway would put the + // camera into the worker's round-robin to be skipped forever, taking a turn from cameras + // that do want describing. using var pipeline = new CameraVisionPipeline( - camera, ai, _vision, _detector, _loggerFactory.CreateLogger(), - _scheduler, _load); + camera, + ai, + camera.AiVision ? _vision : null, + _detector, + _loggerFactory.CreateLogger(), + _scheduler, + _load); try { @@ -539,9 +572,21 @@ private async Task ReadDetectFramesAsync( } catch (OperationCanceledException) when (!cancellationToken.IsCancellationRequested) { + // The ingest session too, not this one alone. Frames stop for two different kinds + // of reason and only one of them is on this side: a reader that has wedged is fixed + // by rebuilding it, and a writer that has stopped is not fixed by anything that + // happens here — re-subscribing to a producer that is no longer producing gets the + // same silence and the same timeout, forever, at whatever the idle period is. + // + // Asked for before returning because the request is a signal and not a handshake: + // it hands the ingest supervisor a token to cancel and comes straight back, so both + // halves rebuild at once rather than this one waiting on the other. _logger.LogWarning( - "Camera {CameraId}: no detect frame for {Seconds:0}s; restarting its AI session.", + "Camera {CameraId}: no detect frame for {Seconds:0}s; restarting its AI session " + + "and asking its detect ingest session to restart.", camera.Id, idleTimeout.TotalSeconds); + + _detectRestarts.Request(camera.Id); return; } diff --git a/Server/Serval.Server/Ai/CameraVisionPipeline.cs b/Server/Serval.Server/Ai/CameraVisionPipeline.cs index 4ec67b3..ae85fd9 100644 --- a/Server/Serval.Server/Ai/CameraVisionPipeline.cs +++ b/Server/Serval.Server/Ai/CameraVisionPipeline.cs @@ -18,9 +18,11 @@ namespace Serval.Server.Ai; /// Two gates can stand in front of the vision model and only one runs: /// /// -/// Object detection, when a detector is loaded and enabled. It reports what is present, -/// which is what makes "a person arrived" and "the car parked here has left" expressible. -/// Frame differencing otherwise — the path for a host with no model on disk. +/// Object detection, when a detector is loaded and this camera is set to use it. It +/// reports what is present, which is what makes "a person arrived" and "the car parked here has +/// left" expressible. +/// Frame differencing otherwise — the path for a host with no model on disk, and for a +/// camera that has switched detection off. /// /// /// **Alternatives rather than a chain.** Running motion in front of the detector buys a little @@ -28,6 +30,12 @@ namespace Serval.Server.Ai; /// motion" exactly when a floodlight has come on because someone is there, and slow or distant /// movement never reaches the threshold at all. /// +/// The gate outlives the thing it gates. Detection and scene description are separate +/// capabilities, so the detector runs whether or not there is a vision model behind it — a camera +/// can record what is there and never be asked to write prose about it. Which of the two gates runs +/// is decided here regardless; whether anything is described is decided by whether this pipeline was +/// handed a worker. +/// /// Two inputs, and the split is deliberate. Detection runs on raw frames from /// , already scaled and never through a JPEG — re-encoding a /// picture only to decode it again costs time and the detail a small distant object can least @@ -39,6 +47,9 @@ public sealed class CameraVisionPipeline : IDisposable { private readonly Camera _camera; private readonly AiOptions _ai; + /// Null when this camera produces no descriptions — either because no vision model is + /// loaded on this host, or because the camera detects without wanting prose written about it. + /// The pipeline cannot tell the two apart and does not need to. private readonly SceneDescriptionWorker? _vision; private readonly IObjectDetector? _detector; private readonly SceneDescriptionService? _scenes; @@ -124,7 +135,15 @@ public CameraVisionPipeline( } /// Whether the object gate is the one running. False means frame differencing. - public bool UsesDetection => _detector is not null && _ai.Detection.Enabled; + public bool UsesDetection => Detects(_ai, _detector is not null); + + /// + /// The rule on its own, so can answer the same question before + /// a pipeline exists — it has to count detecting cameras to divide the inference budget, and a + /// second hand-written copy of this is how that count drifts from what is actually running. + /// + internal static bool Detects(AiOptions ai, bool hasDetector) => + hasDetector && ai.Detection.Enabled; /// /// Folds one snapshot in: it is always a candidate frame for a description, and on a host with no @@ -534,8 +553,9 @@ private void Request(string trigger, double? motionScore) { if (_vision is null || _scenes is null) { - // Detection without a vision model is a supported deployment: the 12 MB detector runs, - // episodes are recorded, and there is simply nothing to describe them with. + // Detection without descriptions is a supported deployment twice over: a host with the + // 12 MB detector and no vision model, and a camera that asked for objects and not prose. + // Either way episodes are recorded and there is nothing to describe them with. return; } diff --git a/Server/Serval.Server/Cameras/Camera.cs b/Server/Serval.Server/Cameras/Camera.cs index c3d213e..95788f3 100644 --- a/Server/Serval.Server/Cameras/Camera.cs +++ b/Server/Serval.Server/Cameras/Camera.cs @@ -131,11 +131,16 @@ public sealed class Camera public double? PlaybackGateRmsThreshold { get; set; } /// - /// Run server-side scene description for this camera, gated on motion. + /// Run server-side scene description for this camera, gated on movement — or on what the object + /// detector finds, when this camera is also looking for objects. /// /// This is the point of the shared detection library: a camera with no edge module still gets /// AI, run by the Server on its behalf. Off by default because the vision model costs seconds /// of CPU per description and one model instance is shared across every camera. + /// + /// Descriptions only. It does not switch the object detector on or off — that is + /// , and the two are independent so a camera can + /// record what is there without being asked to write prose about it, or the reverse. /// public bool AiVision { get; set; } @@ -155,9 +160,11 @@ public sealed class Camera public CameraAudioTuning? AudioTuning { get; set; } /// - /// Per-camera overrides for object detection; null falls back to the server defaults under - /// Serval:Ai:Detection. See — masks especially have - /// no sensible global value, because where a property line runs is a fact about one camera. + /// Per-camera overrides for object detection, starting with + /// — whether this camera is looked at by the + /// detector at all. Null falls back to the server defaults under Serval:Ai:Detection. See + /// — masks especially have no sensible global value, because + /// where a property line runs is a fact about one camera. /// [BsonIgnoreIfNull] public CameraDetectionTuning? DetectionTuning { get; set; } @@ -172,7 +179,7 @@ public sealed class Camera /// /// Per-camera overrides for the movement gate; null falls back to Serval:Ai:Motion. Only - /// reached on a server with object detection switched off, which is where it is the only thing + /// reached when this camera is not looking for objects, which is where it is the only thing /// deciding whether the description model runs — see . /// [BsonIgnoreIfNull] diff --git a/Server/Serval.Server/Cameras/CameraDetectionTuning.cs b/Server/Serval.Server/Cameras/CameraDetectionTuning.cs index ed0feb9..0fd60e9 100644 --- a/Server/Serval.Server/Cameras/CameraDetectionTuning.cs +++ b/Server/Serval.Server/Cameras/CameraDetectionTuning.cs @@ -5,9 +5,9 @@ namespace Serval.Server.Cameras; /// -/// One camera's overrides for what its object detector looks for and what is worth waking the -/// vision model over. Every field is optional; null means "use the server default", exactly as -/// does. +/// One camera's overrides for whether its object detector runs at all, what it looks for, and what +/// is worth waking the vision model over. Every field is optional; null means "use the server +/// default", exactly as does. /// /// A sibling of that type rather than fields on it, because vision knobs on something called /// AudioTuning would be a lie that outlives whoever wrote it. The two are resolved together @@ -24,6 +24,20 @@ namespace Serval.Server.Cameras; /// public sealed class CameraDetectionTuning { + /// + /// Overrides Serval:Ai:Detection:Enabled — whether this camera is looked at by the object + /// detector. Null inherits, which is what makes the server switch mean "detect on every camera + /// that does not say otherwise" rather than "detect on the cameras listed twice". + /// + /// It cannot resurrect a detector the process never opened: the server key decides whether + /// a model is loaded at all, and this chooses within a server that loaded one. A camera asking + /// for detection on a server with the key off is inert, and says so as a startup advisory. + /// + /// Switching it off leaves the camera on the frame-differencing gate, so scene + /// descriptions carry on being triggered by movement. The two are separate capabilities. + /// + public bool? Enabled { get; set; } + /// /// Overrides Serval:Ai:Detection:Classes — which of the model's classes this camera /// records at all. Null inherits; an empty array is rejected at the API rather than silently diff --git a/Server/Serval.Server/Cameras/CameraRegistryCheck.cs b/Server/Serval.Server/Cameras/CameraRegistryCheck.cs index eb17727..58b94c6 100644 --- a/Server/Serval.Server/Cameras/CameraRegistryCheck.cs +++ b/Server/Serval.Server/Cameras/CameraRegistryCheck.cs @@ -103,6 +103,7 @@ public static IReadOnlyList Advisories(Camera camera, ServerOptions opti } AudioTuningAdvisories(camera, options, advisories); + DetectionTuningAdvisories(camera, options, advisories); return advisories; } @@ -232,4 +233,55 @@ void NoiseFloorAdvisory(double? value, string name) } } } + + /// + /// A camera asking to detect on a server that cannot, and thresholds kept for a detector that is + /// not looking at this camera. + /// + /// The asymmetry is the one worth saying out loud. + /// Serval:Ai:Detection:Enabled decides whether a model is loaded at all, so a camera that + /// switches detection on under a server switch that is off is stored, shown in the App, and + /// inert — there is no detector for it to be chosen out of. The way to run detection on two + /// cameras is to turn the server switch on and the other cameras off, which is the opposite of + /// what the two settings look like they do. + /// + /// Deliberately silent about a camera with off and detection + /// inherited on. That is a legitimate steady state — record what is there, write no prose about + /// it — and these are logged at warning level once per startup, so anything permanent and + /// intentional in here is noise that teaches people to skip the rest. + /// + private static void DetectionTuningAdvisories( + Camera camera, ServerOptions options, List advisories) + { + if (camera.DetectionTuning is not { } tuning) + { + return; + } + + if (tuning.Enabled == true && !options.ServerAi.Enabled) + { + advisories.Add( + $"Camera {camera.Id} asks to look for objects but Serval:ServerAi:Enabled is false, " + + "so no models are loaded and nothing will be detected."); + } + + if (tuning.Enabled == true && options.ServerAi.Enabled && !options.Ai.Detection.Enabled) + { + advisories.Add( + $"Camera {camera.Id} asks to look for objects but Serval:Ai:Detection:Enabled is " + + "false, so no detector is loaded and no camera looks for objects. Turn it on " + + "there; cameras that should not detect can then be switched off one at a time."); + } + + // The mirror of the AiAudio advisory above, and the same trap: thresholds tuned against a + // real view, then detection switched off, and the next person reads them as being in force. + if (TuningCatalog.HasAnyOverride(tuning, TuningCatalog.DetectionThresholds) + && !(tuning.Enabled ?? options.Ai.Detection.Enabled)) + { + advisories.Add( + $"Camera {camera.Id} sets object-detection thresholds but is not looking for " + + "objects, so nothing reads them. They are kept, and take effect if it is turned " + + "back on."); + } + } } diff --git a/Server/Serval.Server/Cameras/TuningCatalog.cs b/Server/Serval.Server/Cameras/TuningCatalog.cs index 21ee081..6f46ec5 100644 --- a/Server/Serval.Server/Cameras/TuningCatalog.cs +++ b/Server/Serval.Server/Cameras/TuningCatalog.cs @@ -17,6 +17,11 @@ namespace Serval.Server.Cameras; /// empty-array cases are rejected rather than interpreted for a similar reason: an empty class /// list could defensibly mean "everything" or "nothing", and a camera that silently detects /// nothing while its configuration looks deliberate is the worse of the two readings. +/// +/// Refusing all of that is only fair because there is now an honest way to say it: +/// switches a camera's detector off in as many words. +/// So a ScoreThreshold of 1 is still rejected, and the person who reached for it has +/// somewhere to go that reads the same way to whoever finds the camera later. /// internal static class TuningCatalog { @@ -68,14 +73,29 @@ public void Apply(TBag bag, TTarget target) } } - internal static readonly IReadOnlyList> Detection = + /// + /// Whether this camera detects at all. The one knob with nothing to validate: both values are + /// meaningful, and unset is a third meaning rather than a missing one. + /// + private static readonly IKnob DetectionSwitch = + new ValueKnob( + t => t.Enabled, + _ => null, + (d, v) => d.Enabled = v); + + /// + /// Every detection knob but the switch — the ones saying how this camera detects rather + /// than whether it does. Named apart so an advisory can report them as stored and unread + /// without counting the switch that made them unread. + /// + internal static readonly IReadOnlyList> DetectionThresholds = [ new ListKnob( t => t.Classes, v => v.Length == 0 ? "Classes must name at least one class when set. Omit it to inherit the server " + "default; an empty list would leave this camera detecting nothing while looking " - + "deliberately configured." + + "deliberately configured. To stop this camera detecting, set Enabled to false." : null, (d, v) => d.Classes = [.. v]), new ListKnob( @@ -167,6 +187,11 @@ public void Apply(TBag bag, TTarget target) (d, v) => d.NoveltySeconds = v), ]; + /// The switch first, so a camera's resolved options read in the order the settings page + /// asks about them: whether, then how. + internal static readonly IReadOnlyList> Detection = + [DetectionSwitch, .. DetectionThresholds]; + /// /// The audio knobs write straight onto the resolved , replacing whole /// branches, because the three of them land on three different objects. The sound-gate knob diff --git a/Server/Serval.Server/Ingest/DetectSessionRestarts.cs b/Server/Serval.Server/Ingest/DetectSessionRestarts.cs new file mode 100644 index 0000000..363fb77 --- /dev/null +++ b/Server/Serval.Server/Ingest/DetectSessionRestarts.cs @@ -0,0 +1,65 @@ +using System.Collections.Concurrent; + +namespace Serval.Server.Ingest; + +/// +/// Lets the AI half ask for a camera's detect session to be started again. +/// +/// The raw frames object detection runs on are written by an ingest session and read by +/// , which supervises sessions of its own and restarts them when +/// a reader falls silent. That is the right response to a consumer that has wedged and no response +/// at all to a producer that has stopped: re-subscribing cannot conjure frames nobody is writing. +/// This is how the consumer reaches the one thing that can put them back. +/// +/// A signal rather than an injected , because both sides are +/// hosted services — resolving one from the other would build a second copy of it, supervising a +/// second set of ffmpegs. A singleton both of them take instead keeps each with one owner. +/// +/// Only the detect session. A camera's recording is supervised separately so that a sub +/// stream's trouble costs snapshots and AI and never footage, and recovering a detector by dropping +/// the recording alongside it would be the wrong trade every time — the more so because the recording +/// is usually a different stream, off a different connection, and perfectly healthy. +/// +public sealed class DetectSessionRestarts +{ + private readonly ConcurrentDictionary _attempts = new(StringComparer.Ordinal); + + /// + /// Ends 's running detect session so its supervisor starts a fresh + /// one, which re-probes the source and rebuilds the outputs from what it answers. + /// + /// Does nothing for a camera with no detect session in flight — one between attempts, or one + /// whose frames come from the recording session instead. Both are ordinary states, and a + /// request that lands in either is satisfied by the attempt that follows it. + /// + public void Request(string cameraId) + { + if (_attempts.TryGetValue(cameraId, out Action? restart)) + { + restart(); + } + } + + /// Publishes the running attempt's canceller, for as long as that attempt lasts. + internal IDisposable Register(string cameraId, Action restart) + { + _attempts[cameraId] = restart; + return new Registration(this, cameraId, restart); + } + + private sealed class Registration(DetectSessionRestarts owner, string cameraId, Action restart) + : IDisposable + { + /// + /// Removes this attempt's canceller and only this one. + /// + /// By key and value together because attempts overlap for a moment at the handover — the + /// next one registers before the one it replaces has finished unwinding — and a plain + /// remove-by-key from the outgoing attempt would take the incoming attempt's canceller with + /// it, leaving a session nothing could ask to restart. + /// + public void Dispose() => + ((ICollection>)owner._attempts) + .Remove(new KeyValuePair(cameraId, restart)); + } +} diff --git a/Server/Serval.Server/Ingest/FfmpegSnapshotSession.cs b/Server/Serval.Server/Ingest/FfmpegSnapshotSession.cs index 0aa5961..48379c6 100644 --- a/Server/Serval.Server/Ingest/FfmpegSnapshotSession.cs +++ b/Server/Serval.Server/Ingest/FfmpegSnapshotSession.cs @@ -63,12 +63,31 @@ public async Task RunAsync(CancellationToken cancellationToken) PreviewRing.Reset(_cameraDir); // Asked for the frame size, which both frame outputs need, and — now that the ring copies - // this stream to disk — for its codec, which decides whether the ring can exist at all. A - // source that will not answer costs the raw detect frames and the ring; the JPEG path needs - // neither. + // this stream to disk — for its codec, which decides whether the ring can exist at all. VideoProbe probe = await SourceProbe.VideoAsync( _stream.Url, _options.FfprobePath, _camera.Id, _logger, cancellationToken); + // A source that will not give its dimensions gets no session at all, rather than one built + // from the outputs that do not need them. The JPEG leg needs neither dimensions nor codec, + // so a snapshot-only session starts cleanly and stays up: ffmpeg is running and producing + // exactly what it was asked for, the wall and /snapshot.jpg are live, and every symptom of + // detection having no frames — and no way to ever get them, because nothing here fails and + // so nothing retries — surfaces somewhere else entirely, as an AI session restarting on its + // idle timeout for as long as the process lives. + // + // Thrown rather than raised as IngestConfigurationException: a stream that did not answer + // inside the probe timeout is a source problem that usually clears on its own, so this + // wants the supervisor's exponential backoff, not the go-to-the-cap-and-wait-for-an-edit + // path that exists for settings a human has to change. The recording half refuses an + // unplannable session on the same principle — see IngestPlanner.ResolveVideo. + if (_options.DetectFps > 0 && probe is not { Width: > 0, Height: > 0 }) + { + throw new InvalidOperationException( + $"Stream '{_stream.Name}' did not report its frame size, so this session could " + + "produce snapshots but no frames for object detection. Retrying until the probe " + + "answers."); + } + DetectFramePlan? detect = DetectFrameReader.Plan(_camera.Id, probe, _options, _logger); // What this session calls media offset zero, stamped immediately before ffmpeg starts for diff --git a/Server/Serval.Server/Ingest/StreamIngestManager.cs b/Server/Serval.Server/Ingest/StreamIngestManager.cs index 5cfb0c3..04b93a7 100644 --- a/Server/Serval.Server/Ingest/StreamIngestManager.cs +++ b/Server/Serval.Server/Ingest/StreamIngestManager.cs @@ -18,11 +18,16 @@ public sealed class StreamIngestManager : BackgroundService { private static readonly TimeSpan ReconcileInterval = TimeSpan.FromSeconds(5); + /// The supervised role that writes detect frames, and so the one + /// can reach. + private const string DetectRole = "detect"; + private readonly CameraRepository _cameras; private readonly RecordingIndex _recordings; private readonly SnapshotBroadcaster _snapshots; private readonly DetectFrameBroadcaster _detectFrames; private readonly PreviewRingIndex _previewRing; + private readonly DetectSessionRestarts _detectRestarts; private readonly IOptionsMonitor _options; private readonly FfmpegCapabilities _capabilities; private readonly string _mediaRoot; @@ -45,6 +50,7 @@ public StreamIngestManager( SnapshotBroadcaster snapshots, DetectFrameBroadcaster detectFrames, PreviewRingIndex previewRing, + DetectSessionRestarts detectRestarts, IOptionsMonitor options, FfmpegCapabilities capabilities, ILoggerFactory loggerFactory) @@ -54,6 +60,7 @@ public StreamIngestManager( _snapshots = snapshots; _detectFrames = detectFrames; _previewRing = previewRing; + _detectRestarts = detectRestarts; _options = options; _capabilities = capabilities; @@ -336,9 +343,21 @@ private Task RunSupervised( while (!cts.IsCancellationRequested) { + // One token per attempt, linked to the camera's. It is what lets a restart request + // end the session that is running without ending the loop that has to replace it — + // cancelling the camera's own token would stop supervising this role for good. + using var attempt = CancellationTokenSource.CreateLinkedTokenSource(cts.Token); + + // Only the detect role: it is the one that writes the frames the AI half reads, and + // so the only one anything asks to restart. A null registration for every other + // role keeps the using below honest without a branch around it. + using IDisposable? restartable = role == DetectRole + ? _detectRestarts.Register(camera.Id, () => RequestStop(attempt)) + : null; + try { - await run(cts.Token); + await run(attempt.Token); // A clean exit resets the backoff. backoff = TimeSpan.FromSeconds(_options.CurrentValue.Ingest.ReconnectSeconds); } @@ -346,6 +365,10 @@ private Task RunSupervised( { break; } + catch (OperationCanceledException) when (attempt.IsCancellationRequested) + { + // Handled below, with the clean-return ending of the same event. + } catch (IngestConfigurationException ex) { // Not a dead source — a camera or a server setting that cannot produce a @@ -365,6 +388,25 @@ private Task RunSupervised( camera.Id, role, backoff); } + // Reported here rather than from a catch, because a requested restart has two + // endings and this is the only place both arrive. Cancelling the token mid-write + // makes ffmpeg exit non-zero, and FfmpegRunner deliberately reports nothing when + // the token it was handed is the one that stopped it — so the usual ending is a + // clean return, and the exception is only what a cancel before ffmpeg started + // produces. + // + // Neither is a fault or a dead source, so the backoff earned by whatever the + // session was doing before the request is dropped; this goes round again at the + // base delay. + if (attempt.IsCancellationRequested && !cts.IsCancellationRequested) + { + sessionLogger.LogInformation( + "Camera {CameraId} {Role} stream restarting: its frames stopped reaching " + + "the AI session.", + camera.Id, role); + backoff = TimeSpan.FromSeconds(_options.CurrentValue.Ingest.ReconnectSeconds); + } + try { await Task.Delay(backoff, cts.Token); @@ -378,6 +420,25 @@ private Task RunSupervised( } }); + /// + /// Cancels one attempt's token on behalf of a restart request. + /// + /// Swallows the disposal race rather than locking against it: a request can read an attempt's + /// canceller a moment before that attempt ends on its own and disposes the token behind it. The + /// restart it was asking for is already happening, so there is nothing left to do and nothing + /// worth logging. + /// + private static void RequestStop(CancellationTokenSource attempt) + { + try + { + attempt.Cancel(); + } + catch (ObjectDisposedException) + { + } + } + /// /// Stops every session at once, on the way out. /// diff --git a/Server/Serval.Server/Program.cs b/Server/Serval.Server/Program.cs index f564303..32ddb48 100644 --- a/Server/Serval.Server/Program.cs +++ b/Server/Serval.Server/Program.cs @@ -301,6 +301,11 @@ // clips out of it. builder.Services.AddSingleton(); +// How the AI half asks ingest to rebuild a detect session whose frames have stopped reaching it. +// Registered unconditionally, and outside the AI block below: the ingest manager takes it whether +// or not anything is running detection, and a camera's detect session exists either way. +builder.Services.AddSingleton(); + builder.Services.AddHostedService(); builder.Services.AddHostedService();