feat: audio ai stack - onnx runtime, whisper, tts, audiogen, and audio analysis - #684
Conversation
Phase 1 - OnnxModel Infrastructure (#280): - Add IOnnxModel and IOnnxModelDownloader interfaces - Implement OnnxModel<T> wrapper with multi-provider support - Add OnnxTensorConverter for Tensor<T> <-> ONNX conversion - Implement OnnxModelDownloader for HuggingFace Hub - Support CPU, CUDA, TensorRT, DirectML execution providers Phase 2 - Audio Features (#396): - Add IAudioFeatureExtractor interface and base class - Implement MfccExtractor using existing MelSpectrogram - Implement ChromaExtractor for pitch class profiles - Implement SpectralFeatureExtractor (centroid, bandwidth, rolloff, flux, flatness, contrast, zcr) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Whisper ASR (#269): - WhisperModel with encoder-decoder ONNX pipeline - WhisperTokenizer with multilingual support (99+ languages) - WhisperOptions for model size, language, translate mode - Auto-download from HuggingFace Hub Text-to-Speech (#270): - TtsModel with acoustic model + vocoder pipeline - TtsPreprocessor for text normalization and G2P - Griffin-Lim fallback when neural vocoder unavailable - Speaking rate and pitch control 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
… recognition Implements remaining audio AI components: - AudioGenModel: Text-conditioned audio generation with classifier-free guidance, top-k/top-p sampling, and EnCodec decoder - Audio Fingerprinting: Chromaprint-style and Shazam-style peak-based fingerprinting with similarity matching - Music Analysis: Beat tracking, chord recognition using chroma templates, and key detection with Krumhansl-Kessler profiles - Speaker Recognition: Speaker embedding extraction, verification, identification, and diarization via agglomerative clustering Also fixes .NET Framework 4.7.1 compatibility: - Replace Math.Clamp with Math.Max/Math.Min pattern - Replace Math.Log2 with MathHelper.Log2 - Fix string.Split and string.Replace overloads - Add preprocessor directives for async Stream methods - Fix nullable reference type warnings with pattern matching Closes #271, #396 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add unit tests for all audio AI components: - ONNX model tests (OnnxModelOptions, OnnxExecutionProvider, OnnxModelDownloader) - Audio feature tests (MFCC, Chroma, Spectral features) - Fingerprinting tests (Chromaprint, Spectrogram fingerprinting) - Music analysis tests (BeatTracker, ChordRecognizer, KeyDetector) - Speaker recognition tests (SpeakerEmbeddingExtractor, SpeakerVerifier, SpeakerDiarizer) All 150 tests pass on net8.0. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add ConstantQTransform for music analysis with logarithmic frequency resolution - Add MusicSourceSeparator with HPSS-based harmonic-percussive separation - Add GenreClassifier for rule-based and model-based music genre classification - Add AudioEventDetector for AudioSet-style event detection - Add SceneClassifier for DCASE-style acoustic scene classification - Add SoundLocalizer with GCC-PHAT, MUSIC, and SRP-PHAT algorithms - Add OnnxExporter for exporting neural networks to ONNX format Addresses #280, #396 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughAdds a large audio subsystem: ONNX runtime tooling and downloader, a shared AudioNeuralNetworkBase for ONNX/native dual-mode models, many audio models (generative AudioGen/AudioLDM, TTS, Whisper/Wav2Vec2 ASR, LID), feature extractors, classifiers, fingerprinters, separation/enhancement/effects, localization/VAD/pitch, many interfaces/DTOs, helpers/enums, and unit tests. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant AudioGen as AudioGenModel<T>
participant TextEnc as Text Encoder (ONNX/native)
participant LM as Language Model (ONNX/native)
participant Decoder as Audio Decoder (ONNX/native)
Client->>AudioGen: GenerateAudio(prompt, options)
activate AudioGen
AudioGen->>TextEnc: EncodeText(prompt)
activate TextEnc
TextEnc-->>AudioGen: tokenEmbeddings
deactivate TextEnc
AudioGen->>LM: AutoregressiveGenerate(tokenEmbeddings, guidanceScale)
activate LM
note right of LM `#DDEBF7`: Generates discrete audio codes\n(possibly CFG-guided sampling)
LM-->>AudioGen: audioCodes
deactivate LM
AudioGen->>Decoder: DecodeAudio(audioCodes, duration)
activate Decoder
note right of Decoder `#F7F1E1`: Decodes codes → waveform\n(ONNX or native decoder)
Decoder-->>AudioGen: waveformTensor
deactivate Decoder
AudioGen-->>Client: Return waveformTensor
deactivate AudioGen
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (3 warnings)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🟡 Minor comments (16)
src/Audio/SourceSeparation/MusicSourceSeparator.cs-149-154 (1)
149-154: Cancellation token doesn't cancel in-progress separation.The
CancellationTokenis only passed toTask.Run, which means onceSeparatestarts executing, it cannot be interrupted. For long audio files, this could lead to unresponsive cancellation.Consider adding cancellation checks within the separation loops, or documenting this limitation.
src/Audio/SourceSeparation/MusicSourceSeparator.cs-55-58 (1)
55-58:IsReadyreturnsfalsefor CPU-only instances.When created via
CreateCpuOnly,_modelis null, soIsReadywill always returnfalseeven though the separator can still perform spectral-based separation. Consider clarifying the property documentation or adjusting the logic to reflect actual separation capability.🔎 Suggested fix
/// <summary> - /// Gets whether the model is ready for separation. + /// Gets whether the neural network model is loaded and ready. + /// Note: CPU-only instances can still perform separation via HPSS even when this returns false. /// </summary> - public bool IsReady => _model?.IsLoaded == true; + public bool IsReady => _model?.IsLoaded ?? false;Alternatively, add a separate property:
/// <summary> /// Gets whether the separator can perform separation (either model-based or CPU-based). /// </summary> public bool CanSeparate => !_disposed;src/Interfaces/IOnnxModelDownloader.cs-96-99 (1)
96-99: Minor: XML documentation says "Gets or sets" but property is read-only.The
FileProgressproperty is a computed read-only property (getter only), but the XML doc says "Gets or sets."🔎 Proposed fix
/// <summary> - /// Gets or sets the current file progress (0.0 to 1.0). + /// Gets the current file progress (0.0 to 1.0). /// </summary> public double FileProgress => TotalBytes > 0 ? (double)BytesDownloaded / TotalBytes : 0;src/Audio/Features/AudioFeatureExtractorBase.cs-72-96 (1)
72-96: Tensor shape assumption may cause issues.The
Extract(Vector<T> audio)method assumes the output tensor has exactly 2 dimensions (features.Shape[0]andfeatures.Shape[1]). If a derived class returns a different shape, this will throw anIndexOutOfRangeException.🔎 Proposed defensive check
public virtual Matrix<T> Extract(Vector<T> audio) { var audioTensor = new Tensor<T>([audio.Length]); for (int i = 0; i < audio.Length; i++) { audioTensor[i] = audio[i]; } var features = Extract(audioTensor); + if (features.Shape.Length != 2) + throw new InvalidOperationException($"Expected 2D tensor output, got {features.Shape.Length}D"); + // Convert to Matrix int frames = features.Shape[0]; int featureDim = features.Shape[1];src/Audio/Features/AudioFeatureExtractorBase.cs-119-135 (1)
119-135: Potential division by zero in window functions.When
lengthis 1, the expression(length - 1)becomes 0, causing a division by zero in2 * Math.PI * i / (length - 1).🔎 Proposed fix
protected T[] CreateHannWindow(int length) { + if (length <= 0) + throw new ArgumentOutOfRangeException(nameof(length), "Window length must be positive."); + if (length == 1) + return [NumOps.FromDouble(1.0)]; + var window = new T[length]; for (int i = 0; i < length; i++)src/Audio/TextToSpeech/TtsPreprocessor.cs-161-216 (1)
161-216: Integer overflow risk forMath.Abs(int.MinValue).When
numberisint.MinValue,Math.Abs(number)throwsOverflowExceptionbecauseint.MinValuehas no positive counterpart inint.🔎 Proposed fix
private static string NumberToWords(int number) { if (number == 0) return "zero"; - if (number < 0) return "minus " + NumberToWords(Math.Abs(number)); + if (number == int.MinValue) return "minus two billion one hundred forty seven million four hundred eighty three thousand six hundred forty eight"; + if (number < 0) return "minus " + NumberToWords(-number);src/Audio/Speaker/SpeakerVerifier.cs-185-236 (1)
185-236: ComputeCentroid returns reference to original embedding when count is 1.When only one embedding is provided, the method returns
embeddings[0]directly. If the caller later modifies this embedding, it could corrupt the enrolled speaker's centroid. Consider returning a copy.🔎 Proposed fix
private SpeakerEmbedding<T> ComputeCentroid(IReadOnlyList<SpeakerEmbedding<T>> embeddings) { if (embeddings.Count == 1) - return embeddings[0]; + { + // Return a copy to avoid external modifications affecting the enrolled centroid + var original = embeddings[0]; + return new SpeakerEmbedding<T> + { + Vector = (T[])original.Vector.Clone(), + Duration = original.Duration, + NumFrames = original.NumFrames + }; + }src/Audio/Classification/SceneClassifier.cs-272-281 (1)
272-281: Potential division by zero if audio produces only one frame.Line 280 divides by
(numFrames - 1). IfnumFrames == 1, this causes a division by zero.🔎 Proposed fix
// Delta (first derivative) double sumDelta = 0; for (int t = 1; t < numFrames; t++) { double prev = _numOps.ToDouble(mfccs[t - 1, c]); double curr = _numOps.ToDouble(mfccs[t, c]); sumDelta += Math.Abs(curr - prev); } - mfccDelta[c] = sumDelta / (numFrames - 1); + mfccDelta[c] = numFrames > 1 ? sumDelta / (numFrames - 1) : 0;src/Audio/TextToSpeech/TtsModel.cs-153-180 (1)
153-180: Missing cleanup on partial failure inFromFiles.If
OnnxModel<T>construction for the vocoder (line 168) orGriffinLim<T>(line 173-176) throws afteracousticModelis created (line 162), the acoustic model won't be disposed.🔎 Proposed fix
public static TtsModel<T> FromFiles( string acousticModelPath, string? vocoderPath = null, TtsOptions? options = null) { options ??= new TtsOptions(); options.AcousticModelPath = acousticModelPath; options.VocoderModelPath = vocoderPath; var acousticModel = new OnnxModel<T>(acousticModelPath, options.OnnxOptions); OnnxModel<T>? vocoder = null; GriffinLim<T>? griffinLim = null; + try + { if (vocoderPath is not null && vocoderPath.Length > 0) { vocoder = new OnnxModel<T>(vocoderPath, options.OnnxOptions); } if (options.UseGriffinLimFallback || vocoder is null) { griffinLim = new GriffinLim<T>( nFft: options.FftSize, hopLength: options.HopLength, iterations: options.GriffinLimIterations); } return new TtsModel<T>(options, acousticModel, vocoder, griffinLim); + } + catch + { + acousticModel.Dispose(); + vocoder?.Dispose(); + throw; + } }src/Audio/Features/MfccExtractor.cs-51-60 (1)
51-60:FeatureDimensioncalculation is incorrect when both deltas are enabled.When
_appendDelta = trueand_appendDeltaDelta = true:
- Line 56:
dim = 13 * 2 = 26- Line 57:
dim = 26 / 2 * 3 = 39But the actual output has
13 + 13 + 13 = 39features. The math works here, but the logic is confusing and fragile. If_appendDeltaDeltais enabled without_appendDelta, it produces13 / 2 * 3 = 18(integer division), which doesn't match the actual behavior.🔎 Proposed clearer implementation
public override int FeatureDimension { get { int dim = _numCoefficients; - if (_appendDelta) dim *= 2; - if (_appendDeltaDelta) dim = dim / 2 * 3; + if (_appendDelta) dim += _numCoefficients; + if (_appendDeltaDelta) dim += _numCoefficients; return dim; } }src/Audio/Classification/AudioEventDetector.cs-321-330 (1)
321-330: Guard against division by zero inComputeEnergy.If an empty audio tensor is passed,
audio.Lengthwill be 0, causing a division-by-zero.🔎 Suggested fix
private double ComputeEnergy(Tensor<T> audio) { + if (audio.Length == 0) return 0; + double sum = 0; for (int i = 0; i < audio.Length; i++) { double val = _numOps.ToDouble(audio[i]); sum += val * val; } return sum / audio.Length; }src/Audio/Classification/GenreClassifier.cs-258-266 (1)
258-266: Guard against division by zero inComputeMean.If an empty tensor is passed, this will cause a division-by-zero exception.
🔎 Suggested fix
private double ComputeMean(Tensor<T> tensor) { + if (tensor.Length == 0) return 0; + double sum = 0; for (int i = 0; i < tensor.Length; i++) { sum += _numOps.ToDouble(tensor[i]); } return sum / tensor.Length; }src/Onnx/OnnxModelDownloader.cs-400-407 (1)
400-407: Misleading constant name inOnnxModelRepositories.AudioGen.
AudioGen.Smallis assigned the value"facebook/audiogen-medium", which is contradictory. Either the constant should be renamed toMediumor the value should point to a "small" model.🔎 Suggested fix
public static class AudioGen { - /// <summary>AudioGen small model.</summary> - public const string Small = "facebook/audiogen-medium"; + /// <summary>AudioGen medium model.</summary> + public const string Medium = "facebook/audiogen-medium"; /// <summary>MusicGen small model.</summary> public const string MusicGenSmall = "facebook/musicgen-small"; }src/Audio/Classification/AudioEventDetector.cs-332-343 (1)
332-343: Guard against division by zero inComputeZeroCrossingRate.Similar to
ComputeEnergy, this method will throw if an empty tensor is passed.🔎 Suggested fix
private double ComputeZeroCrossingRate(Tensor<T> audio) { + if (audio.Length <= 1) return 0; + int crossings = 0; for (int i = 1; i < audio.Length; i++) { double prev = _numOps.ToDouble(audio[i - 1]); double curr = _numOps.ToDouble(audio[i]); if ((prev >= 0 && curr < 0) || (prev < 0 && curr >= 0)) crossings++; } - return (double)crossings / audio.Length; + return (double)crossings / (audio.Length - 1); }Note: The normalization should arguably be
audio.Length - 1since that's the number of transitions examined.src/Audio/Whisper/WhisperModel.cs-201-206 (1)
201-206:DetectedLanguagereturns configured language, not detected.When
_options.Languageis null or when the model could detect a different language than configured, this field is misleading. Consider renaming toLanguageor implementing actual language detection if Whisper's language detection output is available.src/Onnx/OnnxModel.cs-246-251 (1)
246-251: Returning empty tensor on null output may cause downstream errors.If
outputis null, returningnew Tensor<T>([0])could cause shape mismatches or unexpected behavior in calling code. Consider throwing an exception with a descriptive message instead.🔎 Proposed fix
- return output is not null - ? OnnxTensorConverter.FromOnnxFloat<T>(output) - : new Tensor<T>([0]); + if (output is null) + throw new InvalidOperationException($"ONNX model returned null output for input '{inputName}'."); + + return OnnxTensorConverter.FromOnnxFloat<T>(output);
🧹 Nitpick comments (38)
src/Audio/Features/ConstantQTransform.cs (2)
83-119: Add input parameter validation for robustness.The constructor accepts several parameters but does not validate them. Invalid values could lead to division by zero, invalid array allocations, or nonsensical results. Consider adding validation:
sampleRate > 0fMin > 0binsPerOctave > 0(critical: line 101 divides by this)numOctaves > 0hopLength > 0🔎 Suggested validation
public ConstantQTransform( int sampleRate = 22050, double fMin = 32.70, int binsPerOctave = 12, int numOctaves = 7, int hopLength = 512, WindowType windowType = WindowType.Hann) { + if (sampleRate <= 0) + throw new ArgumentOutOfRangeException(nameof(sampleRate), "Sample rate must be positive."); + if (fMin <= 0) + throw new ArgumentOutOfRangeException(nameof(fMin), "Minimum frequency must be positive."); + if (binsPerOctave <= 0) + throw new ArgumentOutOfRangeException(nameof(binsPerOctave), "Bins per octave must be positive."); + if (numOctaves <= 0) + throw new ArgumentOutOfRangeException(nameof(numOctaves), "Number of octaves must be positive."); + if (hopLength <= 0) + throw new ArgumentOutOfRangeException(nameof(hopLength), "Hop length must be positive."); + _numOps = MathHelper.GetNumericOperations<T>(); _sampleRate = sampleRate;
316-334: Consider adding bounds checking for bin index parameters.Both
GetMidiNoteandGetNoteNameaccept abinIndexparameter but don't validate that it's within the valid range[0, _numBins). While callers should provide valid indices, adding a bounds check would provide clearer error messages for invalid usage.🔎 Suggested bounds check
public double GetMidiNote(int binIndex) { + if (binIndex < 0 || binIndex >= _numBins) + throw new ArgumentOutOfRangeException(nameof(binIndex), + $"Bin index must be between 0 and {_numBins - 1}."); double frequency = _frequencies[binIndex]; return 69 + 12 * (Math.Log(frequency / 440.0) / Math.Log(2)); }src/Audio/SourceSeparation/MusicSourceSeparator.cs (2)
417-418: Unused parameterharmonic.The
harmonicparameter is declared but never used in this method. Consider removing it.🔎 Proposed fix
-private Tensor<T> ExtractBassline(Tensor<T> harmonic, Tensor<T> harmonicMag, Tensor<T> phase) +private Tensor<T> ExtractBassline(Tensor<T> harmonicMag, Tensor<T> phase)Update the call site on line 212:
- Bass = ExtractBassline(harmonic, harmonicMag, phase), + Bass = ExtractBassline(harmonicMag, phase),
444-485: Silent fallback when mask dimensions don't match.The method assumes 4 stems and silently uses
mask=0when dimensions don't match (line 465). If the ONNX model outputs unexpected shapes, this could produce empty stems without any warning.Consider adding a log warning when the mask shape doesn't match expected dimensions to aid debugging.
src/Interfaces/IAudioFeatureExtractor.cs (1)
66-97: Consider adding validation for configuration values.The options class lacks validation for edge cases that could cause downstream issues (e.g., negative or zero values for
SampleRate,FftSize, orHopLength). Since these would cause division-by-zero or invalid array allocations in extractors, consider adding guards.🔎 Proposed validation approach
public class AudioFeatureOptions { + private int _sampleRate = 16000; + private int _fftSize = 512; + private int _hopLength = 160; + /// <summary> /// Gets or sets the sample rate of the audio. /// </summary> - public int SampleRate { get; set; } = 16000; + public int SampleRate + { + get => _sampleRate; + set => _sampleRate = value > 0 ? value : throw new ArgumentOutOfRangeException(nameof(value), "SampleRate must be positive."); + }src/Audio/MusicAnalysis/BeatTracker.cs (2)
133-167: Edge case: MinTempo > MaxTempo could cause unexpected behavior.If
MinTempoexceedsMaxTempoin options,minLagwill be greater thanmaxLag, causing the loop at line 143 to never execute andbestLagto remain atminLag, potentially producing an invalid tempo.🔎 Add validation in constructor or EstimateTempo
public BeatTracker(BeatTrackerOptions? options = null) { _numOps = MathHelper.GetNumericOperations<T>(); _options = options ?? new BeatTrackerOptions(); + + if (_options.MinTempo >= _options.MaxTempo) + throw new ArgumentException("MinTempo must be less than MaxTempo.");
107-112: Hardcoded feature index creates fragile coupling.The spectral flux is accessed via hardcoded index
4, which assumes a specific feature order fromSpectralFeatureExtractor. If that extractor's feature ordering changes, this will silently produce incorrect onset envelopes.🔎 Consider using named feature access or a constant
+ // Spectral flux is at index 4 in SpectralFeatureExtractor output: + // [0]=Centroid, [1]=Bandwidth, [2]=Rolloff, [3]=Flux, [4]=Flatness + private const int SpectralFluxIndex = 3; // Verify against SpectralFeatureExtractor + private double[] ComputeOnsetEnvelope(Tensor<T> audio) { // ... for (int f = 0; f < numFrames; f++) { - // Use spectral flux (index 4) as onset strength - onsetEnvelope[f] = Math.Max(0, _numOps.ToDouble(features[f, 4])); + onsetEnvelope[f] = Math.Max(0, _numOps.ToDouble(features[f, SpectralFluxIndex])); }Verify the correct index for spectral flux in
SpectralFeatureExtractor:#!/bin/bash # Check the feature ordering in SpectralFeatureExtractor rg -n "Flux|flux" --type=cs -A2 -B2 src/Audio/Features/SpectralFeatureExtractor.cssrc/Audio/Fingerprinting/ChromaprintFingerprinter.cs (2)
96-112: Storing hash as generic type T may lose precision.Converting
uinthash toTvia_numOps.FromDouble(hash)could lose precision whenTisfloat(which has ~7 significant digits) for hash values exceeding ~16 million. This affects theDataarray but not theHasharray which correctly storesuint[].This is acceptable if
Datais primarily used for correlation-based similarity (which normalizes values anyway), but worth documenting.
212-248: Cross-correlation alignment has O(n·m) complexity.For long audio files, the hash arrays can be large, and the nested loop scanning all offsets may become slow. This is acceptable for typical use cases but worth noting for very long recordings.
Consider documenting performance characteristics or adding an optional early-exit threshold for production use with long audio.
src/Audio/Localization/SoundLocalizer.cs (2)
75-78: Usestring.IsNullOrEmptyfor consistency and clarity.The current check
_options.ModelPath is not null && _options.ModelPath.Length > 0works but is verbose. The idiomatic C# approach is clearer.🔎 Proposed simplification
- if (_options.ModelPath is not null && _options.ModelPath.Length > 0) + if (!string.IsNullOrEmpty(_options.ModelPath)) { _model = new OnnxModel<T>(_options.ModelPath, _options.OnnxOptions); }
510-518: Confidence calculation may produce unexpected results with negative GCC values.
gcc.Average()can be negative when GCC-PHAT has negative correlation values, makingMath.Abs(mean) + 0.01the denominator. While this works, usingpeak - meanor the standard deviation might provide a more meaningful sharpness metric.tests/AiDotNet.Tests/Audio/Features/MfccExtractorTests.cs (1)
130-163: Frequency discrimination test could be more robust.The test only compares the first 5 coefficients of the first frame with a threshold of 0.1. Consider comparing more frames or using a statistical measure across all frames to reduce brittleness.
tests/AiDotNet.Tests/Onnx/OnnxModelTests.cs (2)
47-55: Test verifies no exception but not cache directory behavior.The test only asserts the downloader is not null. Consider also verifying that a default cache directory property is accessible or set.
191-219: Good coverage of factory methods, but missing ForTensorRT test.The AI summary mentions
ForTensorRTas a factory method, but there's no test for it here. Consider adding one for completeness.🔎 Add test for ForTensorRT
[Fact] public void OnnxModelOptions_ForTensorRT_SetsCorrectProvider() { // Act var options = OnnxModelOptions.ForTensorRT(); // Assert Assert.Equal(OnnxExecutionProvider.TensorRT, options.ExecutionProvider); }tests/AiDotNet.Tests/Audio/Fingerprinting/FingerprintingTests.cs (2)
63-88: Consider extracting audio generation into reusable helpers.Lines 69-78 duplicate the signal generation pattern that could be generalized in the
CreateTestAudiohelper by adding a frequency parameter. This would reduce duplication and improve maintainability.🔎 Proposed refactor
- private static Tensor<float> CreateTestAudio(int sampleRate = 22050, double durationSeconds = 5.0) + private static Tensor<float> CreateTestAudio(int sampleRate = 22050, double durationSeconds = 5.0, double frequency = 440) { int numSamples = (int)(sampleRate * durationSeconds); var audio = new Tensor<float>([numSamples]); - // Create a complex audio signal with multiple frequencies for (int i = 0; i < numSamples; i++) { double t = (double)i / sampleRate; - // Mix of frequencies to create more interesting fingerprint - audio[i] = (float)( - 0.5 * Math.Sin(2 * Math.PI * 440 * t) + - 0.3 * Math.Sin(2 * Math.PI * 880 * t) + - 0.2 * Math.Sin(2 * Math.PI * 1320 * t)); + audio[i] = (float)Math.Sin(2 * Math.PI * frequency * t); } return audio; } + + private static Tensor<float> CreateComplexTestAudio(int sampleRate = 22050, double durationSeconds = 5.0) + { + int numSamples = (int)(sampleRate * durationSeconds); + var audio = new Tensor<float>([numSamples]); + + for (int i = 0; i < numSamples; i++) + { + double t = (double)i / sampleRate; + audio[i] = (float)( + 0.5 * Math.Sin(2 * Math.PI * 440 * t) + + 0.3 * Math.Sin(2 * Math.PI * 880 * t) + + 0.2 * Math.Sin(2 * Math.PI * 1320 * t)); + } + + return audio; + }
90-135: Consider adding symmetry for SpectrogramFingerprinter tests.The
SpectrogramFingerprinterhas fewer tests compared toChromaprintFingerprinter. Consider adding tests forDifferentAudio_DifferentFingerprintand similarity computation to ensure parity in coverage.src/Audio/AudioGen/AudioGenOptions.cs (1)
35-43: Consider documenting the relationship between DurationSeconds and MaxDurationSeconds.It's not immediately clear what happens if
DurationSecondsexceedsMaxDurationSeconds. Consider adding XML documentation clarifying whether validation occurs or which value takes precedence.src/Audio/Whisper/WhisperTokenizer.cs (1)
189-194: Consider validating negative time values.
GetTimestampTokendoesn't validate iftimeSecondsis negative, which would produce invalid token IDs (less thanTimestampBeginId).🔎 Proposed fix
public int GetTimestampToken(double timeSeconds) { + if (timeSeconds < 0) + { + throw new ArgumentOutOfRangeException(nameof(timeSeconds), "Time must be non-negative."); + } + // Whisper uses 20ms precision for timestamps int index = (int)(timeSeconds / 0.02); return TimestampBeginId + index; }src/Audio/MusicAnalysis/KeyDetector.cs (1)
104-112: Consider using Span or avoiding element-by-element copy.The
Vector<T>overload copies each element individually to a new tensor. If performance becomes a concern for large audio, consider whether a more efficient conversion is possible.src/Audio/TextToSpeech/TtsPreprocessor.cs (1)
45-68: Placeholder dictionary acknowledged; consider documenting production expectations.The comments appropriately indicate this is a simplified implementation. The phoneme IDs in
WordToPhonemesandCharToPhonemeconflict (e.g.,CharToPhoneme['a'] = 1andWordToPhonemes["hello"] = [1, 2, 3, 4]). This is fine for placeholder purposes but could cause confusion if extended without refactoring.src/Audio/Features/AudioFeatureExtractorBase.cs (1)
188-203: Minor inefficiency in PadAudioCenter.The loop initializing
paddedwith zeros is unnecessary sincenew T[]already initializes value types to their default (zero) values.🔎 Proposed simplification
protected T[] PadAudioCenter(T[] audio) { int padAmount = WindowLength / 2; var padded = new T[audio.Length + 2 * padAmount]; - // Initialize with zeros - for (int i = 0; i < padded.Length; i++) - { - padded[i] = NumOps.Zero; - } - // Copy audio to center Array.Copy(audio, 0, padded, padAmount, audio.Length); return padded; }src/Interfaces/IAudioFingerprinter.cs (1)
77-113: Consider usinginitaccessor for immutable properties after construction.Properties like
Duration,SampleRate, andAlgorithmare typically set once during fingerprint creation. Usinginitaccessors would better express this intent and prevent accidental modification.🔎 Proposed change
- public double Duration { get; set; } + public double Duration { get; init; } - public int SampleRate { get; set; } + public int SampleRate { get; init; } - public string Algorithm { get; set; } = string.Empty; + public string Algorithm { get; init; } = string.Empty;src/Audio/Features/SpectralFeatureExtractor.cs (1)
107-116: Redundant centroid computation when both Centroid and Bandwidth are enabled.Spectral centroids are computed twice: once for the Centroid feature and once for Bandwidth calculation. Consider caching the result.
🔎 Proposed optimization
// Extract features var features = new List<double[]>(); +double[]? centroids = null; + +// Compute centroids if needed by either feature +if (_featureTypes.HasFlag(SpectralFeatureType.Centroid) || + _featureTypes.HasFlag(SpectralFeatureType.Bandwidth)) +{ + centroids = ComputeSpectralCentroid(magnitude, freqBins, numFrames, numFreqs); +} if (_featureTypes.HasFlag(SpectralFeatureType.Centroid)) { - features.Add(ComputeSpectralCentroid(magnitude, freqBins, numFrames, numFreqs)); + features.Add(centroids!); } if (_featureTypes.HasFlag(SpectralFeatureType.Bandwidth)) { - var centroids = ComputeSpectralCentroid(magnitude, freqBins, numFrames, numFreqs); features.Add(ComputeSpectralBandwidth(magnitude, freqBins, centroids, numFrames, numFreqs)); }src/Audio/Fingerprinting/SpectrogramFingerprinter.cs (1)
174-225: UnusedhashToTimedictionary.The
hashToTimedictionary is populated but never used in this method or returned in the fingerprint. Either remove it or include it in the fingerprint metadata for debugging/matching purposes.🔎 Option 1: Remove unused variable
private AudioFingerprint<T> CreateFingerprintFromPeaks(List<SpectralPeak> peaks, int audioLength) { var hashes = new List<uint>(); - var hashToTime = new Dictionary<uint, double>(); // Create hashes from peak pairs (anchor + target) for (int i = 0; i < peaks.Count; i++) { var anchor = peaks[i]; // Find target peaks within the target zone for (int j = i + 1; j < peaks.Count; j++) { var target = peaks[j]; int timeDiff = target.Frame - anchor.Frame; if (timeDiff < _options.TargetZoneStart) continue; if (timeDiff > _options.TargetZoneEnd) break; // Create hash: freq1 (10 bits) | freq2 (10 bits) | time delta (12 bits) uint hash = CreateHash(anchor.Bin, target.Bin, timeDiff); hashes.Add(hash); - - // Store time for this hash - double time = anchor.Frame * _options.HopLength / (double)_options.SampleRate; - if (!hashToTime.ContainsKey(hash)) - { - hashToTime[hash] = time; - } } }tests/AiDotNet.Tests/Audio/Speaker/SpeakerTests.cs (2)
36-50: Consider disposing IDisposable instances in tests.
SpeakerEmbeddingExtractor<T>implementsIDisposable. While xUnit handles some cleanup, explicitly disposing or usingusingstatements ensures resources are released promptly, especially if the extractor holds ONNX sessions or other native resources.🔎 Example fix for one test
[Fact] public void SpeakerEmbeddingExtractor_Extract_ReturnsEmbedding() { // Arrange - var extractor = new SpeakerEmbeddingExtractor<float>(); - var audio = CreateSpeakerAudio(); + using var extractor = new SpeakerEmbeddingExtractor<float>(); + var audio = CreateSpeakerAudio(); // Act var embedding = extractor.Extract(audio);
226-240: DisposeSpeakerDiarizerinstances after use.Same as above—
SpeakerDiarizer<T>implementsIDisposableand owns aSpeakerEmbeddingExtractor. Useusingto ensure cleanup.src/Onnx/OnnxTensorConverter.cs (1)
32-47: Unused variableshapeon line 35.The
shapevariable is computed but never used. TheDenseTensorconstructor on line 36 usestensor.Shapedirectly.🔎 Proposed fix
public static OnnxTensors.DenseTensor<float> ToOnnxFloat<T>(Tensor<T> tensor) { var numOps = MathHelper.GetNumericOperations<T>(); - var shape = tensor.Shape.Select(d => (long)d).ToArray(); var onnxTensor = new OnnxTensors.DenseTensor<float>(tensor.Shape); var sourceData = tensor.ToArray();src/Audio/Classification/AudioEventDetector.cs (2)
120-131: CreateAsync mutates the passed-in options object.The
options.ModelPathis assigned in theCreateAsyncmethod, which mutates the caller's options object if they passed one. This could lead to unexpected side effects.🔎 Suggested fix: Clone options before mutation
public static async Task<AudioEventDetector<T>> CreateAsync( AudioEventDetectorOptions? options = null, IProgress<double>? progress = null, CancellationToken cancellationToken = default) { options ??= new AudioEventDetectorOptions(); + + // Clone to avoid mutating caller's options + options = new AudioEventDetectorOptions + { + SampleRate = options.SampleRate, + FftSize = options.FftSize, + HopLength = options.HopLength, + NumMels = options.NumMels, + FMin = options.FMin, + FMax = options.FMax, + WindowSize = options.WindowSize, + WindowOverlap = options.WindowOverlap, + Threshold = options.Threshold, + CustomLabels = options.CustomLabels, + ModelPath = options.ModelPath, + OnnxOptions = options.OnnxOptions + }; if (options.ModelPath is null || options.ModelPath.Length == 0) {
146-150: Potential timing mismatch between window start calculation and hop logic.The
startTimecalculation at line 149 computeswindowIdx * _options.WindowSize * (1 - _options.WindowOverlap), but this should ideally use the same hop calculation asSplitIntoWindowsfor consistency:windowIdx * hopSamples / _options.SampleRate.🔎 Suggested fix
for (int windowIdx = 0; windowIdx < windows.Count; windowIdx++) { var window = windows[windowIdx]; - double startTime = windowIdx * _options.WindowSize * (1 - _options.WindowOverlap); + int windowSamples = (int)(_options.WindowSize * _options.SampleRate); + int hopSamples = (int)(windowSamples * (1 - _options.WindowOverlap)); + double startTime = (double)(windowIdx * hopSamples) / _options.SampleRate;src/Audio/Speaker/SpeakerEmbeddingExtractor.cs (1)
278-298: Similarity calculation silently truncates to minimum length.When comparing embeddings of different lengths,
CosineSimilarityusesMath.Min(Vector.Length, other.Vector.Length). This could lead to incorrect similarity scores if embeddings were extracted with differentEmbeddingDimensionsettings. Consider throwing or warning when lengths differ.🔎 Suggested fix - add validation
public double CosineSimilarity(SpeakerEmbedding<T> other) { + if (Vector.Length != other.Vector.Length) + { + throw new ArgumentException( + $"Embedding dimensions must match. Got {Vector.Length} and {other.Vector.Length}.", + nameof(other)); + } + double dot = 0; double norm1 = 0; double norm2 = 0; - int len = Math.Min(Vector.Length, other.Vector.Length); + int len = Vector.Length;tests/AiDotNet.Tests/Audio/MusicAnalysis/MusicAnalysisTests.cs (1)
119-132: Test validates internal consistency rather than correctness.The
BeatTracker_AverageBeatInterval_MatchesTempotest calculatesexpectedInterval = 60.0 / result.Tempoand compares it toresult.AverageBeatInterval. This only verifies that these two properties are consistent with each other, not that either value is correct. Consider adding a test that validates against the known input BPM (120).🔎 Suggested additional test
[Fact] public void BeatTracker_DetectedTempo_CloseToInputBpm() { // Arrange double inputBpm = 120; var tracker = new BeatTracker<float>(); var audio = CreateRhythmicAudio(bpm: inputBpm); // Act var result = tracker.Track(audio); // Assert - detected tempo should be within 10% of input Assert.True(Math.Abs(result.Tempo - inputBpm) < inputBpm * 0.1, $"Detected tempo {result.Tempo} should be close to input {inputBpm}"); }src/Audio/AudioGen/AudioGenModel.cs (1)
373-386: Quadratic memory allocation in autoregressive loop.Each iteration creates a new tensor and copies all previous values. For
numTokens = DurationSeconds * 50tokens, this results in O(n²) memory allocations and copies. Consider pre-allocating a larger buffer or using a different data structure.🔎 Suggested optimization
+ // Pre-allocate for all tokens + var allTokens = new Tensor<T>([1, numCodebooks, numTokens + 1]); + + // Initialize with start token for (int cb = 0; cb < numCodebooks; cb++) { - currentTokens[0, cb, 0] = _numOps.FromDouble(0); + allTokens[0, cb, 0] = _numOps.FromDouble(0); } for (int t = 0; t < numTokens; t++) { + // Create view of current tokens (0 to t inclusive) + var currentLength = t + 1; + var currentTokens = new Tensor<T>([1, numCodebooks, currentLength]); + for (int cb = 0; cb < numCodebooks; cb++) + { + for (int i = 0; i < currentLength; i++) + { + currentTokens[0, cb, i] = allTokens[0, cb, i]; + } + } + // ... rest of loop ... // Store generated tokens directly for (int cb = 0; cb < numCodebooks; cb++) { int nextToken = SampleFromLogits(logits, cb, random); codes[0, cb, t] = _numOps.FromDouble(nextToken); - - // Update current tokens for next iteration - if (t < numTokens - 1) - { - // ... expensive reallocation ... - } + allTokens[0, cb, t + 1] = _numOps.FromDouble(nextToken); } }Note: A better approach would be if the language model can accept a padded tensor with an attention mask.
src/Onnx/OnnxModelDownloader.cs (1)
247-261: Error handling after resume check could leave partial file in inconsistent state.If the server returns an error status other than 416 (e.g., 404 Not Found),
EnsureSuccessStatusCode()throws, but the.partialfile isn't cleaned up. Subsequent download attempts might try to resume from a corrupted state.🔎 Suggested improvement
// Handle non-resumable download if (response.StatusCode == System.Net.HttpStatusCode.RequestedRangeNotSatisfiable) { existingLength = 0; File.Delete(tempPath); } - else + else if (!response.IsSuccessStatusCode) { - response.EnsureSuccessStatusCode(); + // Clean up partial file on error + if (File.Exists(tempPath)) + { + File.Delete(tempPath); + } + response.EnsureSuccessStatusCode(); // Will throw }src/Onnx/OnnxExporter.cs (2)
34-37: Unused constants.
IrVersion(line 35) is defined but the hardcoded value8is used directly inBuild()at line 521. Consider using the constant for consistency.🔎 Suggested fix
- modelBytes.AddRange(CreateVarintField(FieldIrVersion, 8)); // ir_version + modelBytes.AddRange(CreateVarintField(FieldIrVersion, OnnxExporter.IrVersion)); // ir_versionNote: This requires making
IrVersionaccessible toOnnxModelBuilder, either by passing it as a parameter or making the constant internal.
247-249: Silently skipping unsupported layers could produce incorrect models.Returning
nullfor unsupported layers causes them to be silently skipped, which could result in a disconnected graph or incorrect model behavior. Consider logging a warning or throwing for critical layers.🔎 Suggested improvement
if (layerTypeName.Contains("Flatten")) { var outputName = $"flatten_{nodeIndex}"; builder.AddFlatten(inputName, outputName); return outputName; } - // Skip unsupported layers + // Log warning for unsupported layers - they will be skipped + System.Diagnostics.Debug.WriteLine( + $"Warning: Skipping unsupported layer type '{layerTypeName}' at index {nodeIndex}"); + return null; }Alternatively, consider throwing
NotSupportedExceptionwith the layer type name to fail fast.src/Audio/Whisper/WhisperModel.cs (1)
244-277: Consider usingSpan<T>orArray.Copyfor better performance.The element-by-element copy loop works correctly but could be optimized. However, this is a minor optimization and the current implementation is correct.
src/Onnx/OnnxModel.cs (2)
259-270: Silent exception swallowing hinders debugging.Catching and ignoring all exceptions when trying providers makes it difficult to diagnose why a specific provider failed. Consider logging the exception or collecting failures to report if all providers fail.
🔎 Proposed improvement
+ var exceptions = new List<Exception>(); foreach (var provider in providers) { try { var sessionOptions = CreateSessionOptions(provider); var session = new InferenceSession(modelPath, sessionOptions); return (session, provider.ToString()); } - catch (Exception) + catch (Exception ex) { - // Try next provider + exceptions.Add(ex); continue; } } // Fallback to CPU - var cpuOptions = CreateSessionOptions(OnnxExecutionProvider.Cpu); - return (new InferenceSession(modelPath, cpuOptions), "CPU"); + try + { + var cpuOptions = CreateSessionOptions(OnnxExecutionProvider.Cpu); + return (new InferenceSession(modelPath, cpuOptions), "CPU"); + } + catch (Exception ex) + { + throw new AggregateException( + "Failed to create ONNX session with any provider.", + exceptions.Append(ex)); + }
517-545: Inefficient memory handling for large model downloads.Using
List<byte>withAddRange(buffer.Take(bytesRead))is inefficient:
Take()creates an intermediate enumerable each iterationList<byte>may reallocate multiple times during download- For large models (hundreds of MB), this causes significant memory pressure
🔎 Proposed fix using MemoryStream
- var bytes = new List<byte>(); + using var memoryStream = new MemoryStream(); #if NET6_0_OR_GREATER using var stream = await response.Content.ReadAsStreamAsync(cancellationToken); #else using var stream = await response.Content.ReadAsStreamAsync(); #endif var buffer = new byte[8192]; int bytesRead; long totalBytesRead = 0; #if NET6_0_OR_GREATER while ((bytesRead = await stream.ReadAsync(buffer.AsMemory(0, buffer.Length), cancellationToken)) > 0) #else while ((bytesRead = await stream.ReadAsync(buffer, 0, buffer.Length, cancellationToken)) > 0) #endif { - bytes.AddRange(buffer.Take(bytesRead)); + memoryStream.Write(buffer, 0, bytesRead); totalBytesRead += bytesRead; if (totalBytes > 0) { progress?.Report((double)totalBytesRead / totalBytes); } } progress?.Report(1.0); - return new OnnxModel<T>([.. bytes], options); + return new OnnxModel<T>(memoryStream.ToArray(), options);
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (44)
src/Audio/AudioGen/AudioGenModel.cssrc/Audio/AudioGen/AudioGenOptions.cssrc/Audio/Classification/AudioEventDetector.cssrc/Audio/Classification/GenreClassifier.cssrc/Audio/Classification/SceneClassifier.cssrc/Audio/Features/AudioFeatureExtractorBase.cssrc/Audio/Features/ChromaExtractor.cssrc/Audio/Features/ConstantQTransform.cssrc/Audio/Features/MfccExtractor.cssrc/Audio/Features/SpectralFeatureExtractor.cssrc/Audio/Fingerprinting/ChromaprintFingerprinter.cssrc/Audio/Fingerprinting/SpectrogramFingerprinter.cssrc/Audio/Localization/SoundLocalizer.cssrc/Audio/MusicAnalysis/BeatTracker.cssrc/Audio/MusicAnalysis/ChordRecognizer.cssrc/Audio/MusicAnalysis/KeyDetector.cssrc/Audio/SourceSeparation/MusicSourceSeparator.cssrc/Audio/Speaker/SpeakerDiarizer.cssrc/Audio/Speaker/SpeakerEmbeddingExtractor.cssrc/Audio/Speaker/SpeakerVerifier.cssrc/Audio/TextToSpeech/TtsModel.cssrc/Audio/TextToSpeech/TtsOptions.cssrc/Audio/TextToSpeech/TtsPreprocessor.cssrc/Audio/Whisper/WhisperModel.cssrc/Audio/Whisper/WhisperOptions.cssrc/Audio/Whisper/WhisperTokenizer.cssrc/Interfaces/IAudioFeatureExtractor.cssrc/Interfaces/IAudioFingerprinter.cssrc/Interfaces/IOnnxModel.cssrc/Interfaces/IOnnxModelDownloader.cssrc/Onnx/OnnxExecutionProvider.cssrc/Onnx/OnnxExporter.cssrc/Onnx/OnnxModel.cssrc/Onnx/OnnxModelDownloader.cssrc/Onnx/OnnxModelMetadata.cssrc/Onnx/OnnxModelOptions.cssrc/Onnx/OnnxTensorConverter.cstests/AiDotNet.Tests/Audio/Features/ChromaExtractorTests.cstests/AiDotNet.Tests/Audio/Features/MfccExtractorTests.cstests/AiDotNet.Tests/Audio/Features/SpectralFeatureExtractorTests.cstests/AiDotNet.Tests/Audio/Fingerprinting/FingerprintingTests.cstests/AiDotNet.Tests/Audio/MusicAnalysis/MusicAnalysisTests.cstests/AiDotNet.Tests/Audio/Speaker/SpeakerTests.cstests/AiDotNet.Tests/Onnx/OnnxModelTests.cs
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-12-18T08:49:25.295Z
Learnt from: ooples
Repo: ooples/AiDotNet PR: 444
File: src/Interfaces/IPruningMask.cs:1-102
Timestamp: 2025-12-18T08:49:25.295Z
Learning: In the AiDotNet repository, the project-level global using includes AiDotNet.Tensors.LinearAlgebra via AiDotNet.csproj. Therefore, Vector<T>, Matrix<T>, and Tensor<T> are available without per-file using directives. Do not flag missing using directives for these types in any C# files within this project. Apply this guideline broadly to all C# files (not just a single file) to avoid false positives. If a file uses a type from a different namespace not covered by the global using, flag as usual.
Applied to files:
tests/AiDotNet.Tests/Onnx/OnnxModelTests.cstests/AiDotNet.Tests/Audio/Features/MfccExtractorTests.cssrc/Onnx/OnnxExecutionProvider.cssrc/Audio/TextToSpeech/TtsOptions.cssrc/Interfaces/IOnnxModelDownloader.cstests/AiDotNet.Tests/Audio/Features/SpectralFeatureExtractorTests.cssrc/Audio/MusicAnalysis/BeatTracker.cstests/AiDotNet.Tests/Audio/Fingerprinting/FingerprintingTests.cssrc/Interfaces/IAudioFeatureExtractor.cstests/AiDotNet.Tests/Audio/Features/ChromaExtractorTests.cssrc/Audio/Whisper/WhisperTokenizer.cssrc/Audio/Features/ChromaExtractor.cssrc/Interfaces/IOnnxModel.cssrc/Audio/AudioGen/AudioGenModel.cssrc/Audio/AudioGen/AudioGenOptions.cssrc/Audio/Features/AudioFeatureExtractorBase.cssrc/Audio/Classification/SceneClassifier.cssrc/Audio/Features/ConstantQTransform.cssrc/Audio/Features/MfccExtractor.cssrc/Audio/Speaker/SpeakerVerifier.cssrc/Onnx/OnnxTensorConverter.cssrc/Audio/MusicAnalysis/KeyDetector.cssrc/Audio/Classification/AudioEventDetector.cssrc/Audio/Features/SpectralFeatureExtractor.cssrc/Audio/Classification/GenreClassifier.cssrc/Onnx/OnnxModelMetadata.cssrc/Audio/TextToSpeech/TtsPreprocessor.cstests/AiDotNet.Tests/Audio/MusicAnalysis/MusicAnalysisTests.cssrc/Audio/TextToSpeech/TtsModel.cssrc/Audio/Speaker/SpeakerDiarizer.cssrc/Interfaces/IAudioFingerprinter.cssrc/Audio/Speaker/SpeakerEmbeddingExtractor.cssrc/Audio/Whisper/WhisperModel.cstests/AiDotNet.Tests/Audio/Speaker/SpeakerTests.cssrc/Onnx/OnnxModelDownloader.cssrc/Audio/MusicAnalysis/ChordRecognizer.cssrc/Audio/Fingerprinting/SpectrogramFingerprinter.cssrc/Audio/SourceSeparation/MusicSourceSeparator.cssrc/Onnx/OnnxModel.cssrc/Onnx/OnnxExporter.cssrc/Audio/Localization/SoundLocalizer.cssrc/Audio/Fingerprinting/ChromaprintFingerprinter.cssrc/Onnx/OnnxModelOptions.cssrc/Audio/Whisper/WhisperOptions.cs
📚 Learning: 2025-12-18T08:49:53.103Z
Learnt from: ooples
Repo: ooples/AiDotNet PR: 444
File: src/Interfaces/IPruningStrategy.cs:1-4
Timestamp: 2025-12-18T08:49:53.103Z
Learning: In this repository, global using directives are declared in AiDotNet.csproj for core namespaces (AiDotNet.Tensors.* and AiDotNet.*) and common system types. When reviewing C# files, assume these global usings are in effect; avoid adding duplicate using statements for these namespaces and for types like Vector<T>, Matrix<T>, Tensor<T>, etc. If a type is not found, verify the global usings or consider adding a file-scoped using if needed. Prefer relying on global usings to reduce boilerplate.
Applied to files:
tests/AiDotNet.Tests/Onnx/OnnxModelTests.cstests/AiDotNet.Tests/Audio/Features/MfccExtractorTests.cssrc/Onnx/OnnxExecutionProvider.cssrc/Audio/TextToSpeech/TtsOptions.cssrc/Interfaces/IOnnxModelDownloader.cstests/AiDotNet.Tests/Audio/Features/SpectralFeatureExtractorTests.cssrc/Audio/MusicAnalysis/BeatTracker.cstests/AiDotNet.Tests/Audio/Fingerprinting/FingerprintingTests.cssrc/Interfaces/IAudioFeatureExtractor.cstests/AiDotNet.Tests/Audio/Features/ChromaExtractorTests.cssrc/Audio/Whisper/WhisperTokenizer.cssrc/Audio/Features/ChromaExtractor.cssrc/Interfaces/IOnnxModel.cssrc/Audio/AudioGen/AudioGenModel.cssrc/Audio/AudioGen/AudioGenOptions.cssrc/Audio/Features/AudioFeatureExtractorBase.cssrc/Audio/Classification/SceneClassifier.cssrc/Audio/Features/ConstantQTransform.cssrc/Audio/Features/MfccExtractor.cssrc/Audio/Speaker/SpeakerVerifier.cssrc/Onnx/OnnxTensorConverter.cssrc/Audio/MusicAnalysis/KeyDetector.cssrc/Audio/Classification/AudioEventDetector.cssrc/Audio/Features/SpectralFeatureExtractor.cssrc/Audio/Classification/GenreClassifier.cssrc/Onnx/OnnxModelMetadata.cssrc/Audio/TextToSpeech/TtsPreprocessor.cstests/AiDotNet.Tests/Audio/MusicAnalysis/MusicAnalysisTests.cssrc/Audio/TextToSpeech/TtsModel.cssrc/Audio/Speaker/SpeakerDiarizer.cssrc/Interfaces/IAudioFingerprinter.cssrc/Audio/Speaker/SpeakerEmbeddingExtractor.cssrc/Audio/Whisper/WhisperModel.cstests/AiDotNet.Tests/Audio/Speaker/SpeakerTests.cssrc/Onnx/OnnxModelDownloader.cssrc/Audio/MusicAnalysis/ChordRecognizer.cssrc/Audio/Fingerprinting/SpectrogramFingerprinter.cssrc/Audio/SourceSeparation/MusicSourceSeparator.cssrc/Onnx/OnnxModel.cssrc/Onnx/OnnxExporter.cssrc/Audio/Localization/SoundLocalizer.cssrc/Audio/Fingerprinting/ChromaprintFingerprinter.cssrc/Onnx/OnnxModelOptions.cssrc/Audio/Whisper/WhisperOptions.cs
📚 Learning: 2025-12-19T19:05:13.598Z
Learnt from: ooples
Repo: ooples/AiDotNet PR: 445
File: src/Interfaces/IPredictionModelBuilder.cs:7-8
Timestamp: 2025-12-19T19:05:13.598Z
Learning: In ooples/AiDotNet, IAggregationStrategy<TModel> is declared in AiDotNet.Interfaces (src/Interfaces/IAggregationStrategy.cs). For src/Interfaces/IPredictionModelBuilder.cs (also in AiDotNet.Interfaces), referencing IAggregationStrategy does not require an additional using. Avoid flagging missing using directives for this case in future reviews.
Applied to files:
src/Interfaces/IOnnxModelDownloader.cssrc/Interfaces/IOnnxModel.cssrc/Onnx/OnnxModel.cs
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: CodeQL analysis (csharp)
- GitHub Check: CodeQL Analysis
- GitHub Check: Build (Windows)
There was a problem hiding this comment.
Pull request overview
This PR implements a comprehensive Audio AI stack covering ONNX Runtime integration, speech-to-text (Whisper), text-to-speech (FastSpeech2 + HiFi-GAN), audio generation (AudioGen), and extensive audio analysis capabilities including speaker recognition, music analysis, and audio fingerprinting.
Key Changes:
- ONNX Runtime infrastructure with multi-provider support (CPU, CUDA, TensorRT, DirectML)
- Whisper ASR model with encoder-decoder architecture and tokenization
- TTS pipeline with acoustic model and vocoder components
- Comprehensive audio analysis: MFCCs, chroma features, spectral features, beat tracking, chord recognition, key detection
- Speaker recognition: embedding extraction, verification, and diarization
- Audio fingerprinting with Chromaprint and spectrogram-based approaches
- Music source separation using HPSS and neural network-based methods
Reviewed changes
Copilot reviewed 44 out of 44 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| tests/AiDotNet.Tests/Onnx/OnnxModelTests.cs | Unit tests for ONNX model infrastructure |
| tests/AiDotNet.Tests/Audio/Speaker/SpeakerTests.cs | Tests for speaker embedding, verification, and diarization |
| tests/AiDotNet.Tests/Audio/MusicAnalysis/MusicAnalysisTests.cs | Tests for beat tracking, chord recognition, and key detection |
| tests/AiDotNet.Tests/Audio/Fingerprinting/FingerprintingTests.cs | Tests for audio fingerprinting algorithms |
| tests/AiDotNet.Tests/Audio/Features/*.cs | Tests for MFCC, chroma, and spectral feature extraction |
| src/Onnx/*.cs | ONNX Runtime integration with model loading, tensor conversion, and model downloading |
| src/Interfaces/*.cs | Interface definitions for ONNX models, audio feature extraction, and fingerprinting |
| src/Audio/Whisper/*.cs | Whisper speech recognition implementation with tokenization |
| src/Audio/TextToSpeech/*.cs | TTS model with text preprocessing and synthesis pipeline |
| src/Audio/Speaker/*.cs | Speaker recognition components including embeddings, verification, and diarization |
| src/Audio/SourceSeparation/*.cs | Music source separation with HPSS and neural network approaches |
| src/Audio/MusicAnalysis/*.cs | Chord recognition and key detection using chromagram analysis |
| src/Audio/AudioGen/*.cs | Audio generation configuration and options |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
CodeQL found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.
Add comprehensive interfaces following IFullModel pattern: - ISpeechRecognizer for ASR/transcription (Whisper-style) - ITextToSpeech for TTS synthesis - IAudioGenerator for audio/music generation Speaker recognition: - ISpeakerEmbeddingExtractor for d-vector extraction - ISpeakerVerifier for speaker verification - ISpeakerDiarizer for speaker diarization Music analysis: - IBeatTracker for tempo/beat detection - IChordRecognizer for chord recognition - IKeyDetector for musical key detection - IMusicSourceSeparator for source separation Classification: - IGenreClassifier for genre classification - IAudioEventDetector for event detection - ISceneClassifier for scene classification - ISoundLocalizer for sound localization 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Create 5 intermediate base classes for audio processing: - AudioNeuralNetworkBase<T>: Core audio NN base extending NeuralNetworkBase<T> with ONNX support, mel spectrogram utilities, and dual-mode inference - SpeakerRecognitionBase<T>: Speaker recognition base with cosine similarity, embedding normalization, and MFCC extractor utilities - MusicAnalysisBase<T>: Music analysis base with onset detection, tempogram, and chromagram extraction for beat/chord/key analysis - AudioClassifierBase<T>: Classification base with softmax, top-k predictions, and class weight computation for genre/event/scene classification - AudioFingerprinterBase<T>: Fingerprinting base implementing IAudioFingerprinter<T> with Hamming distance and alignment utilities 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Refactor WhisperModel to extend AudioNeuralNetworkBase and implement ISpeechRecognizer - Refactor TtsModel to extend AudioNeuralNetworkBase and implement ITextToSpeech - Refactor AudioGenModel to extend AudioNeuralNetworkBase and implement IAudioGenerator - Add proper NeuralNetworkArchitecture configuration for ONNX-only models - Implement required NeuralNetworkBase abstract methods (Predict, Train, UpdateParameters, etc.) - Add correct ModelMetadata implementation using proper property names - Use appropriate task types (SpeechRecognition, Generative) and complexity (VeryDeep, Deep) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Refactor ChromaprintFingerprinter to extend AudioFingerprinterBase<T> - Refactor SpectrogramFingerprinter to extend AudioFingerprinterBase<T> - Add override keyword for abstract/virtual methods from base class - Use NumOps from base class instead of private field - Set SampleRate and FingerprintLength through base class properties 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- BeatTracker now extends MusicAnalysisBase<T> - ChordRecognizer now extends MusicAnalysisBase<T> - KeyDetector now extends MusicAnalysisBase<T> - Use NumOps from base class instead of private field - Set base class properties (SampleRate, HopLength, FftSize) in constructors 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Use protected NumOps field instead of private _numOps for extensibility - Add useful public properties (SampleRate, thresholds, etc.) - Add ExtractTensor and ComputeSimilarity helper methods to extractor - Maintain lightweight utility class design (no heavy interface inheritance) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 9
♻️ Duplicate comments (4)
src/Audio/Speaker/SpeakerVerifier.cs (1)
40-73: Thread safety concern already flagged.The
_enrolledSpeakersdictionary is not thread-safe for concurrent enrollment/verification operations.src/Audio/AudioGen/AudioGenModel.cs (2)
205-224: Resource leak if model loading fails - already flagged.
FromFilesdoesn't clean up already-loaded models if a subsequent model load fails.
476-508: Tokenization bug for digits - already flagged.The tokenization computes
c - 'a' + 10for both letters and digits, producing negative values for digits.src/Audio/Whisper/WhisperModel.cs (1)
207-227: Resource leak if decoder loading fails - already flagged.
FromFilesdoesn't dispose the encoder if the decoder constructor throws.
🧹 Nitpick comments (26)
src/Interfaces/IMusicSourceSeparator.cs (3)
137-143: Consider documenting whydoubleis used for volume multipliers.The
Remixmethod usesdoublefor volume multipliers while the rest of the interface is generic overT. This is likely intentional (volume is always floating-point regardless of audio sample type), but adding a brief note in the documentation would clarify this design choice.
150-184: Consider making result class immutable or usingrequired/initmodifiers.
SourceSeparationResult<T>is a result container but has mutable public setters. For result objects, immutability is typically preferred to prevent accidental modification after creation.Additionally,
OriginalMixusesdefault!which suppresses the null warning but doesn't prevent runtime null issues if not properly initialized.🔎 Suggested approach using init-only properties
public class SourceSeparationResult<T> { - public IReadOnlyDictionary<string, Tensor<T>> Sources { get; set; } = new Dictionary<string, Tensor<T>>(); - public Tensor<T> OriginalMix { get; set; } = default!; - public int SampleRate { get; set; } - public double Duration { get; set; } - public SeparationQuality<T>? Quality { get; set; } + public required IReadOnlyDictionary<string, Tensor<T>> Sources { get; init; } + public required Tensor<T> OriginalMix { get; init; } + public required int SampleRate { get; init; } + public required double Duration { get; init; } + public SeparationQuality<T>? Quality { get; init; }
190-206: Same immutability suggestion applies toSeparationQuality<T>.For consistency with the recommended refactor on
SourceSeparationResult<T>, consider usinginitsetters here as well. Quality metrics are typically computed once and shouldn't change afterward.🔎 Suggested approach
public class SeparationQuality<T> { - public IReadOnlyDictionary<string, T> SDR { get; set; } = new Dictionary<string, T>(); - public IReadOnlyDictionary<string, T> SIR { get; set; } = new Dictionary<string, T>(); - public IReadOnlyDictionary<string, T> SAR { get; set; } = new Dictionary<string, T>(); + public IReadOnlyDictionary<string, T> SDR { get; init; } = new Dictionary<string, T>(); + public IReadOnlyDictionary<string, T> SIR { get; init; } = new Dictionary<string, T>(); + public IReadOnlyDictionary<string, T> SAR { get; init; } = new Dictionary<string, T>(); }src/Interfaces/ISoundLocalizer.cs (3)
37-37: Consider adding a generic type constraint for numeric operations.The interface and all related types use
Twithout any constraint, but this is a numeric computation library. Properties likeDistance(line 189) useT?, which behaves differently for value types vs reference types. For a sound localization API dealing with angles and distances, constrainingTensures type safety.Suggested constraint
-public interface ISoundLocalizer<T> +public interface ISoundLocalizer<T> where T : struct, INumber<T>Apply similar constraints to the supporting classes (
LocalizationResult<T>,SoundSource<T>, etc.).
147-168: Multiple types defined in a single interface file.The PR objectives specify "one class per file and matching namespaces" as an architectural requirement. This file contains 1 interface, 8 classes, and 1 enum. Consider splitting into separate files:
ISoundLocalizer.cs(interface only)LocalizationResult.cs,SoundSource.cs,DirectionEstimate.cs, etc.- Alternatively, group all DTOs into a
SoundLocalizationsubfolder underModels/.This is flagged based on the stated architectural requirements in the PR objectives.
Also applies to: 174-205, 211-227, 233-249, 255-276, 282-303, 309-330, 351-362
147-168: Result classes are mutable; consider using init-only setters.
LocalizationResult<T>andSoundTrackingResult<T>are result objects returned from methods. Using public setters allows consumers to mutate results after creation, which can lead to unexpected behavior. Usinginitsetters or making these records would improve safety.Example with init setters
public class LocalizationResult<T> { - public IReadOnlyList<SoundSource<T>> Sources { get; set; } = Array.Empty<SoundSource<T>>(); + public IReadOnlyList<SoundSource<T>> Sources { get; init; } = Array.Empty<SoundSource<T>>(); public int NumSources => Sources.Count; - public SoundSource<T>? DominantSource { get; set; } + public SoundSource<T>? DominantSource { get; init; } - public double Duration { get; set; } + public double Duration { get; init; } }Also applies to: 233-249
src/Interfaces/IChordRecognizer.cs (2)
178-182: Consider documenting theEndTime >= StartTimeinvariant.The
Durationproperty assumesEndTime >= StartTime. While the recognizer implementation should enforce this, adding a brief XML doc note about this assumption would help consumers understand the expected contract.🔎 Optional documentation enhancement
/// <summary> /// Gets the duration of this segment. /// </summary> + /// <remarks> + /// Assumes EndTime >= StartTime. Negative values indicate invalid segment data. + /// </remarks> public double Duration => EndTime - StartTime;
114-209: Consider splitting result types into separate files for consistency.The PR objectives mention "one class per file" as an architectural requirement. This file contains the interface plus three result/DTO classes. While co-locating tightly coupled result types with their interface is a common pattern, you may want to split these for consistency with the project's conventions.
This is a low-priority suggestion given that the current organization is pragmatic and the types are cohesive.
src/Interfaces/IBeatTracker.cs (1)
220-243: Consider adding validation for time signature values.The constructor accepts any
intvalues, but a denominator of 0 or negative values would be musically invalid and could cause division-by-zero issues downstream.🔎 Proposed validation
public TimeSignature(int numerator, int denominator) { + if (numerator <= 0) + throw new ArgumentOutOfRangeException(nameof(numerator), "Numerator must be positive."); + if (denominator <= 0) + throw new ArgumentOutOfRangeException(nameof(denominator), "Denominator must be positive."); Numerator = numerator; Denominator = denominator; }src/Audio/MusicAnalysis/KeyDetector.cs (1)
107-115: Vector-to-Tensor conversion could be optimized.The element-by-element copy pattern appears in multiple files. Consider adding a utility method or constructor to
Tensor<T>that accepts aVector<T>.🔎 Inline simplification (if Tensor supports span/array constructor)
public KeyDetectionResult Detect(Vector<T> audio) { - var tensor = new Tensor<T>([audio.Length]); - for (int i = 0; i < audio.Length; i++) - { - tensor[i] = audio[i]; - } + // If Tensor<T> supports this pattern: + var tensor = Tensor<T>.FromVector(audio); return Detect(tensor); }src/Audio/MusicAnalysis/ChordRecognizer.cs (1)
143-165: Redundant normalization in template creation.The template is already normalized when weights are assigned as
1.0 / intervals.Length(since all non-zero values sum to 1.0). The second normalization (lines 155-162) is unnecessary.🔎 Proposed simplification
private static double[] CreateTemplate(int root, int[] intervals) { var template = new double[12]; double weight = 1.0 / intervals.Length; foreach (int interval in intervals) { int bin = (root + interval) % 12; template[bin] = weight; } - // Normalize - double sum = template.Sum(); - if (sum > 0) - { - for (int i = 0; i < 12; i++) - { - template[i] /= sum; - } - } - return template; }src/Audio/MusicAnalysis/BeatTracker.cs (1)
120-125: Magic index for spectral flux is fragile.The comment explains index 4 is spectral flux, but this relies on a specific feature ordering in
SpectralFeatureExtractor. If the extractor changes, this code will silently break.🔎 Consider using a named constant or feature lookup
+private const int SpectralFluxFeatureIndex = 4; // Corresponds to SpectralFeatureType.Flux ordering + // Get spectral flux (column 4 in our SpectralFeatureExtractor) for (int f = 0; f < numFrames; f++) { - // Use spectral flux (index 4) as onset strength - onsetEnvelope[f] = Math.Max(0, NumOps.ToDouble(features[f, 4])); + onsetEnvelope[f] = Math.Max(0, NumOps.ToDouble(features[f, SpectralFluxFeatureIndex])); }src/Audio/TextToSpeech/TtsModel.cs (2)
246-251: Unusedpitchparameter.The
pitchparameter is accepted but never used in the synthesis logic. Consider removing it or adding a TODO comment if pitch shifting is planned for future implementation.
477-480: Cloned instance will be non-functional.
CreateNewInstancecreates aTtsModelwith null models, soIsReadywill be false. If this is intentional for deep-clone workflows where models are loaded separately, consider adding a clarifying comment.src/Interfaces/IAudioGenerator.cs (1)
215-246: Consider separatingAudioGenerationOptions<T>to its own file.Per project conventions (one class per file),
AudioGenerationOptions<T>could be moved to a separate file. Additionally, the generic parameter<T>appears unused in this class—it has noT-typed members—so it could potentially be a non-genericAudioGenerationOptions.src/Interfaces/ISpeakerVerifier.cs (1)
161-182: Consider usingrequiredmodifier for mandatory properties.
SpeakerVerificationResult<T>.Score,Threshold, andConfidenceare initialized withdefault!which suppresses null warnings but could lead to uninitialized value types behaving unexpectedly. If C# 11+ features are available, consider usingrequiredor ensuring these are always set during construction.src/Audio/AudioNeuralNetworkBase.cs (1)
169-177: No-op forward pass whenLayersis empty.If
Layersis empty (which is likely in ONNX mode),Forwardreturns the input unchanged. This is probably intentional, but consider adding a guard or documentation clarifying this behavior.src/Audio/Speaker/SpeakerEmbeddingExtractor.cs (1)
318-338: Silent handling of mismatched embedding lengths.
CosineSimilarityusesMath.Minwhen vector lengths differ, which is defensive but may mask configuration issues. If embeddings should always match, consider adding validation or at least a debug assertion.src/Interfaces/ITextToSpeech.cs (1)
183-218: Consider usingrecordfor immutable voice metadata.
VoiceInfo<T>is essentially a data transfer object. Using arecordwould provide immutability and value-based equality, which is often preferable for configuration/metadata types. However, this is optional since mutable DTOs work fine here.src/Interfaces/IGenreClassifier.cs (1)
129-230: Consider consistency in numeric type usage.
GenreDistributioninGenreTrackingResult<T>usesdoublewhile other confidence/probability fields useT. This asymmetry may be intentional for distribution percentages, but consider documenting the rationale or usingIReadOnlyDictionary<string, T>for consistency.src/Audio/Classification/GenreClassifier.cs (1)
309-364: Consider extending tempo range for faster genres.The tempo estimation limits autocorrelation search to 30-120 BPM (lines 336-337), which may miss faster genres like metal (140+ BPM). Consider extending the range:
-int minLag = (int)(0.5 * _options.SampleRate / frameSize); // 120 BPM +int minLag = (int)(0.25 * _options.SampleRate / frameSize); // 240 BPMsrc/Audio/Fingerprinting/AudioFingerprinterBase.cs (2)
79-88: Consider extracting the Vector-to-Tensor conversion as a reusable utility.Both derived classes (
SpectrogramFingerprinterandChromaprintFingerprinter) override this method with nearly identical implementations. The conversion logic could be centralized in a helper or the base class could provide a protectedVectorToTensormethod.🔎 Proposed refactor
+ /// <summary> + /// Converts a vector to a 1D tensor. + /// </summary> + protected Tensor<T> VectorToTensor(Vector<T> audio) + { + var tensor = new Tensor<T>([audio.Length]); + for (int i = 0; i < audio.Length; i++) + { + tensor[i] = audio[i]; + } + return tensor; + } + public virtual AudioFingerprint<T> Fingerprint(Vector<T> audio) { - // Convert vector to tensor and delegate to main implementation - var tensor = new Tensor<T>([audio.Length]); - for (int i = 0; i < audio.Length; i++) - { - tensor[i] = audio[i]; - } - return Fingerprint(tensor); + return Fingerprint(VectorToTensor(audio)); }
122-134: Consider usingBitOperations.PopCountfor hardware-accelerated bit counting.The current loop-based implementation is correct, but
System.Numerics.BitOperations.PopCountprovides O(1) hardware-accelerated population count on modern CPUs.🔎 Proposed refactor
+using System.Numerics; + protected int ComputeHammingDistance(uint fp1, uint fp2) { - uint xor = fp1 ^ fp2; - int distance = 0; - - while (xor != 0) - { - distance += (int)(xor & 1); - xor >>= 1; - } - - return distance; + return BitOperations.PopCount(fp1 ^ fp2); }src/Audio/Fingerprinting/ChromaprintFingerprinter.cs (2)
294-315: Approximate matching generates many hash variants—consider documenting performance characteristics.With
MaxBitDifference=2,GetHashVariantsproduces 529 variants per query hash (1 + 32 + 496). For long audio with many hashes, this could impact performance. Consider documenting this trade-off or providing an option to disable approximate matching when exact matching suffices.
411-420:BitCountduplicates the bit-counting logic from the base class.Consider reusing
ComputeHammingDistancewith an identity comparison (i.e.,ComputeHammingDistance(n, 0)won't work directly) or usingSystem.Numerics.BitOperations.PopCount(n)for consistency and hardware acceleration.🔎 Proposed refactor
+using System.Numerics; + -private static int BitCount(uint n) -{ - int count = 0; - while (n != 0) - { - count++; - n &= (n - 1); - } - return count; -} +private static int BitCount(uint n) => BitOperations.PopCount(n);src/Audio/Fingerprinting/SpectrogramFingerprinter.cs (1)
324-329: Consider using areadonly record structforSpectralPeak.Since
SpectralPeakis a simple data container, areadonly record structwould provide value semantics, immutability, and better memory locality when stored in lists.🔎 Proposed refactor
-/// <summary> -/// Represents a spectral peak. -/// </summary> -internal class SpectralPeak -{ - public int Frame { get; set; } - public int Bin { get; set; } - public double Magnitude { get; set; } -} +/// <summary> +/// Represents a spectral peak. +/// </summary> +internal readonly record struct SpectralPeak(int Frame, int Bin, double Magnitude);Update usage to use constructor syntax:
-peaks.Add(new SpectralPeak -{ - Frame = f, - Bin = b, - Magnitude = value -}); +peaks.Add(new SpectralPeak(f, b, value));
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (34)
src/Audio/AudioGen/AudioGenModel.cssrc/Audio/AudioNeuralNetworkBase.cssrc/Audio/Classification/AudioClassifierBase.cssrc/Audio/Classification/AudioEventDetector.cssrc/Audio/Classification/GenreClassifier.cssrc/Audio/Classification/SceneClassifier.cssrc/Audio/Fingerprinting/AudioFingerprinterBase.cssrc/Audio/Fingerprinting/ChromaprintFingerprinter.cssrc/Audio/Fingerprinting/SpectrogramFingerprinter.cssrc/Audio/MusicAnalysis/BeatTracker.cssrc/Audio/MusicAnalysis/ChordRecognizer.cssrc/Audio/MusicAnalysis/KeyDetector.cssrc/Audio/MusicAnalysis/MusicAnalysisBase.cssrc/Audio/Speaker/SpeakerDiarizer.cssrc/Audio/Speaker/SpeakerEmbeddingExtractor.cssrc/Audio/Speaker/SpeakerRecognitionBase.cssrc/Audio/Speaker/SpeakerVerifier.cssrc/Audio/TextToSpeech/TtsModel.cssrc/Audio/Whisper/WhisperModel.cssrc/Interfaces/IAudioEventDetector.cssrc/Interfaces/IAudioGenerator.cssrc/Interfaces/IBeatTracker.cssrc/Interfaces/IChordRecognizer.cssrc/Interfaces/IGenreClassifier.cssrc/Interfaces/IKeyDetector.cssrc/Interfaces/IMusicSourceSeparator.cssrc/Interfaces/ISceneClassifier.cssrc/Interfaces/ISoundLocalizer.cssrc/Interfaces/ISpeakerDiarizer.cssrc/Interfaces/ISpeakerEmbeddingExtractor.cssrc/Interfaces/ISpeakerVerifier.cssrc/Interfaces/ISpeechRecognizer.cssrc/Interfaces/ITextToSpeech.cstests/AiDotNet.Tests/Audio/Classification/ClassificationTests.cs
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-12-18T08:49:25.295Z
Learnt from: ooples
Repo: ooples/AiDotNet PR: 444
File: src/Interfaces/IPruningMask.cs:1-102
Timestamp: 2025-12-18T08:49:25.295Z
Learning: In the AiDotNet repository, the project-level global using includes AiDotNet.Tensors.LinearAlgebra via AiDotNet.csproj. Therefore, Vector<T>, Matrix<T>, and Tensor<T> are available without per-file using directives. Do not flag missing using directives for these types in any C# files within this project. Apply this guideline broadly to all C# files (not just a single file) to avoid false positives. If a file uses a type from a different namespace not covered by the global using, flag as usual.
Applied to files:
tests/AiDotNet.Tests/Audio/Classification/ClassificationTests.cssrc/Audio/Classification/AudioClassifierBase.cssrc/Interfaces/ISpeakerEmbeddingExtractor.cssrc/Interfaces/ISoundLocalizer.cssrc/Audio/Fingerprinting/SpectrogramFingerprinter.cssrc/Audio/Speaker/SpeakerVerifier.cssrc/Audio/MusicAnalysis/ChordRecognizer.cssrc/Audio/Speaker/SpeakerRecognitionBase.cssrc/Audio/Classification/SceneClassifier.cssrc/Interfaces/IBeatTracker.cssrc/Interfaces/IAudioGenerator.cssrc/Audio/AudioNeuralNetworkBase.cssrc/Interfaces/IAudioEventDetector.cssrc/Audio/Speaker/SpeakerDiarizer.cssrc/Audio/MusicAnalysis/BeatTracker.cssrc/Interfaces/IMusicSourceSeparator.cssrc/Interfaces/IGenreClassifier.cssrc/Audio/MusicAnalysis/MusicAnalysisBase.cssrc/Interfaces/IChordRecognizer.cssrc/Audio/MusicAnalysis/KeyDetector.cssrc/Audio/Classification/AudioEventDetector.cssrc/Audio/Speaker/SpeakerEmbeddingExtractor.cssrc/Audio/Fingerprinting/ChromaprintFingerprinter.cssrc/Audio/Fingerprinting/AudioFingerprinterBase.cssrc/Interfaces/ISceneClassifier.cssrc/Audio/Classification/GenreClassifier.cssrc/Audio/TextToSpeech/TtsModel.cssrc/Interfaces/ITextToSpeech.cssrc/Interfaces/ISpeechRecognizer.cssrc/Interfaces/IKeyDetector.cssrc/Interfaces/ISpeakerDiarizer.cssrc/Interfaces/ISpeakerVerifier.cssrc/Audio/Whisper/WhisperModel.cssrc/Audio/AudioGen/AudioGenModel.cs
📚 Learning: 2025-12-18T08:49:53.103Z
Learnt from: ooples
Repo: ooples/AiDotNet PR: 444
File: src/Interfaces/IPruningStrategy.cs:1-4
Timestamp: 2025-12-18T08:49:53.103Z
Learning: In this repository, global using directives are declared in AiDotNet.csproj for core namespaces (AiDotNet.Tensors.* and AiDotNet.*) and common system types. When reviewing C# files, assume these global usings are in effect; avoid adding duplicate using statements for these namespaces and for types like Vector<T>, Matrix<T>, Tensor<T>, etc. If a type is not found, verify the global usings or consider adding a file-scoped using if needed. Prefer relying on global usings to reduce boilerplate.
Applied to files:
tests/AiDotNet.Tests/Audio/Classification/ClassificationTests.cssrc/Audio/Classification/AudioClassifierBase.cssrc/Interfaces/ISpeakerEmbeddingExtractor.cssrc/Interfaces/ISoundLocalizer.cssrc/Audio/Fingerprinting/SpectrogramFingerprinter.cssrc/Audio/Speaker/SpeakerVerifier.cssrc/Audio/MusicAnalysis/ChordRecognizer.cssrc/Audio/Speaker/SpeakerRecognitionBase.cssrc/Audio/Classification/SceneClassifier.cssrc/Interfaces/IBeatTracker.cssrc/Interfaces/IAudioGenerator.cssrc/Audio/AudioNeuralNetworkBase.cssrc/Interfaces/IAudioEventDetector.cssrc/Audio/Speaker/SpeakerDiarizer.cssrc/Audio/MusicAnalysis/BeatTracker.cssrc/Interfaces/IMusicSourceSeparator.cssrc/Interfaces/IGenreClassifier.cssrc/Audio/MusicAnalysis/MusicAnalysisBase.cssrc/Interfaces/IChordRecognizer.cssrc/Audio/MusicAnalysis/KeyDetector.cssrc/Audio/Classification/AudioEventDetector.cssrc/Audio/Speaker/SpeakerEmbeddingExtractor.cssrc/Audio/Fingerprinting/ChromaprintFingerprinter.cssrc/Audio/Fingerprinting/AudioFingerprinterBase.cssrc/Interfaces/ISceneClassifier.cssrc/Audio/Classification/GenreClassifier.cssrc/Audio/TextToSpeech/TtsModel.cssrc/Interfaces/ITextToSpeech.cssrc/Interfaces/ISpeechRecognizer.cssrc/Interfaces/IKeyDetector.cssrc/Interfaces/ISpeakerDiarizer.cssrc/Interfaces/ISpeakerVerifier.cssrc/Audio/Whisper/WhisperModel.cssrc/Audio/AudioGen/AudioGenModel.cs
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Build (Windows)
- GitHub Check: CodeQL Analysis
🔇 Additional comments (118)
src/Interfaces/IMusicSourceSeparator.cs (2)
1-44: Well-documented interface with beginner-friendly explanations.The XML documentation is comprehensive, covering use cases, common separations, and popular models. This aligns well with the project's requirement for beginner-friendly documentation.
46-69: Interface properties are well-designed.Read-only properties with appropriate types. Using
IReadOnlyList<string>forSupportedSourcescorrectly signals immutability to consumers.src/Interfaces/ISoundLocalizer.cs (3)
1-41: Well-documented interface with beginner-friendly explanations.The XML documentation is comprehensive, covering both technical details (ITD, ILD, azimuth/elevation) and practical use cases. This aligns well with the project's requirement for "beginner-friendly defaults with validation and documentation."
69-141: Interface method signatures are well-designed.The API surface covers essential sound localization operations: synchronous/async localization, tracking, DOA estimation, spatial spectrum computation, and beamforming. Default parameter values (e.g.,
windowDuration = 0.1,azimuthResolution = 5.0) provide sensible defaults for beginners.
335-345: ArrayType enum is clear and covers common geometries.The enum covers Linear, Circular, Spherical, and Custom array types, which aligns with standard microphone array configurations used in sound localization algorithms (GCC-PHAT, MUSIC, SRP-PHAT mentioned in the AI summary).
src/Interfaces/IChordRecognizer.cs (3)
1-112: Well-designed interface with excellent documentation.The interface is cohesive, properly scoped, and includes both sync and async variants. The XML documentation with beginner-friendly explanations aligns well with the project's architectural requirements for documentation.
114-140: Result class design is acceptable.The mutable DTO pattern with sensible defaults (empty arrays/dictionaries) is practical for result objects, especially given the multi-target framework support (net8.0 and net471).
184-209: Statistics class is well-structured.Clean design with clear property semantics. Consistent with the other DTO classes in this file.
src/Interfaces/IKeyDetector.cs (2)
132-210: LGTM! Data models are well-structured.The
KeyDetectionResult<T>,MusicalMode, andKeyHypothesis<T>types follow project conventions:
- Strict property initialization (string.Empty, default!, Array.Empty)
- Clear XML documentation
- Appropriate use of
MusicalModeenum for type safetyThe structure is intuitive and the defaults are sensible.
212-291: LGTM! Time-tracking models are well-designed.The
KeyTrackingResult<T>,KeySegment<T>, andModulationPoint<T>types properly model temporal aspects of key detection:
- Time properties use
double(appropriate for seconds)- Proper initialization with
Array.Emptyfor collections- Clear semantics for segments and modulation points
- Good documentation explaining modulation detection
The design supports both simple key detection and advanced time-series key tracking.
src/Interfaces/IBeatTracker.cs (2)
1-119: Well-designed interface with comprehensive beat tracking API.The interface provides a complete contract for beat tracking with good separation between tempo estimation, beat detection, and downbeat analysis. The async variant with
CancellationTokensupport is appreciated.
125-156: I was unable to verify the review comment claims due to repository access issues. Manual verification is needed to confirm:
- Whether
BeatTracker<T>class implementsIBeatTracker<T>- Whether a non-generic
BeatTrackingResultclass exists inBeatTracker.cswith different structure than the genericBeatTrackingResult<T>- The actual return types and property types in both the interface and implementation
src/Audio/MusicAnalysis/KeyDetector.cs (4)
37-80: Well-structured key detector with standard Krumhansl-Kessler profiles.The constructor properly initializes the chroma extractor and builds rotated profiles for all 12 keys. The beginner documentation is excellent.
150-174: LGTM!The chroma averaging and normalization logic is correct. Handles the zero-sum edge case appropriately.
229-248: LGTM!Standard Pearson correlation with proper handling of degenerate cases (zero variance).
254-323: LGTM!Well-documented result and options classes with sensible defaults.
src/Audio/MusicAnalysis/ChordRecognizer.cs (4)
34-64: Well-designed chord recognizer with comprehensive chord vocabulary.The constructor properly initializes templates for 9 chord qualities across all 12 roots (108 templates + "N"). Good use of the base class infrastructure.
167-220: LGTM with minor note.The frame classification logic is correct. The "N" template iteration could be skipped (it always scores 0), but this doesn't affect correctness.
241-302: LGTM!Segment merging correctly handles consecutive frames, duration filtering, and confidence averaging. The final segment is properly handled.
305-370: LGTM!Clean data structures with computed
Durationproperty and sensible option defaults.src/Audio/MusicAnalysis/MusicAnalysisBase.cs (5)
28-66: LGTM!Clean base class design with protected NumOps and sensible defaults for audio processing parameters.
124-161: LGTM!Autocorrelation-based tempogram computation with proper generic arithmetic using NumOps.
198-237: LGTM!Peak detection with proper threshold handling and minimum distance enforcement. The replacement logic correctly selects the highest peak within a cluster.
245-261: LGTM!Standard frame-time conversion utilities with optional hop length override.
80-109: Verify spectral flux feature index consistency.This method reads flux from
features[i, 0], butBeatTracker.ComputeOnsetEnvelopeInternalreads fromfeatures[f, 4]. Confirm whether this discrepancy is intentional (different feature configurations) or a bug by examining theSpectralFeatureExtractorimplementation and feature column ordering.src/Audio/MusicAnalysis/BeatTracker.cs (4)
36-70: LGTM!Constructor properly initializes options and spectral extractor. Good documentation for beginners.
146-180: LGTM!Autocorrelation-based tempo estimation correctly finds the best lag and converts to BPM within the configured bounds.
182-265: Well-implemented dynamic programming beat tracker.The forward pass with Gaussian penalty for tempo deviation and backtracking is a solid approach. The lookback window of 2× beat period provides flexibility for tempo variations.
291-356: LGTM!Result and options classes are well-documented with sensible defaults. Note: The type mismatch with
IBeatTracker<T>'sBeatTrackingResult<T>was already flagged in the interface file review.src/Audio/TextToSpeech/TtsModel.cs (1)
589-601: LGTM!The dispose pattern correctly handles
_acousticModellocally while relying on the base class to dispose the vocoder (stored inOnnxModel). The comment clarifies this design.src/Interfaces/ISpeakerEmbeddingExtractor.cs (1)
38-153: LGTM!Well-designed interface with comprehensive XML documentation. The API surface covers the essential embedding operations (extraction, similarity, aggregation, normalization) and follows the established patterns in the codebase.
src/Audio/Speaker/SpeakerRecognitionBase.cs (1)
76-102: LGTM!The cosine similarity implementation correctly handles the edge case where the norm product is zero, preventing division by zero.
src/Audio/Classification/AudioClassifierBase.cs (1)
60-95: LGTM!Numerically stable softmax implementation with max subtraction to prevent overflow. The mapping to class labels is clean and correct.
src/Interfaces/IAudioGenerator.cs (1)
38-209: LGTM!Comprehensive interface design covering text-to-audio, text-to-music, continuation, and inpainting scenarios. The capability flags (
SupportsTextToAudio, etc.) allow implementations to indicate feature availability, with properNotSupportedExceptiondocumentation for unsupported methods.src/Interfaces/ISpeakerVerifier.cs (1)
38-155: LGTM!Well-designed interface covering enrollment, verification, and profile management workflows. The documentation clearly explains FAR/FRR trade-offs which is valuable for users configuring threshold values.
src/Audio/AudioNeuralNetworkBase.cs (2)
188-197: LGTM!Dispose correctly cleans up all three ONNX model holders and delegates to the base class.
32-86: LGTM!Good design establishing audio-specific properties (SampleRate, NumMels) with sensible defaults, and clear ONNX mode detection based on presence of any ONNX model.
src/Audio/Speaker/SpeakerEmbeddingExtractor.cs (2)
39-39: Consider implementingISpeakerEmbeddingExtractor<T>.This class provides speaker embedding functionality but doesn't implement the
ISpeakerEmbeddingExtractor<T>interface defined in this PR. If this is intentional (to keep it simpler thanIFullModel<T, ...>), consider documenting why. Otherwise, implementing the interface would ensure API consistency withISpeakerVerifier.EmbeddingExtractor.
361-397: LGTM!Well-structured options class with sensible defaults for speaker embedding extraction. The nullable
ModelPathenables graceful fallback to MFCC statistics mode.src/Interfaces/ITextToSpeech.cs (2)
1-181: Well-designed TTS interface with comprehensive API surface.The interface provides a cohesive API for text-to-speech with good coverage of advanced features (voice cloning, emotion control, streaming). Documentation is thorough and beginner-friendly.
220-256: LGTM!The
VoiceGenderenum andIStreamingSynthesisSession<T>interface are well-designed. The streaming session properly extendsIDisposablefor resource cleanup.src/Audio/Speaker/SpeakerVerifier.cs (3)
75-108: LGTM!Enrollment methods have proper validation and correctly recompute the centroid when embeddings change.
120-186: LGTM!Verification and identification logic is correct. The
Verifymethod gracefully handles unenrolled speakers with an informative error result rather than throwing.
252-350: LGTM!Supporting data types are well-structured with appropriate defaults. The threshold values (0.7 for verification, 0.6 for identification) are reasonable for cosine similarity-based speaker recognition.
src/Audio/Speaker/SpeakerDiarizer.cs (6)
39-79: LGTM!Constructor properly initializes the embedding extractor with options. The class correctly implements
IDisposableand disposes the extractor.
81-123: LGTM!The diarization pipeline is well-structured: segment audio → extract embeddings → cluster → create speaker turns. The
Vector<T>overload correctly converts toTensor<T>.
156-268: LGTM!The agglomerative clustering implementation using average linkage is correct. The algorithm properly merges clusters above the threshold and relabels to consecutive integers.
345-395: LGTM!Turn creation logic correctly merges consecutive same-speaker segments and filters by minimum duration. The final turn is properly handled after the loop.
397-426: LGTM!Disposal pattern is correctly implemented with proper cleanup of the embedding extractor.
428-542: LGTM!Supporting data types are well-designed.
SpeakingTimePerSpeakeras a computed property is a nice touch for convenience. Default options are reasonable for typical diarization scenarios.src/Interfaces/ISpeechRecognizer.cs (2)
1-150: Well-designed speech recognition interface.The interface provides a comprehensive API for ASR with language detection, streaming support, and word-level timestamps. Documentation is thorough and beginner-friendly.
152-234: LGTM!Data types are well-structured. The streaming session interface properly extends
IDisposableand provides clear semantics for incremental transcription.src/Audio/AudioGen/AudioGenModel.cs (5)
43-112: LGTM!Class structure is well-organized with clear property definitions and proper initialization. The private constructor ensures models are created through factory methods.
126-203: LGTM!
CreateAsyncproperly handles resource cleanup on failure. The try-catch block ensures all loaded models are disposed if any subsequent load fails.
226-332: LGTM!Audio generation pipeline is well-structured. The classifier-free guidance implementation follows the standard formula. Unsupported features appropriately throw
NotSupportedException.
661-750: LGTM!Sampling implementation with temperature, top-k, and top-p filtering is correct. Softmax handles numerical stability by subtracting the max value.
794-815: LGTM!Disposal correctly cleans up text encoder and language model. The audio decoder is stored as
OnnxModelin the base class and disposed there.src/Audio/Whisper/WhisperModel.cs (5)
48-112: LGTM!Class structure is well-organized with proper initialization of mel spectrogram preprocessor and tokenizer. Whisper-specific parameters (25ms window, 10ms hop) are correctly configured.
144-205: LGTM!
CreateAsyncproperly handles resource cleanup on failure with try-catch ensuring both encoder and decoder are disposed if an exception occurs.
592-672: LGTM!Token decoding with greedy strategy is correctly implemented. The autoregressive loop properly handles the end-of-text token to terminate generation.
283-377: LGTM!Language detection correctly extracts per-language probabilities from decoder logits. The softmax implementation is numerically stable with proper handling of the generic type.
697-716: LGTM!Disposal pattern is correctly implemented, deferring encoder/decoder cleanup to the base class.
tests/AiDotNet.Tests/Audio/Classification/ClassificationTests.cs (6)
1-10: LGTM!Imports and test class setup look correct. The namespace follows project conventions.
11-96: LGTM!Well-structured synthetic audio generators with deterministic seeding for reproducible tests. The signal generation covers diverse audio characteristics (harmonic content, rhythm patterns, formants) suitable for classifier testing.
100-168: LGTM!Comprehensive test coverage for GenreClassifier including result validation, probability distribution checks, ordering verification, and feature extraction. The tolerance of 0.01 for probability sum is appropriate.
189-272: LGTM!AudioEventDetector tests appropriately validate event detection, timestamp relationships, confidence bounds, and top-K result limits. The assertion
topK.Count <= 5correctly handles the case where fewer events than K are detected.
276-374: LGTM!SceneClassifier tests follow the same thorough patterns as GenreClassifier tests, with proper validation of classification results, probability distributions, category mapping, and feature extraction.
378-445: LGTM!Disposal tests correctly verify that Dispose() doesn't throw and that subsequent method calls on disposed instances throw
ObjectDisposedException. This ensures proper resource cleanup semantics.src/Audio/Classification/SceneClassifier.cs (9)
1-54: LGTM!Well-structured class with proper field declarations, beginner-friendly XML documentation, and protected
NumOpsfor derived classes. The use ofINumericOperations<T>follows project conventions.
54-98: LGTM!Comprehensive scene taxonomy with sensible category groupings (indoor, outdoor_urban, outdoor_nature, transportation). The
GetCategoryfallback to "unknown" handles edge cases safely.
109-158: LGTM!Constructor properly initializes feature extractors with configurable options and conditionally loads the ONNX model. The
CreateAsyncfactory provides a convenient async initialization path with model download support.
165-221: LGTM!The
Classifymethod correctly handles both model-based and rule-based inference paths, with proper disposal checking and result construction. The manual loop for finding the best probability is appropriate for clarity.
316-409: LGTM!Spectral feature computations are mathematically sound with proper zero-division guards. The spectral flatness uses an appropriate epsilon floor (1e-10) to avoid log(0).
411-482: LGTM!Temporal and band energy feature extraction methods are correctly implemented with proper array bounds handling.
484-528: Verify feature normalization matches expected model input.The hardcoded scaling factors (centroid/100, bandwidth/100, rms10, zcr100) must match the normalization used during model training. Document or make these configurable if the model expects different scaling.
530-653: LGTM!Rule-based classification provides a reasonable fallback when no ONNX model is loaded. The Gaussian scoring function and disposal pattern are correctly implemented.
656-729: LGTM!Options class has sensible audio processing defaults. The use of
requiredproperties for mandatory data inSceneFeaturesandSceneClassificationResultensures proper initialization.src/Interfaces/IGenreClassifier.cs (2)
1-42: LGTM!Excellent beginner-friendly documentation explaining genre classification concepts, use cases, and challenges. The interface properly extends
IFullModel<T, Tensor<T>, Tensor<T>>.
43-127: LGTM!Well-designed interface with comprehensive capabilities: sample rate specification, multi-label support, ONNX mode detection, synchronous/async classification, probability retrieval, top-K predictions, time-tracking, and feature extraction.
src/Audio/Classification/AudioEventDetector.cs (8)
1-54: LGTM!Proper class structure with protected
NumOpsfield and appropriate field declarations. XML documentation is comprehensive and beginner-friendly.
56-111: LGTM!
CommonEventLabelsprovides a well-organized taxonomy of audio events covering human sounds, animals, music, environmental, and household categories. Constructor properly initializes extractors with configurable options.
141-187: LGTM!Windowed detection logic correctly handles overlap and merges events by label. The approach of processing sliding windows and filtering by threshold follows standard audio event detection patterns.
244-272: LGTM!Window splitting correctly handles the edge case where the audio is shorter than a single window, ensuring at least one window is processed.
274-322: LGTM!Classification methods correctly handle both ONNX-based inference (with sigmoid for multi-label) and rule-based fallback. Input tensor reshaping for the model is appropriate.
408-488: LGTM!Event scoring heuristics provide reasonable signal-based detection for common event types. The baseline score of 0.05 ensures no event is completely ruled out in rule-based mode.
490-554: LGTM!Event merging algorithm correctly groups by label, handles overlapping events with sensible tolerance (0.1s), and takes maximum confidence during merge. Disposal pattern is properly implemented.
557-620: LGTM!Options class has appropriate defaults for audio event detection (16kHz sample rate, 1s window). The
AudioEventclass usesrequiredforLabeland provides a usefulToStringoverride.src/Interfaces/IAudioEventDetector.cs (3)
1-44: LGTM!Thorough documentation distinguishing event detection from classification, with practical use cases and challenge descriptions. The interface properly extends the base model interface.
45-147: LGTM!Comprehensive interface design with threshold overloads, async support, type-specific detection, frame-level probabilities, and streaming session support. The dual
Detectoverloads (default vs custom threshold) provide good flexibility.
149-277: LGTM!Well-designed data model supporting batch results with statistics, individual events with peak time tracking, and a streaming session interface with event-driven notifications. The
GetEventsByTypeconvenience method is helpful.src/Interfaces/ISceneClassifier.cs (4)
1-41: LGTM!Excellent documentation clearly differentiating scene classification from event detection, with practical examples and use cases including accessibility applications.
42-120: LGTM!Interface provides comprehensive scene classification capabilities including minimum duration requirements, scene tracking, and acoustic feature extraction. The
TrackSceneChangesmethod is particularly useful for analyzing recordings with location changes.
122-216: LGTM!Rich result types with scene categories, acoustic characteristics (reverberation, crowd density, traffic/nature indicators), and sorted predictions. The
AcousticCharacteristics<T>type provides valuable environmental context.
218-302: LGTM!Scene tracking types properly model temporal scene information with segments, transitions, and distribution statistics. The
SceneTransition<T>type captures both the timing and confidence of detected scene changes.src/Audio/Classification/GenreClassifier.cs (6)
1-97: LGTM!Well-structured implementation following the same patterns as SceneClassifier. The constructor properly initializes MFCC and spectral feature extractors with configurable options.
99-197: LGTM!
CreateAsyncfactory andClassify/ClassifyAsync/ClassifyBatchmethods follow consistent patterns with other classifiers. The batch classification correctly leverages the single-item classifier.
199-259: LGTM!Feature extraction correctly computes MFCC statistics and extracts spectral features. The fallback to
NumOps.Zerofor missing rolloff data is appropriate.
366-442: LGTM!Model-based classification correctly flattens features with normalization and applies numerically stable softmax. The rule-based fallback provides reasonable heuristic classification when no model is available.
444-556: LGTM!Genre-specific scoring heuristics are well-tuned for typical characteristics of each genre (tempo, spectral content, energy). The Gaussian scoring function and disposal pattern are correctly implemented.
559-638: LGTM!Options and result classes follow the same patterns as other classifiers with sensible defaults and proper use of
requiredproperties for mandatory data.src/Audio/Fingerprinting/AudioFingerprinterBase.cs (4)
1-65: Well-structured base class with clean abstractions.The class establishes a solid foundation for fingerprinting implementations with appropriate use of
NumOpsfromMathHelper, consistent property initialization patterns, and clear separation of abstract/virtual members. Documentation is thorough and beginner-friendly.
193-214: Alignment overlap logic is correct.The offset-based overlap calculation properly handles both positive and negative offsets, ensuring array accesses remain within bounds.
222-243: Hash computation is clean and correct.Proper binarization with threshold comparison and 32-bit limitation per hash value.
250-263: Frame/time conversion utilities are straightforward and correct.src/Audio/Fingerprinting/ChromaprintFingerprinter.cs (7)
39-53: Clean constructor with proper delegation to base class and options initialization.
58-77: Dual fingerprint paths are intentional for type-specific chroma extraction.The overridden
Fingerprint(Vector<T>)method uses a separate code path (ComputeFingerprintFromMatrix) to work withMatrix<T>output from the chroma extractor, which is appropriate ifChromaExtractorreturns different types for different inputs.
162-188: Gray code computation correctly implements differential encoding with proper bit limiting.The dual-comparison approach (horizontal and vertical differences) and the 32-bit limit are appropriate for the Chromaprint algorithm.
204-240: Hash similarity with cross-correlation is correctly implemented with proper bounds checking.
348-372: Hash variant generation is correct and clearly implemented.
374-409: Segment detection logic is sound with reasonable gap tolerance.The hardcoded gap threshold of 3 at line 387 works well for typical use cases. Consider making it configurable via
ChromaprintOptionsif users need to tune matching sensitivity.
426-457: Configuration options are well-documented with sensible defaults.src/Audio/Fingerprinting/SpectrogramFingerprinter.cs (7)
39-50: Constructor properly initializes STFT with configurable parameters.
55-81: Fingerprinting pipeline follows the Shazam methodology correctly.The STFT → magnitude → peaks → hash pipeline is well-structured.
83-102: Magnitude computation is correct and efficiently converts to double for peak detection.
219-227: Hash encoding is correctly implemented with proper bit packing.The 10+10+12 bit layout for (freq1, freq2, timeDelta) is clearly documented and clamped appropriately.
232-248: Jaccard similarity is appropriate for peak-pair hash sets.Unlike the bit-level similarity in
ChromaprintFingerprinter, the set-based Jaccard similarity here correctly reflects the Shazam-style matching where each hash represents a unique peak pair.
253-318: Match detection with offset histogram is well-implemented.The algorithm correctly builds position indices, aggregates offsets, and calculates match regions with proper time boundary handling.
334-380: Configuration options provide sensible defaults for Shazam-style fingerprinting.The target zone range (1-64 frames) and peak detection parameters are appropriate for the algorithm. The
PeaksPerSecondnaming issue was noted above.
Extract all nested classes, enums, and result types from multi-class files into their own individual files following C# best practices (one class per file): - Whisper: WhisperModelSize, WhisperResult, WhisperWord, WhisperSegment - Speaker: SpeakerEmbedding, SpeakerEmbeddingOptions, SpeakerTurn, DiarizationResult, SpeakerDiarizerOptions, VerificationResult, IdentificationResult, SpeakerMatch, SpeakerVerifierOptions - Classification: GenreClassifierOptions, GenreFeatures, GenreClassificationResult, AudioEventDetectorOptions, AudioEvent, SceneClassifierOptions, SceneFeatures, SceneClassificationResult - MusicAnalysis: BeatTrackingResult, BeatTrackerOptions, ChordSegment, ChordRecognizerOptions, KeyDetectionResult, KeyMode, KeyDetectorOptions - Fingerprinting: ChromaprintOptions, SpectrogramFingerprintOptions - Features: ChromaOptions, MfccOptions, SpectralFeatureType, SpectralFeatureOptions, WindowType - TextToSpeech: TtsResult, TtsModelType, VocoderType - AudioGen: AudioGenModelSize, AudioGenResult - SourceSeparation: SourceSeparationOptions, SeparationResult - Localization: SoundLocalizerOptions, LocalizationAlgorithm, LocalizationResult 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Current Status UpdateCompleted Work
Critical Issue IdentifiedThe audio AI models are ONNX-only and cannot be trained from scratch. Current protected override void InitializeLayers()
{
// ONNX-only model - no native layers to initialize // <- WRONG!
}
public override void Train(...)
{
throw new NotImplementedException("Native training ... not yet implemented."); // <- WRONG!
}This defeats the purpose of integrating with AiDotNet. Users should be able to:
Remaining Work (Phase 3)The audio models need trainable architectures:
|
…audio models - Create LanguageModelBackbone enum for type-safe model selection - Add LanguageModelTokenizerFactory for backbone-appropriate tokenizers - Update Blip2NeuralNetwork, FlamingoNeuralNetwork, LLaVANeuralNetwork: - ONNX mode: Require tokenizer (throw with helpful message) - Native mode: Use factory to create appropriate default tokenizer - Update IBlip2Model, IFlamingoModel, ILLaVAModel to use enum - Apply pattern to AudioGenModel as example: - Tokenizer now required for ONNX mode (T5-based) - Replace placeholder tokenization with proper ITokenizer usage The golden standard pattern: - ONNX mode: Tokenizer is REQUIRED (user loads pretrained models) - Native/Training mode: Create appropriate default per backbone 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- AudioGenModel: remove AudioGenOptions dependency, add two constructors (ONNX and Native mode), add code regions, fix UpdateParameters - WhisperModel: remove WhisperOptions dependency, add two constructors, add code regions, use DenseLayer instead of Conv1DLayer - TtsModel: remove TtsOptions dependency, add two constructors, add code regions, fix Train method with ToVector conversion 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Remove dependency on nerfoptions class - Single constructor with all parameters having defaults - Add proper code regions for organization - Fix train method to use _lossfunction and tovector() pattern - Industry-standard defaults (10 position encoding levels, etc.) - Comprehensive documentation with for beginners sections 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 7
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
♻️ Duplicate comments (5)
src/Audio/Whisper/WhisperModel.cs (1)
578-609: Incorrect duration calculation uses sample value instead of sample count.Line 606 computes
DurationSecondsby reading the last audio sample's value (audio[audio.Length - 1]) instead of using the audio length. This produces incorrect duration values. The correct calculation should divide the number of samples by the sample rate.🔎 Proposed fix
return new TranscriptionResult<T> { Text = text, Language = effectiveLanguage, Confidence = NumOps.FromDouble(1.0), // TODO: Extract from decoder - DurationSeconds = NumOps.ToDouble(audio[audio.Length - 1]) / SampleRate, + DurationSeconds = (double)audio.Length / SampleRate, Segments = includeTimestamps ? ExtractSegments(tokens, text) : Array.Empty<TranscriptionSegment<T>>() };src/Audio/SourceSeparation/MusicSourceSeparator.cs (2)
292-318: Buffer overflow whenkernelSizeis even.The
windowarray is allocated with sizekernelSize, but the loop iterates2 * halfKernel + 1times. WhenkernelSizeis even (e.g., 4),halfKernel = 2, and the loop runs 5 times, causing anIndexOutOfRangeExceptiononwindow[count++].🔎 Proposed fix
private Tensor<T> MedianFilterTime(Tensor<T> input, int kernelSize) { int numFrames = input.Shape[0]; int numBins = input.Shape[1]; var output = new Tensor<T>([numFrames, numBins]); int halfKernel = kernelSize / 2; + int windowSize = 2 * halfKernel + 1; for (int f = 0; f < numBins; f++) { - var window = new double[kernelSize]; + var window = new double[windowSize]; for (int t = 0; t < numFrames; t++) { int count = 0; for (int k = -halfKernel; k <= halfKernel; k++) { int ti = Math.Max(0, Math.Min(numFrames - 1, t + k)); window[count++] = _numOps.ToDouble(input[ti, f]); } Array.Sort(window, 0, count); output[t, f] = _numOps.FromDouble(window[count / 2]); } } return output; }
320-346: Same buffer overflow issue asMedianFilterTime.Apply the same fix here—allocate
windowwith size2 * halfKernel + 1instead ofkernelSize.🔎 Proposed fix
private Tensor<T> MedianFilterFrequency(Tensor<T> input, int kernelSize) { int numFrames = input.Shape[0]; int numBins = input.Shape[1]; var output = new Tensor<T>([numFrames, numBins]); int halfKernel = kernelSize / 2; + int windowSize = 2 * halfKernel + 1; for (int t = 0; t < numFrames; t++) { - var window = new double[kernelSize]; + var window = new double[windowSize];src/Audio/Fingerprinting/SpectrogramFingerprinter.cs (2)
148-161:PeaksPerSecondnaming issue remains unaddressed.As noted in a previous review,
windowSizeis set toPeaksPerSecond, but this value represents a frame window size, not peaks per second. The naming is misleading.
203-217:FrameCountis set to peak count instead of actual frame count.As noted in a previous review,
FrameCount = peaks.Countis inconsistent with the semantic meaning of frame count. This should be the actual number of spectrogram frames for consistency withChromaprintFingerprinter.
🟠 Major comments (20)
src/Audio/Whisper/WhisperModel.cs-293-359 (1)
293-359: Resource leak if decoder ONNX model loading fails.At lines 348-349, the encoder ONNX model is created before the decoder. If the decoder constructor throws (e.g., file not found, invalid ONNX), the encoder is never disposed, causing a resource leak.
🔎 Proposed fix
// Load ONNX models var options = onnxOptions ?? new OnnxModelOptions(); - OnnxEncoder = new OnnxModel<T>(encoderPath, options); - OnnxDecoder = new OnnxModel<T>(decoderPath, options); + OnnxModel<T>? encoder = null; + try + { + encoder = new OnnxModel<T>(encoderPath, options); + OnnxDecoder = new OnnxModel<T>(decoderPath, options); + OnnxEncoder = encoder; + } + catch + { + encoder?.Dispose(); + throw; + }src/Audio/Features/SpectralFeatureExtractor.cs-112-116 (1)
112-116: Avoid duplicate centroid computation.When both
CentroidandBandwidthflags are enabled,ComputeSpectralCentroidis called twice (Line 109 and Line 114). Cache the result to eliminate redundant work.🔎 Proposed fix
// Extract features var features = new List<double[]>(); + double[]? centroids = null; if (_featureTypes.HasFlag(SpectralFeatureType.Centroid)) { - features.Add(ComputeSpectralCentroid(magnitude, freqBins, numFrames, numFreqs)); + centroids = ComputeSpectralCentroid(magnitude, freqBins, numFrames, numFreqs); + features.Add(centroids); } if (_featureTypes.HasFlag(SpectralFeatureType.Bandwidth)) { - var centroids = ComputeSpectralCentroid(magnitude, freqBins, numFrames, numFreqs); + centroids ??= ComputeSpectralCentroid(magnitude, freqBins, numFrames, numFreqs); features.Add(ComputeSpectralBandwidth(magnitude, freqBins, centroids, numFrames, numFreqs)); }src/Audio/SourceSeparation/MusicSourceSeparator.cs-444-485 (1)
444-485: Validate mask tensor shape before indexing.Line 465 assumes the mask tensor has shape
[batch, stems, time, freq]with at least 4 dimensions. If the ONNX model returns a different shape (e.g.,[stems, time, freq]or[batch, time, freq, stems]), the bounds check may pass incorrectly or the indexing at Line 467 will throw an exception.🔎 Proposed fix
private SeparationResult<T> ApplyMasksAndReconstruct(Tensor<Complex<T>> stft, Tensor<T> masks) { + // Validate mask tensor shape + if (masks.Rank != 4) + throw new ArgumentException($"Expected 4D mask tensor [batch, stems, time, freq], got rank {masks.Rank}"); + + if (masks.Shape[1] < 4) + throw new ArgumentException($"Expected at least 4 stems in mask tensor, got {masks.Shape[1]}"); + // masks expected shape: [batch, stems, time, freq] or [batch, stems, freq, time] int numFrames = stft.Shape[0]; int numBins = stft.Shape[1];src/Helpers/LayerHelper.cs-4482-4498 (1)
4482-4498: Documentation is misleading given architectural limitations.The documentation states "This creates a trainable model from scratch" (line 4495), but the implementation cannot create a functional Whisper model due to the architectural issue: encoder-decoder models cannot be expressed as a flat sequential layer list.
Update the documentation once the architectural issue is resolved to accurately describe what the method creates and how it should be used.
src/Helpers/LayerHelper.cs-4631-4631 (1)
4631-4631: Consider softmax activation for vocabulary prediction.The output layer uses
identityActivation, which outputs raw logits. For vocabulary prediction in text generation, you typically want a softmax activation to produce a probability distribution over the vocabulary. Compare with other text generation methods in this class (e.g., line 1202, 1207, 2619).🔎 Proposed change
// Output projection to vocabulary -yield return new DenseLayer<T>(modelDimension, vocabularySize, identityActivation); +yield return new DenseLayer<T>(modelDimension, vocabularySize, new SoftmaxActivation<T>() as IActivationFunction<T>);Alternatively, if you prefer to keep logits output (e.g., for certain loss functions), add a clarifying comment explaining why identity activation is used.
src/Audio/Speaker/SpeakerEmbedding.cs-32-52 (1)
32-52: Validate vector dimensions and add null check.The method takes the minimum length of the two vectors, which allows comparison of embeddings with different dimensions. Speaker embeddings should have fixed dimensions (as configured in
SpeakerEmbeddingOptions.EmbeddingDimension), and comparing partial overlaps produces misleading similarity scores.Additionally, the
otherparameter lacks a null check.🔎 Proposed fix
public double CosineSimilarity(SpeakerEmbedding<T> other) { + ArgumentNullException.ThrowIfNull(other); + + if (Vector.Length != other.Vector.Length) + { + throw new ArgumentException( + $"Vector dimensions must match. Expected {Vector.Length}, got {other.Vector.Length}.", + nameof(other)); + } + double dot = 0; double norm1 = 0; double norm2 = 0; - int len = Math.Min(Vector.Length, other.Vector.Length); - for (int i = 0; i < len; i++) + for (int i = 0; i < Vector.Length; i++) { double v1 = NumOps.ToDouble(Vector[i]); double v2 = NumOps.ToDouble(other.Vector[i]); dot += v1 * v2; norm1 += v1 * v1; norm2 += v2 * v2; } double denominator = Math.Sqrt(norm1 * norm2); if (denominator < 1e-10) return 0; return dot / denominator; }src/Audio/Speaker/SpeakerEmbedding.cs-57-69 (1)
57-69: Validate vector dimensions and add null check.Same issue as
CosineSimilarity: the method uses the minimum length, allowing dimension mismatches that produce incorrect distance metrics. Add a null check and dimension validation.🔎 Proposed fix
public double EuclideanDistance(SpeakerEmbedding<T> other) { + ArgumentNullException.ThrowIfNull(other); + + if (Vector.Length != other.Vector.Length) + { + throw new ArgumentException( + $"Vector dimensions must match. Expected {Vector.Length}, got {other.Vector.Length}.", + nameof(other)); + } + double sumSquared = 0; - int len = Math.Min(Vector.Length, other.Vector.Length); - for (int i = 0; i < len; i++) + for (int i = 0; i < Vector.Length; i++) { double diff = NumOps.ToDouble(Vector[i]) - NumOps.ToDouble(other.Vector[i]); sumSquared += diff * diff; } return Math.Sqrt(sumSquared); }src/Audio/Speaker/SpeakerDiarizer.cs-156-186 (1)
156-186: Memory and performance concerns for long audio.The similarity matrix (line 163) is allocated as a full
n × narray wherenis the number of segments. For long recordings with short hop durations, this can consume significant memory and the subsequent agglomerative clustering has O(n³) complexity.Example: A 1-hour recording with 0.5s hop produces ~7,200 segments, resulting in a 7200×7200 similarity matrix (~414 MB) and potentially millions of cluster comparison operations.
🔎 Recommended optimizations
Option 1: Add input validation with warnings
private int[] ClusterEmbeddings(List<SpeakerEmbedding<T>> embeddings) { if (embeddings.Count == 0) return []; + + if (embeddings.Count > 5000) + throw new InvalidOperationException( + $"Clustering {embeddings.Count} segments may cause memory/performance issues. " + + "Consider increasing WindowDurationSeconds or HopDurationSeconds."); // Compute similarity matrixOption 2: Use sparse clustering for large inputs
- Switch to k-means or online clustering for segment counts > 1000
- Compute similarities on-demand rather than storing full matrix
- Use approximate nearest neighbor techniques (e.g., locality-sensitive hashing)
Option 3: Document limitations
Add to class XML docs:/// <remarks> /// <para><b>Performance Note:</b> Memory and computational complexity scale /// quadratically with the number of audio segments. For recordings longer than /// 10 minutes, increase WindowDurationSeconds and HopDurationSeconds to reduce /// segment count.</para> /// </remarks>src/Audio/Classification/AudioEventDetector.cs-244-272 (1)
244-272: Last partial window is dropped when complete windows exist (data loss issue).The loop at line 250 stops when
start + windowSamples > audio.Length, but the partial window handling at lines 261-269 only activates ifwindows.Count == 0. This means if any complete windows exist, the remaining audio samples that don't fill a complete window are silently dropped.Example: For 1000 samples with
windowSamples=512andhopSamples=256, windows are created at offsets 0, 256, 512 (which requires 1024 samples), but the loop stops. Samples 768-999 (232 samples) are lost.Consider adding logic after line 258 to handle the last partial window even when complete windows exist, possibly by zero-padding or processing the remaining samples.
🔎 Proposed fix to handle last partial window
for (int start = 0; start + windowSamples <= audio.Length; start += hopSamples) { var window = new Tensor<T>([windowSamples]); for (int i = 0; i < windowSamples; i++) { window[i] = audio[start + i]; } windows.Add(window); } + // Handle last partial window if there are remaining samples + int lastStart = windows.Count > 0 ? windows.Count * hopSamples : 0; + if (lastStart < audio.Length) + { + int remainingSamples = audio.Length - lastStart; + var window = new Tensor<T>([remainingSamples]); + for (int i = 0; i < remainingSamples; i++) + { + window[i] = audio[lastStart + i]; + } + windows.Add(window); + } + - // Handle last partial window - if (windows.Count == 0 && audio.Length > 0) - { - var window = new Tensor<T>([audio.Length]); - for (int i = 0; i < audio.Length; i++) - { - window[i] = audio[i]; - } - windows.Add(window); - }src/Audio/Features/ConstantQTransform.cs-227-262 (1)
227-262: Apply the same input validation as suggested for Transform().This method has the same validation issues as
Transform()(null check, shape validation, minimum length check). Apply the same fixes here to ensure consistent error handling.src/Audio/Features/ConstantQTransform.cs-83-119 (1)
83-119: Add input validation to prevent invalid configurations.The constructor accepts parameters without validation, which could lead to runtime errors or nonsensical results. For example:
- Negative or zero
sampleRate,fMin,binsPerOctave,numOctaves, orhopLengthwould produce invalid calculations- Very large
binsPerOctavecould cause division issues in the Q factor calculation- The window length calculation could overflow for extreme parameter combinations
🔎 Proposed fix to add parameter validation
public ConstantQTransform( int sampleRate = 22050, double fMin = 32.70, int binsPerOctave = 12, int numOctaves = 7, int hopLength = 512, WindowType windowType = WindowType.Hann) { + if (sampleRate <= 0) + throw new ArgumentOutOfRangeException(nameof(sampleRate), "Sample rate must be positive."); + if (fMin <= 0) + throw new ArgumentOutOfRangeException(nameof(fMin), "Minimum frequency must be positive."); + if (binsPerOctave <= 0) + throw new ArgumentOutOfRangeException(nameof(binsPerOctave), "Bins per octave must be positive."); + if (numOctaves <= 0) + throw new ArgumentOutOfRangeException(nameof(numOctaves), "Number of octaves must be positive."); + if (hopLength <= 0) + throw new ArgumentOutOfRangeException(nameof(hopLength), "Hop length must be positive."); + _numOps = MathHelper.GetNumericOperations<T>(); _sampleRate = sampleRate;src/Audio/Features/ConstantQTransform.cs-181-220 (1)
181-220: Add input validation for the audio tensor.The method lacks validation that could lead to incorrect results or runtime errors:
- No null check:
audiocould be null- No shape validation: Expects 1D tensor but doesn't verify rank
- Insufficient length handling: If audio is shorter than the longest window,
numFramescalculation becomes negative, then forced to 1, producing incorrect resultsThe boundary check
(frameStart + n) < numSamplesprevents crashes but silently produces zero-padded results for too-short audio, which may not be the intended behavior.🔎 Proposed fix to add validation
public Tensor<T> Transform(Tensor<T> audio) { + if (audio == null) + throw new ArgumentNullException(nameof(audio)); + if (audio.Rank != 1) + throw new ArgumentException("Audio must be a 1D tensor.", nameof(audio)); + int numSamples = audio.Shape[0]; + + int minRequiredSamples = _windowLengths[_numBins - 1]; + if (numSamples < minRequiredSamples) + throw new ArgumentException( + $"Audio too short: need at least {minRequiredSamples} samples, got {numSamples}.", + nameof(audio)); + int numFrames = (numSamples - _windowLengths[_numBins - 1]) / _hopLength + 1; - numFrames = Math.Max(1, numFrames);src/Audio/Classification/SceneClassifier.cs-411-420 (1)
411-420: Guard against empty audio in RMS energy computation.Line 419 divides by
audio.Length. If the audio tensor is empty, this throwsDivideByZeroException. Add a guard:- return Math.Sqrt(sum / audio.Length); + return audio.Length > 0 ? Math.Sqrt(sum / audio.Length) : 0;src/Audio/Classification/SceneClassifier.cs-378-409 (1)
378-409: Guard against zero frames in spectral contrast computation.Line 408 divides by
melSpec.Shape[0] * numBands. IfmelSpec.Shape[0](number of frames) is zero, this throwsDivideByZeroException. Add a guard:return totalContrast / (melSpec.Shape[0] * numBands); + return melSpec.Shape[0] > 0 ? totalContrast / (melSpec.Shape[0] * numBands) : 0;src/Audio/Classification/SceneClassifier.cs-422-433 (1)
422-433: Guard against empty audio in zero crossing rate computation.Line 432 divides by
audio.Length. If the audio tensor is empty, this throwsDivideByZeroException. Add a guard:- return (double)crossings / audio.Length; + return audio.Length > 0 ? (double)crossings / audio.Length : 0;src/Audio/Classification/SceneClassifier.cs-435-453 (1)
435-453: Guard against empty spectrogram in energy variance computation.Lines 449-450 call
Average()and divide byframeEnergies.Length. IfmelSpec.Shape[0]is zero,frameEnergiesis empty, andAverage()throwsInvalidOperationException. Add a guard:+ if (frameEnergies.Length == 0) + return 0; + double mean = frameEnergies.Average(); double variance = frameEnergies.Sum(e => (e - mean) * (e - mean)) / frameEnergies.Length;src/Audio/Classification/SceneClassifier.cs-250-274 (1)
250-274: Guard against zero frames when computing MFCC statistics.In addition to the division by zero at line 283 (already flagged), lines 265 and 273 will throw if
numFrames == 0. The method should validate thatnumFrames > 0at the start:int numFrames = mfccs.Shape[0]; int numCoeffs = mfccs.Shape[1]; + + if (numFrames == 0) + throw new ArgumentException("Audio must contain at least one MFCC frame.", nameof(audio));Or return a safe default feature set for empty audio. This prevents cascading divisions by zero.
src/Audio/AudioGen/AudioGenModel.cs-764-783 (1)
764-783: Optimizer parameter is ignored; learning rate is hardcoded.The constructor accepts an
optimizerparameter (line 340, 477) and stores it in_optimizer(lines 403, 507), butUpdateParametershardcodes the learning rate at 0.001 (line 775) and performs manual gradient descent instead of delegating to the optimizer. This prevents users from configuring learning rates or using different optimization algorithms (Adam, RMSprop, etc.).🔎 Proposed fix
Delegate parameter updates to the optimizer:
public override void UpdateParameters(Vector<T> gradients) { if (!_useNativeMode) { throw new NotSupportedException("Cannot update parameters in ONNX inference mode. Use the native constructor for training."); } - // Get current parameters - var currentParams = GetParameters(); - - // Apply gradient descent: params = params - learning_rate * gradients - T learningRate = NumOps.FromDouble(0.001); // Default learning rate - for (int i = 0; i < currentParams.Length; i++) - { - currentParams[i] = NumOps.Subtract(currentParams[i], NumOps.Multiply(learningRate, gradients[i])); - } - - // Set the updated parameters - SetParameters(currentParams); + // Delegate to the optimizer + var currentParams = GetParameters(); + var updatedParams = _optimizer.UpdateParameters(currentParams, gradients); + SetParameters(updatedParams); }Note: Verify that the optimizer interface supports this usage pattern; adjust as needed for the actual
IOptimizerAPI.Committable suggestion skipped: line range outside the PR's diff.
src/Audio/AudioGen/AudioGenModel.cs-1016-1028 (1)
1016-1028: Inefficient tensor growth causes O(n²) copying.Growing
currentTokensby copying to a new larger tensor on each iteration (lines 1018-1027) results in O(n²) time complexity. For a 30-second generation (~1500 tokens), this performs 1500 iterations with growing copy operations, creating significant overhead.🔎 Proposed optimization
Pre-allocate the full tensor and track the current position:
-var currentTokens = new Tensor<T>([1, _numCodebooks, 1]); +var currentTokens = new Tensor<T>([1, _numCodebooks, numTokens]); +int currentPos = 0; for (int cb = 0; cb < _numCodebooks; cb++) { - currentTokens[0, cb, 0] = NumOps.Zero; + currentTokens[0, cb, currentPos] = NumOps.Zero; } +currentPos++; for (int t = 0; t < numTokens; t++) { Tensor<T> logits; if (_useNativeMode) { - logits = ForwardLanguageModel(textEmbeddings, currentTokens); + // Pass only the filled portion + var currentView = currentTokens.Slice(0, currentPos); + logits = ForwardLanguageModel(textEmbeddings, currentView); } else { var inputs = new Dictionary<string, Tensor<T>> { ["text_embeddings"] = textEmbeddings, - ["audio_codes"] = currentTokens + ["audio_codes"] = currentTokens.Slice(0, currentPos) }; var outputs = _languageModel!.Run(inputs); logits = outputs.Values.First(); } for (int cb = 0; cb < _numCodebooks; cb++) { int nextToken = SampleFromLogits(logits, cb, random); codes[0, cb, t] = NumOps.FromDouble(nextToken); - - if (t < numTokens - 1) - { - var newTokens = new Tensor<T>([1, _numCodebooks, currentTokens.Shape[2] + 1]); - for (int c = 0; c < _numCodebooks; c++) - { - for (int i = 0; i < currentTokens.Shape[2]; i++) - { - newTokens[0, c, i] = currentTokens[0, c, i]; - } - newTokens[0, c, currentTokens.Shape[2]] = codes[0, c, t]; - } - currentTokens = newTokens; - } + currentTokens[0, cb, currentPos] = codes[0, cb, t]; } + currentPos++; }Note: This assumes Tensor supports slicing; if not, consider using a different data structure or passing currentPos as a length parameter.
Apply the same fix to
GenerateAudioCodesWithGuidanceat lines 1124-1136.Committable suggestion skipped: line range outside the PR's diff.
src/Audio/AudioGen/AudioGenModel.cs-888-912 (1)
888-912: Serialization doesn't preserve ONNX mode; deserialization always creates native instances.
CreateNewInstancealways uses the native constructor (line 890) regardless of the original mode. When an ONNX-mode model is serialized,_useNativeMode = falseis written (line 841), but deserialization reads and discards this value (line 866), and CreateNewInstance reconstructs in native mode. This breaks round-trip serialization for ONNX models, as they cannot be restored to their original ONNX state.🔎 Proposed fix
Store the mode and paths in instance fields during deserialization, then use them in CreateNewInstance:
+private bool _deserializedUseNativeMode; +private AudioGenModelSize _deserializedModelSize; +// ... other deserialized fields + protected override void DeserializeNetworkSpecificData(BinaryReader reader) { - // Read values to advance stream position (validation done in CreateNewInstance) - _ = reader.ReadBoolean(); // useNativeMode - _ = reader.ReadInt32(); // modelSize + _deserializedUseNativeMode = reader.ReadBoolean(); + _deserializedModelSize = (AudioGenModelSize)reader.ReadInt32(); // ... read other fields into instance variables - _ = reader.ReadInt32(); // sampleRate + _deserializedSampleRate = reader.ReadInt32(); // ... etc. } protected override IFullModel<T, Tensor<T>, Tensor<T>> CreateNewInstance() { + if (!_deserializedUseNativeMode) + { + // Cannot restore ONNX mode without model paths + throw new NotSupportedException( + "Cannot deserialize ONNX-mode AudioGen models. " + + "ONNX models require file paths that are not preserved in serialization. " + + "Use FromFiles or the ONNX constructor to recreate the model."); + } + return new AudioGenModel<T>( Architecture, - _modelSize, - _sampleRate, + _deserializedModelSize, + _deserializedSampleRate, // ... use deserialized values ); }Alternatively, if ONNX models shouldn't be serializable, document this clearly and throw in
SerializeNetworkSpecificDatawhen!_useNativeMode.Committable suggestion skipped: line range outside the PR's diff.
🟡 Minor comments (7)
src/Audio/SourceSeparation/SourceSeparationOptions.cs-20-20 (1)
20-20: Enforce StemCount constraint.The documentation states "2, 4, or 5" but no validation prevents invalid values like 0, 1, 3, or 100. Invalid stem counts will cause issues downstream (e.g., Line 489-495 in MusicSourceSeparator.cs falls back to 4 for any invalid value).
🔎 Proposed fix
- /// <summary>Number of stems to separate (2, 4, or 5). Default: 4.</summary> - public int StemCount { get; set; } = 4; + private int _stemCount = 4; + + /// <summary>Number of stems to separate (2, 4, or 5). Default: 4.</summary> + public int StemCount + { + get => _stemCount; + set + { + if (value is not (2 or 4 or 5)) + throw new ArgumentOutOfRangeException(nameof(value), "StemCount must be 2, 4, or 5."); + _stemCount = value; + } + }src/Audio/Features/MfccOptions.cs-30-34 (1)
30-34: Validate AppendDeltaDelta constraint.The documentation states that
AppendDeltaDeltarequiresAppendDeltato be true, but there's no validation enforcing this constraint. If a user setsAppendDeltaDelta = truewhileAppendDelta = false, it could lead to incorrect feature dimensions or runtime errors.🔎 Suggested validation approach
Consider adding a validation method or property:
+ /// <summary> + /// Validates the options configuration. + /// </summary> + public void Validate() + { + if (AppendDeltaDelta && !AppendDelta) + throw new InvalidOperationException("AppendDeltaDelta requires AppendDelta to be true."); + }Or validate in the consumer (MfccExtractor constructor):
public MfccExtractor(MfccOptions? options = null) : base(options) { var mfccOptions = options ?? new MfccOptions(); + + if (mfccOptions.AppendDeltaDelta && !mfccOptions.AppendDelta) + throw new ArgumentException("AppendDeltaDelta requires AppendDelta to be true.", nameof(options));Committable suggestion skipped: line range outside the PR's diff.
src/Audio/Features/MfccExtractor.cs-51-60 (1)
51-60: FeatureDimension calculation assumes valid configuration.The calculation at line 57
dim = dim / 2 * 3assumes thatAppendDeltawas already applied (line 56). This only works correctly whenAppendDeltais true. IfAppendDeltaDeltais true butAppendDeltais false (an invalid configuration per MfccOptions documentation), this produces an incorrect dimension.This highlights the importance of validating the constraint documented in MfccOptions (that
AppendDeltaDeltarequiresAppendDelta).🔎 Suggested fix
Add validation in the constructor to enforce the constraint:
public MfccExtractor(MfccOptions? options = null) : base(options) { var mfccOptions = options ?? new MfccOptions(); + + if (mfccOptions.AppendDeltaDelta && !mfccOptions.AppendDelta) + throw new ArgumentException( + "AppendDeltaDelta requires AppendDelta to be true.", + nameof(options)); _numCoefficients = mfccOptions.NumCoefficients;Committable suggestion skipped: line range outside the PR's diff.
src/Audio/Classification/GenreClassifier.cs-309-364 (1)
309-364: Consider edge case in BPM calculation.The tempo estimation uses autocorrelation to find periodicity. Line 363 computes
60.0 / lagSeconds, which could theoretically divide by zero ifbestLagis 0 or very small. WhilebestLagis initialized tominLag(which should be positive for normal sample rates), consider adding a defensive check to avoid potential division by zero in edge cases.🔎 Suggested defensive check
// Convert lag to BPM double lagSeconds = (double)bestLag * frameSize / _options.SampleRate; - return 60.0 / lagSeconds; + return lagSeconds > 0 ? 60.0 / lagSeconds : 120.0; // Default to 120 BPMsrc/Audio/Features/ConstantQTransform.cs-156-174 (1)
156-174: Handle edge case when window length is 1.The window formulas use
(length - 1)as the denominator, which causes division by zero whenlength == 1. While this is unlikely in typical CQT usage, it could occur with extreme parameter combinations (very high frequencies, low sample rates).🔎 Proposed fix to handle length == 1
private static double[] CreateWindow(int length, WindowType windowType) { var window = new double[length]; + + if (length == 1) + { + window[0] = 1.0; + return window; + } for (int n = 0; n < length; n++) {src/Audio/MusicAnalysis/ChordSegment.cs-23-26 (1)
23-26: Duration can be negative if EndTime < StartTime.The computed
Durationproperty doesn't prevent negative values. Ensure upstream code validates thatEndTime >= StartTimewhen populating this result.src/Audio/AudioGen/AudioGenModel.cs-603-632 (1)
603-632: Add validation for duration parameter.The
durationSecondsparameter is not validated against_maxDurationSeconds. Passing values exceeding the maximum could lead to excessive resource consumption or unexpected behavior.🔎 Suggested validation
public Tensor<T> GenerateAudio( string prompt, string? negativePrompt = null, double durationSeconds = 5.0, int numInferenceSteps = 100, double guidanceScale = 3.0, int? seed = null) { ThrowIfDisposed(); + + if (durationSeconds <= 0) + throw new ArgumentOutOfRangeException(nameof(durationSeconds), "Duration must be positive."); + if (durationSeconds > _maxDurationSeconds) + throw new ArgumentOutOfRangeException(nameof(durationSeconds), + $"Duration {durationSeconds}s exceeds maximum {_maxDurationSeconds}s."); int seedUsed = seed ?? _random.Next(); // ... }
… null checks argumentnullexception.throwifnull was introduced in .net 6 and is not available in .net framework 4.7.1. replace with explicit null checks. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
src/Audio/TextToSpeech/TtsModel.cs (1)
321-329: Resource leak if vocoder construction fails.If the
OnnxModel<T>constructor for the vocoder at line 327 throws an exception, the already-created_acousticModelat line 322 will not be disposed, leaking resources.🔎 Proposed fix with exception safety
// Initialize preprocessor _preprocessor = new TtsPreprocessor(); - // Load acoustic model - _acousticModel = new OnnxModel<T>(acousticModelPath, onnxOptions ?? new OnnxModelOptions()); + OnnxModel<T>? acousticModel = null; + OnnxModel<T>? vocoder = null; + + try + { + // Load acoustic model + acousticModel = new OnnxModel<T>(acousticModelPath, onnxOptions ?? new OnnxModelOptions()); - // Load vocoder or create Griffin-Lim fallback - if (vocoderModelPath is not null && vocoderModelPath.Length > 0) - { - _vocoder = new OnnxModel<T>(vocoderModelPath, onnxOptions ?? new OnnxModelOptions()); - OnnxModel = _vocoder; - } + // Load vocoder or create Griffin-Lim fallback + if (vocoderModelPath is not null && vocoderModelPath.Length > 0) + { + vocoder = new OnnxModel<T>(vocoderModelPath, onnxOptions ?? new OnnxModelOptions()); + } + + _acousticModel = acousticModel; + _vocoder = vocoder; + if (_vocoder is not null) + { + OnnxModel = _vocoder; + } + } + catch + { + acousticModel?.Dispose(); + vocoder?.Dispose(); + throw; + }src/Audio/Whisper/WhisperModel.cs (1)
603-610: Incorrect duration calculation uses sample value instead of sample count.Line 608 computes
DurationSecondsby reading the last audio sample's value (audio[audio.Length - 1]) instead of using the audio length. This produces incorrect duration values.🔎 Proposed fix
return new TranscriptionResult<T> { Text = text, Language = effectiveLanguage, Confidence = NumOps.FromDouble(1.0), // TODO: Extract from decoder - DurationSeconds = NumOps.ToDouble(audio[audio.Length - 1]) / SampleRate, + DurationSeconds = (double)audio.Length / SampleRate, Segments = includeTimestamps ? ExtractSegments(tokens, text) : Array.Empty<TranscriptionSegment<T>>() };
🧹 Nitpick comments (1)
src/Audio/Whisper/WhisperModel.cs (1)
509-525: Extract hardcoded vocabulary size to a constant.The Whisper vocabulary size
51865is hardcoded at lines 510 and 525. Extract this to a named constant for maintainability.🔎 Proposed refactor
Add a constant near the top of the class:
+ /// <summary> + /// Whisper model vocabulary size. + /// </summary> + private const int WhisperVocabSize = 51865;Then use it in the layer initialization:
// Decoder: Token embeddings + Encoder output -> Logits // Token embedding (Whisper vocabulary is 51865 tokens) - _tokenEmbedding = new EmbeddingLayer<T>(51865, _modelDim); + _tokenEmbedding = new EmbeddingLayer<T>(WhisperVocabSize, _modelDim); // Decoder transformer layers for (int i = 0; i < _numDecoderLayers; i++) { var decoderLayer = new TransformerDecoderLayer<T>( embeddingSize: _modelDim, numHeads: _numHeads, feedForwardDim: _ffDim, sequenceLength: _maxTokens, ffnActivation: (IActivationFunction<T>?)null); _decoderLayers.Add(decoderLayer); } // Output projection to vocabulary - _outputProjection = new DenseLayer<T>(_modelDim, 51865); + _outputProjection = new DenseLayer<T>(_modelDim, WhisperVocabSize);
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/Audio/TextToSpeech/TtsModel.cssrc/Audio/Whisper/WhisperModel.cs
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-12-18T08:49:25.295Z
Learnt from: ooples
Repo: ooples/AiDotNet PR: 444
File: src/Interfaces/IPruningMask.cs:1-102
Timestamp: 2025-12-18T08:49:25.295Z
Learning: In the AiDotNet repository, the project-level global using includes AiDotNet.Tensors.LinearAlgebra via AiDotNet.csproj. Therefore, Vector<T>, Matrix<T>, and Tensor<T> are available without per-file using directives. Do not flag missing using directives for these types in any C# files within this project. Apply this guideline broadly to all C# files (not just a single file) to avoid false positives. If a file uses a type from a different namespace not covered by the global using, flag as usual.
Applied to files:
src/Audio/TextToSpeech/TtsModel.cssrc/Audio/Whisper/WhisperModel.cs
📚 Learning: 2025-12-18T08:49:53.103Z
Learnt from: ooples
Repo: ooples/AiDotNet PR: 444
File: src/Interfaces/IPruningStrategy.cs:1-4
Timestamp: 2025-12-18T08:49:53.103Z
Learning: In this repository, global using directives are declared in AiDotNet.csproj for core namespaces (AiDotNet.Tensors.* and AiDotNet.*) and common system types. When reviewing C# files, assume these global usings are in effect; avoid adding duplicate using statements for these namespaces and for types like Vector<T>, Matrix<T>, Tensor<T>, etc. If a type is not found, verify the global usings or consider adding a file-scoped using if needed. Prefer relying on global usings to reduce boilerplate.
Applied to files:
src/Audio/TextToSpeech/TtsModel.cssrc/Audio/Whisper/WhisperModel.cs
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Build (Windows)
- GitHub Check: CodeQL Analysis
🔇 Additional comments (1)
src/Audio/TextToSpeech/TtsModel.cs (1)
986-998: Verify disposal of Griffin-Lim vocoder.The
_griffinLimfield is not disposed in theDisposemethod. IfGriffinLim<T>implementsIDisposableor holds unmanaged resources, this could cause a resource leak.
…ttern - Two-constructor pattern (ONNX inference + native training) - CNN-based feature extraction with configurable blocks - Emotion classification with arousal/valence prediction - Time-series emotion recognition for long recordings - Uses internal layer lists following SileroVad pattern 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (4)
src/Audio/Classification/AudioEventDetector.cs (4)
660-678:_melSpectrogramis not reinitialized after deserialization.After deserializing updated option values (SampleRate, NumMels, FftSize, HopLength), the readonly
_melSpectrogramfield retains the instance created at construction with the original options. This causes a mismatch between the deserialized configuration and actual feature extraction behavior—PreprocessAudio(line 598),Detect(line 293), and other methods will extract features using stale parameters.🔎 Recommended approaches
Option 1: Block deserialization until proper support is implemented:
protected override void DeserializeNetworkSpecificData(BinaryReader reader) { + throw new NotSupportedException( + "Deserialization is not currently supported for AudioEventDetector. " + + "Please reconstruct the model from the original options and weights."); + // Restore options properties _options.SampleRate = reader.ReadInt32(); ...Option 2: Make
_melSpectrogrammutable and reinitialize:- private readonly MelSpectrogram<T>? _melSpectrogram; + private MelSpectrogram<T>? _melSpectrogram;Then at the end of
DeserializeNetworkSpecificData:ClassLabels = labels; + + // Reinitialize feature extractor with updated options + _melSpectrogram = new MelSpectrogram<T>( + sampleRate: _options.SampleRate, + nMels: _options.NumMels, + nFft: _options.FftSize, + hopLength: _options.HopLength, + fMin: _options.FMin, + fMax: _options.FMax, + logMel: true);
743-757: Silent fallback to rules-based classification in error state.The
elsebranch callingClassifyWithRulesis reached only when both conditions fail:
- ONNX mode is not properly initialized (
IsOnnxModefalse orOnnxEncodernull)- Native mode is disabled (
_useNativeModeis false)This occurs if the ONNX constructor was used but model loading failed, leaving the detector in an inconsistent state. Silently falling back to heuristic rules without a trained model produces unreliable results and masks initialization failures.
🔎 Proposed fix
Replace the silent fallback with explicit error handling:
private T[] ClassifyWindow(Tensor<T> melSpec, Tensor<T> audio) { if (IsOnnxMode && OnnxEncoder is not null) { return ClassifyWithModel(melSpec); } else if (_useNativeMode) { return ClassifyWithNative(melSpec); } else { - return ClassifyWithRules(melSpec, audio); + throw new InvalidOperationException( + "AudioEventDetector is in an invalid state: neither ONNX model nor native layers are available. " + + "Ensure the ONNX model loaded successfully or use native training mode."); } }Alternatively, if rules-based fallback is intentional for specific scenarios, document when and why this path executes.
1143-1206: CRITICAL: Thread-safety bug—whileloop processes shared state outside the lock.The
lock (_lock)at line 1150 closes after adding incoming samples to_buffer(line 1161), but thewhileloop (lines 1164-1204) that reads_buffer.Count, extracts windows from_buffer, processes them, updates_currentState, adds to_newEvents, and calls_buffer.RemoveRangeexecutes outside the lock.If
FeedAudiois called from multiple threads concurrently (common in real-time audio pipelines) or ifGetNewEvents/GetCurrentStateare called whileFeedAudiois processing, race conditions will corrupt_buffer,_newEvents, and_currentState, leading to exceptions, data loss, or incorrect event detection.Note: A previous review flagged this exact issue as critical and marked it "Addressed in commits 50dbbdb to 5cc2364," but the bug remains in the current code.
🔎 Proposed fix
Move the closing brace of the lock to encompass the entire processing loop:
public void FeedAudio(Tensor<T> audioChunk) { if (_disposed) { throw new ObjectDisposedException(nameof(StreamingEventDetectionSession)); } lock (_lock) { if (_disposed) { throw new ObjectDisposedException(nameof(StreamingEventDetectionSession)); } // Add to buffer for (int i = 0; i < audioChunk.Length; i++) { _buffer.Add(audioChunk[i]); } - // Process complete windows - while (_buffer.Count >= _windowSamples) - { + // Process complete windows + while (_buffer.Count >= _windowSamples) + { // Extract window var window = new Tensor<T>([_windowSamples]); for (int i = 0; i < _windowSamples; i++) { window[i] = _buffer[i]; } // Process window var melSpec = _detector._melSpectrogram?.Forward(window) ?? throw new InvalidOperationException("MelSpectrogram not initialized."); var scores = _detector.ClassifyWindow(melSpec, window); // Update state and check for events double thresholdValue = _detector.NumOps.ToDouble(_threshold); + AudioEvent<T>? evtToRaise = null; for (int i = 0; i < scores.Length && i < _detector.ClassLabels.Count; i++) { _currentState[_detector.ClassLabels[i]] = scores[i]; if (_detector.NumOps.ToDouble(scores[i]) >= thresholdValue) { var evt = new AudioEvent<T> { EventType = _detector.ClassLabels[i], Confidence = scores[i], StartTime = _processedTime, EndTime = _processedTime + _detector._options.WindowSize, PeakTime = _processedTime + _detector._options.WindowSize / 2 }; _newEvents.Add(evt); - EventDetected?.Invoke(this, evt); + evtToRaise = evt; } } // Advance buffer (with overlap) int hopSamples = (int)(_windowSamples * (1 - _detector._options.WindowOverlap)); _buffer.RemoveRange(0, hopSamples); _processedTime += hopSamples / (double)_sampleRate; + + // Invoke event outside lock to prevent deadlock if subscriber acquires locks + if (evtToRaise is not null) + { + // Release lock before invoking to avoid potential deadlocks + // Note: consider collecting all events and invoking after loop if multiple events per window are expected + } - } + } } + + // If events need to be raised, do so here outside the lock }Important: If multiple events can be detected per window, collect them in a local list inside the lock and invoke
EventDetectedfor each outside the lock to avoid holding the lock during callbacks.
204-215: Dead code:CreateMfccExtractoris never called.This method creates an
MfccExtractor<T>instance but is never invoked anywhere in the class. A previous review flagged the_mfccExtractorfield and this method as unused, and the field appears to have been removed, but the method remains.🔎 Proposed fix
Remove the unused method:
- private MfccExtractor<T> CreateMfccExtractor() - { - return new MfccExtractor<T>(new MfccOptions - { - SampleRate = _options.SampleRate, - NumCoefficients = 13, - FftSize = _options.FftSize, - HopLength = _options.HopLength, - FMin = _options.FMin, - FMax = _options.FMax - }); - }
🧹 Nitpick comments (3)
src/Audio/Enhancement/DCCRN.cs (3)
608-639: Consider batch-optimizing the LSTM projection for better performance.The per-timestep projection correctly addresses dimension compatibility but processes each timestep individually in nested loops. For better performance, consider reshaping to
[batch*time, hidden], applying the projection once, then reshaping back to[batch, time, encoderDim].🔎 Potential optimization approach
// Project LSTM output back to encoder spatial dimensions if (_lstmProjection is not null) { - // Apply projection per timestep: [batch, time, hidden] -> [batch, time, encoderDim] - var projectedData = new T[batchSize * timeFrames * _encoderOutputDim]; - for (int b = 0; b < batchSize; b++) - { - for (int t = 0; t < timeFrames; t++) - { - // Extract timestep data - var timestepInput = new Tensor<T>([_lstmHiddenDim]); - for (int h = 0; h < _lstmHiddenDim; h++) - { - int idx = b * timeFrames * _lstmHiddenDim + t * _lstmHiddenDim + h; - if (idx < reshaped.Length) - timestepInput[h] = reshaped.GetFlat(idx); - } - - // Project - var projected = _lstmProjection.Forward(timestepInput); - - // Store result - for (int e = 0; e < _encoderOutputDim && e < projected.Length; e++) - { - int outIdx = b * timeFrames * _encoderOutputDim + t * _encoderOutputDim + e; - if (outIdx < projectedData.Length) - projectedData[outIdx] = projected.GetFlat(e); - } - } - } - reshaped = new Tensor<T>(projectedData, [batchSize, timeFrames, _encoderOutputDim]); + // Reshape to [batch*time, hidden] for batch projection + var flatInput = reshaped.Reshape([batchSize * timeFrames, _lstmHiddenDim]); + var flatProjected = _lstmProjection.Forward(flatInput); + reshaped = flatProjected.Reshape([batchSize, timeFrames, _encoderOutputDim]); }
822-860: Consider making spatial dimension mismatch handling more explicit.The current implementation warns via
Debug.WriteLinewhen spatial dimensions don't match, then usesMath.Minto avoid crashes. While this prevents failures, the truncation behavior may hide architectural issues during development.For better debuggability, consider one of:
- Make the warning more visible (e.g., using a logging framework if available)
- Add a strict validation mode that throws on mismatch during development
- Add an architectural validation pass that checks dimension compatibility before the first forward pass
707-779: Consider using FFT library for better STFT performance.The current implementation uses manual DFT computation (O(N²) per frame), which is significantly slower than FFT-based implementations (O(N log N)). While correct for a reference implementation, real-world usage would benefit from a proper FFT library.
Consider using a FFT library (e.g., MathNet.Numerics.IntegralTransforms or a native binding) for production use, especially when processing longer audio segments.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/Audio/Classification/AudioEventDetector.cssrc/Audio/Enhancement/DCCRN.cs
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-12-18T08:49:25.295Z
Learnt from: ooples
Repo: ooples/AiDotNet PR: 444
File: src/Interfaces/IPruningMask.cs:1-102
Timestamp: 2025-12-18T08:49:25.295Z
Learning: In the AiDotNet repository, the project-level global using includes AiDotNet.Tensors.LinearAlgebra via AiDotNet.csproj. Therefore, Vector<T>, Matrix<T>, and Tensor<T> are available without per-file using directives. Do not flag missing using directives for these types in any C# files within this project. Apply this guideline broadly to all C# files (not just a single file) to avoid false positives. If a file uses a type from a different namespace not covered by the global using, flag as usual.
Applied to files:
src/Audio/Enhancement/DCCRN.cssrc/Audio/Classification/AudioEventDetector.cs
📚 Learning: 2025-12-18T08:49:53.103Z
Learnt from: ooples
Repo: ooples/AiDotNet PR: 444
File: src/Interfaces/IPruningStrategy.cs:1-4
Timestamp: 2025-12-18T08:49:53.103Z
Learning: In this repository, global using directives are declared in AiDotNet.csproj for core namespaces (AiDotNet.Tensors.* and AiDotNet.*) and common system types. When reviewing C# files, assume these global usings are in effect; avoid adding duplicate using statements for these namespaces and for types like Vector<T>, Matrix<T>, Tensor<T>, etc. If a type is not found, verify the global usings or consider adding a file-scoped using if needed. Prefer relying on global usings to reduce boilerplate.
Applied to files:
src/Audio/Enhancement/DCCRN.cssrc/Audio/Classification/AudioEventDetector.cs
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: CodeQL Analysis
- GitHub Check: Build (Windows)
🔇 Additional comments (6)
src/Audio/Enhancement/DCCRN.cs (6)
283-327: LGTM! Parameter validation is thorough.The constructor now properly validates all critical architecture parameters before initialization, including power-of-2 check for FFT size and valid hop size range. This prevents runtime errors from invalid configurations.
331-411: LGTM! Layer initialization correctly implements the DCCRN architecture.The initialization properly creates:
- Encoder stages with Conv + BatchNorm (2 layers each)
- LSTM layers with appropriate input/output dimensions
- LSTM projection layer to map back to encoder spatial dimensions (addresses past dimension mismatch issue)
- Decoder stages with symmetric structure
- Proper mask estimation layer
578-592: LGTM! Encoder forward pass correctly processes layer pairs.The loop correctly iterates by 2 to process Conv + BatchNorm pairs, matching the initialization structure. Skip connections are properly cached for decoder use.
644-669: LGTM! Decoder iteration logic correctly matches initialization.The condition
(i > 0) ? 2 : 1properly processes:
- 2 layers per stage for iterations i = numStages-1 down to 1
- 1 layer for the final iteration when i = 0
This matches the initialization pattern where the first (numStages-1) stages have 2 layers each, and the last stage has 1 layer. The decoderLayerIdx correctly consumes layers in the order they were added.
888-926: LGTM! Serialization correctly persists and restores model configuration.The serialization methods properly:
- Write all architecture parameters and config to binary format
- Restore configuration on deserialization
- Re-initialize native layers to match the restored config (line 922-925)
The approach correctly restores a model to its saved state, including architecture parameters.
1-960: Excellent implementation of dual-mode DCCRN with comprehensive functionality.This implementation successfully provides:
- Dual-mode operation (ONNX inference + native training)
- Proper parameter validation and error handling
- Complete forward and backward passes for training
- Serialization support for model persistence
- Clear documentation with beginner-friendly examples
The architecture correctly implements the DCCRN paper's design with encoder-LSTM-decoder structure, skip connections, and complex mask estimation. All critical issues from past reviews have been properly addressed.
- fix audioldm/audiogen fragile layer indexing using layer boundary fields - fix audiogen o(n^2) tensor copy overhead with pre-allocation - remove null-forgiving operators and use proper null checks - upgrade fftsharp from 2.1.0 to 2.2.0 - add documentation for classifywithrules fallback behavior - improve audioldm tokenizer compatibility documentation 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Security fix: use cryptographically secure random via RandomHelper instead of directly instantiating System.Random. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (9)
src/Audio/AudioLDM/AudioLDMModel.cs (3)
254-258: Tokenizer compatibility concern is properly addressed.The previous review flagged potential CLAP/FlanT5 tokenizer incompatibility. The updated comments (lines 254-257) now clearly document that:
- Native training mode uses T5-style tokenizer (appropriate for training from scratch)
- ONNX inference mode requires the user to provide the matching tokenizer via constructor parameter (line 150)
This is the correct design. The ONNX constructor enforces tokenizer provision (line 175-176) and documents the requirement (lines 136-137, 295-299).
1377-1390: Deserialization properly restores configuration values.The previous review flagged that deserialized values were being discarded. The code now correctly assigns the read values to
_optionsproperties (lines 1381-1384). The only discarded value isuseNativeMode(line 1379), which is appropriate because it's a readonly field set in the constructor. The comments clearly explain this design decision.
307-317: Layer boundaries still use fragile fraction-based logic.While the code now uses boundary fields (lines 83-89) instead of inline fractions, the boundaries are still calculated using simple division (lines 312-317):
_clapLayerEnd = totalLayers / 3; _unetLayerStart = _clapLayerEnd; _unetLayerEnd = 2 * totalLayers / 3;This breaks when users provide custom layer configurations that don't follow the exact 1/3, 1/3, 1/3 split.
ValidateLayerConfigurationonly checks that at least 3 layers exist, but doesn't validate the architecture structure.Recommended approach:
- Add explicit boundary markers to
NeuralNetworkArchitecture<T>(e.g.,ClapLayerEnd,UNetLayerEnd)- When custom layers are provided, require these markers to be set
- Update validation to check markers exist and are in valid order
- Use markers for routing instead of fractions
Also applies to: 83-89, 323-332, 629-640, 653-659, 675-680, 697-703
src/Audio/AudioGen/AudioGenModel.cs (4)
337-354: Parameter validation is comprehensive.The constructors now properly validate all configuration parameters including temperature, topK, topP, guidanceScale, channels, sampleRate, durationSeconds, and maxDurationSeconds. The validation is consistent between both constructors and throws appropriate exceptions with clear messages.
Also applies to: 383-416
Also applies to: 479-497
126-131: Thread-safety properly implemented for random generation.The previous review flagged
_randomas not thread-safe for concurrent generation. The code now includes_randomLock(line 131) and uses it to protect access to_random.Next()(lines 643-646). This ensures safe concurrent calls to generation methods.Also applies to: 404-406, 528-530
Also applies to: 643-646
1017-1070: Pre-allocation eliminates O(n²) overhead.The previous review flagged O(n²) tensor copy overhead during autoregressive generation. The code now:
- Pre-allocates
currentTokensbuffer to full capacity (lines 1017-1020)- Tracks length with
currentLengthvariable (line 1020)- Creates lightweight views for each forward pass (lines 1031-1038)
- Directly assigns new tokens without reallocation (lines 1066-1069)
This reduces complexity from O(n²) to O(n), significantly improving generation speed for longer durations.
Also applies to: 1113-1173
1319-1464: Native decoder implementation is complete and functional.The previous review flagged the native decoder as a critical placeholder that produced silence. The current implementation (lines 1329-1464) provides a complete EnCodec-style decoder with:
- Codebook aggregation (lines 1346-1364): Converts discrete codes to continuous embeddings with proper residual VQ weighting (line 1362:
weight = 1.0 / Math.Pow(2, cb))- Upsampling (lines 1369-1419): Interpolates between tokens using both linear and cosine curves for smoother output
- Click prevention (lines 1422-1461): Fades out remaining samples to avoid artifacts
- Multi-channel support (lines 1407-1418): Handles both mono and stereo output
The implementation is well-documented and produces actual audio instead of silence. This fully resolves the critical issue.
src/Audio/Classification/AudioEventDetector.cs (1)
204-215: Dead code:CreateMfccExtractoris never called.The
CreateMfccExtractormethod is defined but never invoked anywhere in the class. This appears to be leftover code from when the_mfccExtractorfield existed (which was removed in earlier commits).🔎 Proposed fix
- private MfccExtractor<T> CreateMfccExtractor() - { - return new MfccExtractor<T>(new MfccOptions - { - SampleRate = _options.SampleRate, - NumCoefficients = 13, - FftSize = _options.FftSize, - HopLength = _options.HopLength, - FMin = _options.FMin, - FMax = _options.FMax - }); - } -src/Audio/Classification/SceneClassifier.cs (1)
603-617: FeatureCount still doesn't reflect actual input dimensions.The past review noted that
FeatureCountshould represent input features, not output classes. The fix changed fromClassLabels.Countto_options.NumMfccs, but the actual input feature count isNumMfccs * 3 + 7 + 6as computed inCreateMinimalArchitecture(line 190).🔎 Proposed fix
public override ModelMetadata<T> GetModelMetadata() { + int inputFeatures = _options.NumMfccs * 3 + 7 + 6; var metadata = new ModelMetadata<T> { Name = _useNativeMode ? "SceneClassifier-Native" : "SceneClassifier-ONNX", Description = "Acoustic scene classification model (DCASE-style)", ModelType = ModelType.NeuralNetwork, - FeatureCount = _options.NumMfccs, // Input feature count, not output class count + FeatureCount = inputFeatures, Complexity = 1 };
🧹 Nitpick comments (2)
src/Audio/Classification/SceneClassifier.cs (2)
114-116: Consider deferring optimizer creation in ONNX mode.The
_optimizeris instantiated in both ONNX constructors (lines 116, 167) butTrain()throwsInvalidOperationExceptionwhen not in native mode. This wastes memory for inference-only instances.🔎 Proposed fix
Use lazy initialization or defer creation to the training path:
- private readonly IGradientBasedOptimizer<T, Tensor<T>, Tensor<T>> _optimizer; + private IGradientBasedOptimizer<T, Tensor<T>, Tensor<T>>? _optimizer;Then in
Train():public override void Train(Tensor<T> input, Tensor<T> expected) { if (!_useNativeMode) { throw new InvalidOperationException(...); } + _optimizer ??= new AdamWOptimizer<T, Tensor<T>, Tensor<T>>(this); SetTrainingMode(true);
687-746: Bounds checks are redundant after the early return guard.Lines 716, 726, and 738 check
t < mfccs.Shape[0] && c < mfccs.Shape[1], but this is always true after the early return at line 688 ensuresnumFrames > 0 && numCoeffs > 0. These can be safely removed to simplify the code.🔎 Proposed simplification
for (int c = 0; c < numCoeffs; c++) { double sum = 0; for (int t = 0; t < numFrames; t++) { - if (t < mfccs.Shape[0] && c < mfccs.Shape[1]) - { - sum += NumOps.ToDouble(mfccs[t, c]); - } + sum += NumOps.ToDouble(mfccs[t, c]); } mfccMean[c] = sum / numFrames; double sumSq = 0; for (int t = 0; t < numFrames; t++) { - if (t < mfccs.Shape[0] && c < mfccs.Shape[1]) - { - double diff = NumOps.ToDouble(mfccs[t, c]) - mfccMean[c]; - sumSq += diff * diff; - } + double diff = NumOps.ToDouble(mfccs[t, c]) - mfccMean[c]; + sumSq += diff * diff; } mfccStd[c] = Math.Sqrt(sumSq / numFrames); // Delta (first derivative) double sumDelta = 0; - for (int t = 1; t < numFrames && t < mfccs.Shape[0]; t++) + for (int t = 1; t < numFrames; t++) { - if (c < mfccs.Shape[1]) - { - double prev = NumOps.ToDouble(mfccs[t - 1, c]); - double curr = NumOps.ToDouble(mfccs[t, c]); - sumDelta += Math.Abs(curr - prev); - } + double prev = NumOps.ToDouble(mfccs[t - 1, c]); + double curr = NumOps.ToDouble(mfccs[t, c]); + sumDelta += Math.Abs(curr - prev); } mfccDelta[c] = numFrames > 1 ? sumDelta / (numFrames - 1) : 0.0; }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/AiDotNet.csprojsrc/Audio/AudioGen/AudioGenModel.cssrc/Audio/AudioLDM/AudioLDMModel.cssrc/Audio/Classification/AudioEventDetector.cssrc/Audio/Classification/SceneClassifier.cs
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-12-18T08:49:30.125Z
Learnt from: ooples
Repo: ooples/AiDotNet PR: 444
File: src/Interfaces/IPruningMask.cs:1-102
Timestamp: 2025-12-18T08:49:30.125Z
Learning: The AiDotNet project uses project-level global usings (configured in AiDotNet.csproj with `<Using Include=AiDotNet.Tensors.LinearAlgebra />`), making Vector<T>, Matrix<T>, and Tensor<T> available in all files without explicit per-file using directives. Do not flag missing using directives for these types in this project.
Applied to files:
src/AiDotNet.csproj
📚 Learning: 2025-12-18T08:50:00.720Z
Learnt from: ooples
Repo: ooples/AiDotNet PR: 444
File: src/Interfaces/IPruningStrategy.cs:1-4
Timestamp: 2025-12-18T08:50:00.720Z
Learning: The AiDotNet project uses global using directives in src/AiDotNet.csproj via <Using Include="..." /> for AiDotNet.Tensors.LinearAlgebra, AiDotNet.Tensors.Engines, AiDotNet.Tensors.Interfaces, AiDotNet.Tensors.NumericOperations, AiDotNet.Tensors.Helpers, AiDotNet.Autodiff, System.Text, and AiDotNet.Helpers. Types like Vector<T>, Matrix<T>, Tensor<T>, and related linear algebra types are available project-wide without per-file using statements.
Applied to files:
src/AiDotNet.csproj
📚 Learning: 2025-12-18T08:49:25.295Z
Learnt from: ooples
Repo: ooples/AiDotNet PR: 444
File: src/Interfaces/IPruningMask.cs:1-102
Timestamp: 2025-12-18T08:49:25.295Z
Learning: In the AiDotNet repository, the project-level global using includes AiDotNet.Tensors.LinearAlgebra via AiDotNet.csproj. Therefore, Vector<T>, Matrix<T>, and Tensor<T> are available without per-file using directives. Do not flag missing using directives for these types in any C# files within this project. Apply this guideline broadly to all C# files (not just a single file) to avoid false positives. If a file uses a type from a different namespace not covered by the global using, flag as usual.
Applied to files:
src/Audio/AudioLDM/AudioLDMModel.cssrc/Audio/AudioGen/AudioGenModel.cssrc/Audio/Classification/SceneClassifier.cssrc/Audio/Classification/AudioEventDetector.cs
📚 Learning: 2025-12-18T08:49:53.103Z
Learnt from: ooples
Repo: ooples/AiDotNet PR: 444
File: src/Interfaces/IPruningStrategy.cs:1-4
Timestamp: 2025-12-18T08:49:53.103Z
Learning: In this repository, global using directives are declared in AiDotNet.csproj for core namespaces (AiDotNet.Tensors.* and AiDotNet.*) and common system types. When reviewing C# files, assume these global usings are in effect; avoid adding duplicate using statements for these namespaces and for types like Vector<T>, Matrix<T>, Tensor<T>, etc. If a type is not found, verify the global usings or consider adding a file-scoped using if needed. Prefer relying on global usings to reduce boilerplate.
Applied to files:
src/Audio/AudioLDM/AudioLDMModel.cssrc/Audio/AudioGen/AudioGenModel.cssrc/Audio/Classification/SceneClassifier.cssrc/Audio/Classification/AudioEventDetector.cs
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Build (Windows)
- GitHub Check: CodeQL Analysis
🔇 Additional comments (10)
src/AiDotNet.csproj (1)
66-66: Excellent—upgrade to FftSharp 2.2.0 implemented.This addresses the previous recommendation to upgrade from 2.1.0 to 2.2.0. The successful build with zero warnings confirms compatibility with both net8.0 and net471 targets, and the dependency appropriately supports the FFT-based audio processing features introduced in this PR.
src/Audio/Classification/SceneClassifier.cs (9)
44-44: LGTM!Class declaration follows the interface → base class → concrete class pattern as required by the project architecture. XML documentation is comprehensive with examples for both ONNX and training modes.
46-102: Well-structured static scene data.The
StandardScenesarray andSceneCategoriesdictionary provide a comprehensive DCASE-style taxonomy. The readonly modifiers are appropriately applied, and_useNativeModecorrectly lacks readonly to support serialization round-trips.
224-244: LGTM!The async factory method follows the established pattern for model download and initialization. Mutating
options.ModelPathafter download allows callers to inspect the resolved path.
369-392: Good parameter validation addresses infinite loop risk.The validation at lines 374-390 and the
Math.Max(1, ...)guard at line 392 properly prevent the infinite loop scenario flagged in the past review.
484-504: LGTM!The predict method correctly prioritizes ONNX inference, falls back to native layers, and finally to rule-based classification. The fallback cascade is appropriate for a model that may not always have weights loaded.
622-661: LGTM!Serialization and deserialization are symmetric, with all options and class labels properly persisted. The
_useNativeModefield is correctly restored (addressing the past review concern).
932-985: Division-by-zero guards properly implemented.
ComputeRmsEnergy,ComputeZeroCrossingRate, andComputeEnergyVarianceall include appropriate guards for empty input, addressing the past review concerns. The other spectral methods also properly guard their divisions.
1020-1040: LGTM!The rule-based fallback appropriately returns a uniform distribution when no neural network is available. The comment at line 1027 clearly documents this is a fallback mechanism.
1066-1077: Check if feature extractors need disposal.
OnnxEncoderis correctly disposed, but verify whether_melSpectrogramand_mfccExtractorimplementIDisposable. If they hold unmanaged resources, they should also be disposed in theDisposemethod.
- audiogen: use layer type detection for boundaries instead of fractions - audiogen: calculate exact boundaries for default layers based on structure - audioeventdetector: fix potential deadlock by invoking events outside lock 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add parameterless constructors to SpeakerEmbeddingExtractor and SpeakerVerifier - Add SpeakerEmbeddingExtractor(SpeakerEmbeddingOptions) constructor - Add Turns and Duration properties to DiarizationResult<T> for legacy API - Add AllProbabilities, TopPredictions, Features to GenreClassificationResult<T> - Add AllProbabilities, TopPredictions, Features to SceneClassificationResult<T> - Add EventLabels property to AudioEventDetector - Add Scenes and ClassifyCategory to SceneClassifier - Implement IEnumerable<AudioEvent<T>> on AudioEventResult<T> - Fix test: use EventType instead of Label, Count instead of Length 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
src/Audio/Classification/SceneClassifier.cs (1)
623-637: Incorrect FeatureCount in model metadata (regression).Line 630 sets
FeatureCount = _options.NumMfccs, but this is only one component of the input features. The correct input feature count is computed at line 190 asnumMfccs * 3 + 7 + 6(MFCC mean/std/delta + 7 spectral features + 6 band energies), which matches the feature tensor construction at line 800.This was previously flagged and appears to be a regression. The metadata should accurately reflect the model's input dimensions for proper introspection and validation.
🔎 Proposed fix
public override ModelMetadata<T> GetModelMetadata() { + int inputFeatures = _options.NumMfccs * 3 + 7 + 6; var metadata = new ModelMetadata<T> { Name = _useNativeMode ? "SceneClassifier-Native" : "SceneClassifier-ONNX", Description = "Acoustic scene classification model (DCASE-style)", ModelType = ModelType.NeuralNetwork, - FeatureCount = _options.NumMfccs, // Input feature count, not output class count + FeatureCount = inputFeatures, Complexity = 1 };src/Audio/Classification/AudioEventDetector.cs (1)
204-215: Remove unused CreateMfccExtractor method.This method is never called and appears to be dead code left over after the
_mfccExtractorfield was removed in previous commits.🔎 Proposed fix
- private MfccExtractor<T> CreateMfccExtractor() - { - return new MfccExtractor<T>(new MfccOptions - { - SampleRate = _options.SampleRate, - NumCoefficients = 13, - FftSize = _options.FftSize, - HopLength = _options.HopLength, - FMin = _options.FMin, - FMax = _options.FMax - }); - }Also remove orphaned comments at lines 136 and 174:
- // Create feature extractors - // Optimizer not used in ONNX mode but required by interface- // Create feature extractors - InitializeLayers();
🧹 Nitpick comments (5)
src/Audio/Classification/SceneClassifier.cs (1)
731-766: Optional: Remove redundant bounds checks after early return guard.The bounds checks at lines 736-739, 746-750, and 756-764 verify
t < mfccs.Shape[0]andc < mfccs.Shape[1], but these are always true because:
- Lines 708-725 provide an early return when
numFrames == 0ornumCoeffs == 0- Lines 704-705 set
numFrames = mfccs.Shape[0]andnumCoeffs = mfccs.Shape[1]- The loops at lines 731 and 734 use these exact bounds
These redundant checks were likely added defensively, but they add visual noise without providing safety. The early return guard at lines 708-725 (correctly addressing past review feedback) already ensures the loops are safe.
🔎 Simplified code (remove redundant checks)
for (int c = 0; c < numCoeffs; c++) { double sum = 0; for (int t = 0; t < numFrames; t++) { - if (t < mfccs.Shape[0] && c < mfccs.Shape[1]) - { - sum += NumOps.ToDouble(mfccs[t, c]); - } + sum += NumOps.ToDouble(mfccs[t, c]); } mfccMean[c] = sum / numFrames; double sumSq = 0; for (int t = 0; t < numFrames; t++) { - if (t < mfccs.Shape[0] && c < mfccs.Shape[1]) - { - double diff = NumOps.ToDouble(mfccs[t, c]) - mfccMean[c]; - sumSq += diff * diff; - } + double diff = NumOps.ToDouble(mfccs[t, c]) - mfccMean[c]; + sumSq += diff * diff; } mfccStd[c] = Math.Sqrt(sumSq / numFrames); // Delta (first derivative) double sumDelta = 0; for (int t = 1; t < numFrames && t < mfccs.Shape[0]; t++) + for (int t = 1; t < numFrames; t++) { - if (c < mfccs.Shape[1]) - { - double prev = NumOps.ToDouble(mfccs[t - 1, c]); - double curr = NumOps.ToDouble(mfccs[t, c]); - sumDelta += Math.Abs(curr - prev); - } + double prev = NumOps.ToDouble(mfccs[t - 1, c]); + double curr = NumOps.ToDouble(mfccs[t, c]); + sumDelta += Math.Abs(curr - prev); } mfccDelta[c] = numFrames > 1 ? sumDelta / (numFrames - 1) : 0.0; }src/Audio/Classification/AudioEventDetector.cs (2)
558-559: Consider defensive null check for LossFunction.The
Trainmethod usesLossFunction.CalculateLossandLossFunction.CalculateDerivativewithout verifying thatLossFunctionis not null. While the base class likely initializes this property, adding a defensive check would prevent potentialNullReferenceExceptionif the model is constructed in an unexpected state.🔎 Suggested approach
// Forward pass var output = Predict(input); + if (LossFunction is null) + { + throw new InvalidOperationException("LossFunction not initialized."); + } + // Compute loss and gradients var loss = LossFunction.CalculateLoss(output.ToVector(), expected.ToVector()); var gradient = LossFunction.CalculateDerivative(output.ToVector(), expected.ToVector());
829-867: Well-documented fallback, but consider explicit error.The extensive XML documentation (lines 829-846) now clearly explains when the rule-based fallback is used. However, given that this path is only reached during "incomplete initialization" (effectively an error state), consider throwing an
InvalidOperationExceptioninstead of silently falling back to heuristics, which would make debugging easier.💡 Alternative: Throw instead of silent fallback
else if (_useNativeMode) { return ClassifyWithNative(melSpec); } else { - return ClassifyWithRules(melSpec, audio); + throw new InvalidOperationException( + "Detector is in an invalid state: neither ONNX mode nor native mode is active. " + + "This indicates incomplete initialization. Please ensure the model is properly constructed."); }This change would require updating tests that rely on the rule-based fallback. If the fallback is intentionally used for scenarios beyond error recovery, the current implementation with comprehensive documentation is acceptable.
src/Audio/AudioGen/AudioGenModel.cs (2)
607-642: Layer boundary fallback still uses fraction-based heuristic.Lines 639-640 fall back to
layers.Count / 3when noTransformerDecoderLayeris detected. This heuristic may fail for custom architectures that don't follow the expected structure.Consider either:
- Requiring users to specify explicit boundary indices in the
Architectureconfiguration when using custom layers- Throwing an exception instead of falling back to a potentially incorrect guess
- Adding a warning log when the fallback is used
The smart detection (lines 617-627) is good, but the fallback may produce silent failures.
1047-1116: O(n²) tensor copy overhead remains in autoregressive loop.Despite pre-allocating
currentTokens(lines 1060-1063), lines 1074-1081 still create a newinputTokenstensor and copy data on every iteration. For long durations (e.g., 30 seconds ≈ 1500 tokens), this results in ~1.1 million copy operations.The pre-allocation avoids memory reallocation but doesn't eliminate the copy overhead. To truly achieve O(n) complexity, the framework would need to support zero-copy tensor slicing/views.
Current impact: Noticeable slowdown for durations > 10 seconds, but acceptable for typical use cases (5-10 seconds).
🔎 Potential optimization if tensor views become available
If the Tensor API supports slicing without copy:
- var inputTokens = new Tensor<T>([1, _numCodebooks, currentLength]); - for (int cb = 0; cb < _numCodebooks; cb++) - { - for (int i = 0; i < currentLength; i++) - { - inputTokens[0, cb, i] = currentTokens[0, cb, i]; - } - } + // Zero-copy slice: currentTokens[0.., 0.., 0..currentLength] + var inputTokens = currentTokens.Slice([0, 0, 0], [1, _numCodebooks, currentLength]);
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
src/Audio/AudioGen/AudioGenModel.cssrc/Audio/Classification/AudioEventDetector.cssrc/Audio/Classification/SceneClassifier.cssrc/Audio/Speaker/SpeakerEmbeddingExtractor.cssrc/Audio/Speaker/SpeakerVerifier.cssrc/Interfaces/IAudioEventDetector.cssrc/Interfaces/IGenreClassifier.cssrc/Interfaces/ISceneClassifier.cssrc/Interfaces/ISpeakerDiarizer.cssrc/ReinforcementLearning/Environments/DeterministicBanditEnvironment.cstests/AiDotNet.Tests/Audio/Classification/ClassificationTests.cs
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-12-18T08:49:25.295Z
Learnt from: ooples
Repo: ooples/AiDotNet PR: 444
File: src/Interfaces/IPruningMask.cs:1-102
Timestamp: 2025-12-18T08:49:25.295Z
Learning: In the AiDotNet repository, the project-level global using includes AiDotNet.Tensors.LinearAlgebra via AiDotNet.csproj. Therefore, Vector<T>, Matrix<T>, and Tensor<T> are available without per-file using directives. Do not flag missing using directives for these types in any C# files within this project. Apply this guideline broadly to all C# files (not just a single file) to avoid false positives. If a file uses a type from a different namespace not covered by the global using, flag as usual.
Applied to files:
src/Audio/Classification/SceneClassifier.cssrc/Audio/AudioGen/AudioGenModel.cssrc/Audio/Classification/AudioEventDetector.cs
📚 Learning: 2025-12-18T08:49:53.103Z
Learnt from: ooples
Repo: ooples/AiDotNet PR: 444
File: src/Interfaces/IPruningStrategy.cs:1-4
Timestamp: 2025-12-18T08:49:53.103Z
Learning: In this repository, global using directives are declared in AiDotNet.csproj for core namespaces (AiDotNet.Tensors.* and AiDotNet.*) and common system types. When reviewing C# files, assume these global usings are in effect; avoid adding duplicate using statements for these namespaces and for types like Vector<T>, Matrix<T>, Tensor<T>, etc. If a type is not found, verify the global usings or consider adding a file-scoped using if needed. Prefer relying on global usings to reduce boilerplate.
Applied to files:
src/Audio/Classification/SceneClassifier.cssrc/Audio/AudioGen/AudioGenModel.cssrc/Audio/Classification/AudioEventDetector.cs
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: CodeQL Analysis
- GitHub Check: Build (Windows)
🔇 Additional comments (10)
src/Audio/Classification/AudioEventDetector.cs (3)
707-756: LGTM: Comprehensive window handling.The windowing logic now properly handles edge cases including partial windows, very short audio, and padding. The 10% threshold (line 733) ensures very small tail segments are discarded while significant audio is preserved and zero-padded for consistent processing.
684-692: LGTM: MelSpectrogram properly reinitialized after deserialization.The method now correctly reinitializes
_melSpectrogramwith the deserialized options, ensuring feature extraction matches the restored configuration. This addresses the issue from previous reviews.
1140-1288: LGTM: Excellent thread-safe streaming session implementation.The
StreamingEventDetectionSessiondemonstrates proper concurrent programming practices:
- All shared state (
_buffer,_newEvents,_currentState) is protected by_lock- Events are collected inside the lock but invoked outside (lines 1183-1185, 1247-1254), preventing potential deadlocks
volatile bool _disposed(line 1150) ensures memory visibility across threads- Double-checked disposal pattern (lines 1189-1192) prevents race conditions
- All public methods properly synchronize access
This implementation addresses all thread-safety concerns from previous reviews.
src/Audio/AudioGen/AudioGenModel.cs (7)
1-61: AI summary lists methods not present in the code.The AI summary claims
CreateAsyncandFromFilesstatic factory methods are present, but these methods are not implemented in this file. This creates confusion about the actual public API surface.Additionally, past review comments reference a
FromFilesmethod with resource leak issues, but that method no longer exists in the code, making those comments obsolete.
125-131: Thread-safe random generation correctly implemented.The
_randomLockobject protects access to the shared_randomfield (see line 686-689), resolving the thread-safety concern from previous reviews.
302-417: ONNX constructor correctly handles resource cleanup and validation.The try-catch block (lines 383-416) ensures that partially-loaded ONNX models are disposed if instantiation fails, preventing resource leaks. Parameter validation (lines 336-354) is comprehensive and throws clear exceptions for invalid ranges.
1029-1045: Tokenization correctly uses ITokenizer interface.The method properly delegates to
_tokenizer.Encode()rather than implementing manual character-based tokenization, making the past review comment about digit handling obsolete.
1522-1535: IDisposable pattern correctly implemented.The disposal logic properly disposes ONNX models and avoids double-disposal of
_audioDecoder(noted in line 1530 comment, which is stored in base class at line 394).
935-955: Deserialization discards all values without restoring model state.Lines 937-954 read 17 serialized values but immediately discard them via underscore assignments. The comment states "validation done in CreateNewInstance," but
CreateNewInstance(lines 960-984) uses current instance fields (e.g.,_modelSizeat line 964), not deserialized values.Since the configuration fields are readonly, they cannot be updated during deserialization. This raises a concern: either the design intentionally separates serialization concerns (weights vs. configuration), or the deserialized values should be used to reconstruct the model properly.
Verify whether this matches the base class serialization contract and how other neural network models in the codebase handle this pattern.
524-524: Verify LanguageModelBackbone.FlanT5 availability and factory implementation.Line 524 creates a default tokenizer using
LanguageModelBackbone.FlanT5. Ensure that:
- The
LanguageModelBackboneenum andFlanT5value exist in the codebaseLanguageModelTokenizerFactory.CreateForBackboneis implemented and handles this enum value- The resulting tokenizer is compatible with AudioGen's text encoder expectations
- Add parameter validation for numCodebooks, codebookSize, and maxTextLength in AudioGenModel constructor - Fix AudioEventDetector serialization to preserve ONNX inference mode - Make _useNativeMode non-readonly to allow restoration during deserialize - Serialize/deserialize model path and mode flag for proper round-trip 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/Audio/Classification/AudioEventDetector.cs (1)
204-215: Dead code: CreateMfccExtractor is never called.The
CreateMfccExtractormethod is defined but never invoked anywhere in the class. This was likely leftover after the_mfccExtractorfield was removed in previous fixes.🔎 Proposed fix
- private MfccExtractor<T> CreateMfccExtractor() - { - return new MfccExtractor<T>(new MfccOptions - { - SampleRate = _options.SampleRate, - NumCoefficients = 13, - FftSize = _options.FftSize, - HopLength = _options.HopLength, - FMin = _options.FMin, - FMax = _options.FMax - }); - }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/Audio/AudioGen/AudioGenModel.cssrc/Audio/Classification/AudioEventDetector.cs
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-12-18T08:49:25.295Z
Learnt from: ooples
Repo: ooples/AiDotNet PR: 444
File: src/Interfaces/IPruningMask.cs:1-102
Timestamp: 2025-12-18T08:49:25.295Z
Learning: In the AiDotNet repository, the project-level global using includes AiDotNet.Tensors.LinearAlgebra via AiDotNet.csproj. Therefore, Vector<T>, Matrix<T>, and Tensor<T> are available without per-file using directives. Do not flag missing using directives for these types in any C# files within this project. Apply this guideline broadly to all C# files (not just a single file) to avoid false positives. If a file uses a type from a different namespace not covered by the global using, flag as usual.
Applied to files:
src/Audio/AudioGen/AudioGenModel.cssrc/Audio/Classification/AudioEventDetector.cs
📚 Learning: 2025-12-18T08:49:53.103Z
Learnt from: ooples
Repo: ooples/AiDotNet PR: 444
File: src/Interfaces/IPruningStrategy.cs:1-4
Timestamp: 2025-12-18T08:49:53.103Z
Learning: In this repository, global using directives are declared in AiDotNet.csproj for core namespaces (AiDotNet.Tensors.* and AiDotNet.*) and common system types. When reviewing C# files, assume these global usings are in effect; avoid adding duplicate using statements for these namespaces and for types like Vector<T>, Matrix<T>, Tensor<T>, etc. If a type is not found, verify the global usings or consider adding a file-scoped using if needed. Prefer relying on global usings to reduce boilerplate.
Applied to files:
src/Audio/AudioGen/AudioGenModel.cssrc/Audio/Classification/AudioEventDetector.cs
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Build (Windows)
- GitHub Check: CodeQL Analysis
🔇 Additional comments (14)
src/Audio/Classification/AudioEventDetector.cs (4)
267-417: LGTM!The detection API methods are well-structured and handle both synchronous and asynchronous scenarios correctly. Proper disposal checks and null reference guards are in place.
420-492: LGTM!Legacy API methods provide clean backward compatibility with proper conversion between generic and non-generic types.
494-720: LGTM!Core implementation properly handles ONNX and native modes. Serialization/deserialization correctly preserves and restores mode state, including reinitialization of
_melSpectrogramandOnnxEncoderwhen needed.
1155-1306: LGTM!The
StreamingEventDetectionSessionimplementation correctly addresses all previous thread-safety concerns. The locking strategy properly protects shared state while avoiding deadlocks by raising events outside the lock.src/Audio/AudioGen/AudioGenModel.cs (10)
16-61: Excellent documentation for accessibility.The XML documentation is comprehensive, beginner-friendly, and clearly explains the differences between TTS and AudioGen, use cases, and limitations.
123-131: Thread-safe random access properly implemented.The addition of
_randomLockand its usage in generation methods (e.g., lines 692-695) correctly addresses the thread-safety concern for concurrent access to_random.
302-417: Constructor properly validates and handles errors.Parameter validation (lines 337-354) and exception handling with cleanup (lines 410-416) are correctly implemented, ensuring robust initialization.
419-539: Native constructor validation is complete.All parameters including
numCodebooks,codebookSize, andmaxTextLengthare properly validated (lines 498-503), addressing previous concerns about divide-by-zero risks.
543-660: Layer boundary detection improved.The layer initialization now attempts to detect boundaries by identifying
TransformerDecoderLayerinstances (lines 625-633), with a fraction-based fallback. This is a significant improvement over purely fraction-based assumptions.
675-715: Generation logic is clean and thread-safe.The seed generation uses proper locking (lines 692-696), and the overall generation flow (encode text → generate codes → decode audio) is clear and correct.
1053-1225: Autoregressive generation optimized properly.The pre-allocation strategy (lines 1066-1069, 1162-1164) with view creation (lines 1079-1087, 1173-1181) eliminates the O(n²) overhead, reducing complexity from quadratic to linear.
1368-1513: Native decoder implementation is complete.The
DecodeAudioNativemethod now provides a functional EnCodec-style decoder with codebook aggregation, upsampling, interpolation, and smoothing—replacing the previous placeholder implementation.
1528-1541: Resource disposal implemented correctly.The
Disposemethod properly releases all ONNX model resources and calls the base class disposal.
163-179: Past review comment references code not in current file.A previous review flagged the
FromFilesmethod for not cleaning up on partial failure. However, this method is not present in the current version of this file—it may have been moved, removed, or refactored.
- Fix CS8604 null reference warning by using pattern matching for modelPath in deserialization - Document the ClassifyWithRules fallback branch for edge cases when ONNX model file is missing after deserialization 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/Audio/Classification/AudioEventDetector.cs (2)
204-215: Remove unused CreateMfccExtractor method.The
CreateMfccExtractormethod is never called. Additionally, the comments at lines 135 and 174 ("Create feature extractors") suggest extractors should be initialized, but no code follows these comments.🔎 Proposed fix
- private MfccExtractor<T> CreateMfccExtractor() - { - return new MfccExtractor<T>(new MfccOptions - { - SampleRate = _options.SampleRate, - NumCoefficients = 13, - FftSize = _options.FftSize, - HopLength = _options.HopLength, - FMin = _options.FMin, - FMax = _options.FMax - }); - }Also remove or complete the "Create feature extractors" comments at lines 135 and 174.
1055-1099: Consider making event merge tolerance configurable.Line 1073 uses a hardcoded 0.1-second tolerance to determine if events should be merged. This fixed value may not be appropriate for all window configurations. For example, if
TimeResolutionis 0.5 seconds, a 0.1s tolerance might be too restrictive, while for a 0.01s resolution, it might merge events that should remain separate.💡 Suggested approaches
Option 1: Add a configurable merge tolerance to
AudioEventDetectorOptions:public class AudioEventDetectorOptions { // ... existing properties ... public double MergeTolerance { get; set; } = 0.1; }Then use it in
MergeEvents:- if (next.StartTime <= currentEvent.EndTime + 0.1) + if (next.StartTime <= currentEvent.EndTime + _options.MergeTolerance)Option 2: Make it proportional to
TimeResolution:+ double mergeTolerance = TimeResolution * 0.5; // 50% of time resolution + for (int i = 1; i < sortedEvents.Count; i++) { var next = sortedEvents[i]; // Check if events overlap or are adjacent - if (next.StartTime <= currentEvent.EndTime + 0.1) + if (next.StartTime <= currentEvent.EndTime + mergeTolerance)
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/Audio/Classification/AudioEventDetector.cs
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-12-18T08:49:25.295Z
Learnt from: ooples
Repo: ooples/AiDotNet PR: 444
File: src/Interfaces/IPruningMask.cs:1-102
Timestamp: 2025-12-18T08:49:25.295Z
Learning: In the AiDotNet repository, the project-level global using includes AiDotNet.Tensors.LinearAlgebra via AiDotNet.csproj. Therefore, Vector<T>, Matrix<T>, and Tensor<T> are available without per-file using directives. Do not flag missing using directives for these types in any C# files within this project. Apply this guideline broadly to all C# files (not just a single file) to avoid false positives. If a file uses a type from a different namespace not covered by the global using, flag as usual.
Applied to files:
src/Audio/Classification/AudioEventDetector.cs
📚 Learning: 2025-12-18T08:49:53.103Z
Learnt from: ooples
Repo: ooples/AiDotNet PR: 444
File: src/Interfaces/IPruningStrategy.cs:1-4
Timestamp: 2025-12-18T08:49:53.103Z
Learning: In this repository, global using directives are declared in AiDotNet.csproj for core namespaces (AiDotNet.Tensors.* and AiDotNet.*) and common system types. When reviewing C# files, assume these global usings are in effect; avoid adding duplicate using statements for these namespaces and for types like Vector<T>, Matrix<T>, Tensor<T>, etc. If a type is not found, verify the global usings or consider adding a file-scoped using if needed. Prefer relying on global usings to reduce boilerplate.
Applied to files:
src/Audio/Classification/AudioEventDetector.cs
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: CodeQL Analysis
- GitHub Check: Build (Windows)
Update XML documentation to accurately describe when the rule-based fallback is invoked: - Remove incorrect claim about legacy constructor usage - Clarify that this path is reached after deserialization when ONNX model file is missing - Add note explaining why normal constructor paths don't reach this 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/Audio/Classification/AudioEventDetector.cs (1)
204-215: Remove unused CreateMfccExtractor method.The
CreateMfccExtractormethod is never called anywhere in the class. All feature extraction is handled by_melSpectrogram(initialized in constructors at lines 120-127 and 165-172). This method appears to be leftover from an earlier design.🔎 Proposed fix
- private MfccExtractor<T> CreateMfccExtractor() - { - return new MfccExtractor<T>(new MfccOptions - { - SampleRate = _options.SampleRate, - NumCoefficients = 13, - FftSize = _options.FftSize, - HopLength = _options.HopLength, - FMin = _options.FMin, - FMax = _options.FMax - }); - }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/Audio/Classification/AudioEventDetector.cs
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-12-18T08:49:25.295Z
Learnt from: ooples
Repo: ooples/AiDotNet PR: 444
File: src/Interfaces/IPruningMask.cs:1-102
Timestamp: 2025-12-18T08:49:25.295Z
Learning: In the AiDotNet repository, the project-level global using includes AiDotNet.Tensors.LinearAlgebra via AiDotNet.csproj. Therefore, Vector<T>, Matrix<T>, and Tensor<T> are available without per-file using directives. Do not flag missing using directives for these types in any C# files within this project. Apply this guideline broadly to all C# files (not just a single file) to avoid false positives. If a file uses a type from a different namespace not covered by the global using, flag as usual.
Applied to files:
src/Audio/Classification/AudioEventDetector.cs
📚 Learning: 2025-12-18T08:49:53.103Z
Learnt from: ooples
Repo: ooples/AiDotNet PR: 444
File: src/Interfaces/IPruningStrategy.cs:1-4
Timestamp: 2025-12-18T08:49:53.103Z
Learning: In this repository, global using directives are declared in AiDotNet.csproj for core namespaces (AiDotNet.Tensors.* and AiDotNet.*) and common system types. When reviewing C# files, assume these global usings are in effect; avoid adding duplicate using statements for these namespaces and for types like Vector<T>, Matrix<T>, Tensor<T>, etc. If a type is not found, verify the global usings or consider adding a file-scoped using if needed. Prefer relying on global usings to reduce boilerplate.
Applied to files:
src/Audio/Classification/AudioEventDetector.cs
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build (Windows)
|
Summary
This PR implements a comprehensive Audio AI stack covering multiple GitHub issues:
ONNX Infrastructure (#280)
OnnxModel<T>wrapper with InferenceSession and multiple execution providersOnnxModelOptionsfor CPU, CUDA, TensorRT, DirectML configurationOnnxTensorConverterfor AiDotNet Tensor <-> ONNX tensor conversionOnnxModelDownloaderfor HuggingFace model downloadsOnnxExporterfor exporting neural networks to ONNX formatAudio Models (#269, #270, #271)
WhisperModel<T>- Speech-to-text with encoder-decoder architectureTtsModel<T>- Text-to-speech with FastSpeech2 + HiFi-GAN pipelineAudioGenModel<T>- Text-conditioned audio generationAudio Features (#396)
MfccExtractor<T>- Mel-frequency cepstral coefficientsChromaExtractor<T>- Chromagram (12 pitch classes)SpectralFeatureExtractor<T>- Centroid, rolloff, flux, bandwidthConstantQTransform<T>- CQT for music analysisAudio Analysis (#396)
MusicSourceSeparator<T>- HPSS-based stem separation (vocals, drums, bass, other)GenreClassifier<T>- Rule-based and model-based genre classificationAudioEventDetector<T>- AudioSet-style event detectionSceneClassifier<T>- DCASE-style acoustic scene classificationSoundLocalizer<T>- GCC-PHAT, MUSIC, SRP-PHAT localization algorithmsAudio Fingerprinting (#396)
ChromaprintFingerprinter<T>- AcoustID-compatible fingerprintingSpectrogramFingerprinter<T>- Peak-based (Shazam-style)Speaker Recognition (#396)
SpeakerEmbeddingExtractor<T>- d-vector extractionSpeakerVerifier<T>- Cosine similarity verificationSpeakerDiarizer<T>- Clustering-based diarizationMusic Analysis (#396)
BeatTracker<T>- Tempo and beat detectionChordRecognizer<T>- Chord recognition via chroma templatesKeyDetector<T>- Musical key detectionTest plan
Closes
Closes #280, #269, #270, #271, #396
🤖 Generated with Claude Code