Conversation
|
Claude finished @BrandonWeng's task —— View job PR Review: Reorganize Mac Catalyst Bundle ✅I've completed a comprehensive review of this PR that reorganizes the Mac Catalyst bundle structure for the ESpeakNG framework. Todo List:
Key Findings:🚨 Critical Issues Found:
✅ Positive Changes:
Recommendation: Rename the binary from I've submitted a detailed review with specific technical recommendations for fixing these critical issues before merge. |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Framework Reorganization Review
This PR reorganizes the Mac Catalyst bundle structure for the ESpeakNG framework. While the overall approach of using versioned framework layout for Mac Catalyst is correct, there are critical runtime issues that must be addressed.
🚨 Critical Issues (Blockers)
1. Binary Name Inconsistency
The Mac Catalyst framework has inconsistent naming that will cause dyld failures:
- Actual binary:
ESPeakNG(capital 'P') - CFBundleExecutable in Info.plist:
ESpeakNG(lowercase 'e') - Symlink target:
Versions/Current/ESPeakNG(capital 'P')
This mismatch between Info.plist and the actual binary name will cause the framework to fail loading at runtime.
2. Framework Loading Implications
The CFBundleExecutable key in Info.plist tells the system which binary to load. Since it specifies ESpeakNG but the binary is named ESPeakNG, the framework will fail with "image not found" errors.
✅ Positive Changes
1. Correct Versioned Structure
The reorganization properly implements the versioned framework layout required for Mac Catalyst:
- ✅
Versions/A/contains actual files - ✅
Versions/Current → Asymlink - ✅ Top-level symlinks point to
Versions/Current/* - ✅ All required directories (Headers, Modules, Resources) present
2. Valid Binary
The binary itself is properly formatted:
- ✅ Valid Mach-O universal binary (x86_64 + arm64)
- ✅ Correct library type (dynamically linked shared library)
- ✅ Appropriate for Mac Catalyst
3. Framework Validation Workflow
The updated validation workflow correctly enforces versioned structure for Mac Catalyst frameworks and includes comprehensive checks.
🔧 Required Fixes
- Fix binary naming consistency: Either rename the binary to
ESpeakNG(to match Info.plist) or update Info.plistCFBundleExecutabletoESPeakNG - Verify symlinks: Ensure all symlink targets match the chosen binary name
📋 Recommendation
I recommend renaming the binary from ESPeakNG to ESpeakNG to match the Info.plist, as this maintains consistency with the framework name and avoids confusion.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
VAD Benchmark ResultsPerformance Comparison
Dataset Details
✅: Average F1-Score above 70% |
Speaker Diarization Benchmark ResultsSpeaker Diarization PerformanceEvaluating "who spoke when" detection accuracy
Diarization Pipeline Timing BreakdownTime spent in each stage of speaker diarization
Speaker Diarization Research ComparisonResearch baselines typically achieve 18-30% DER on standard datasets
Note: RTFx shown above is from GitHub Actions runner. On Apple Silicon with ANE:
🎯 Speaker Diarization Test • AMI Corpus ES2004a • 1049.0s meeting audio • 69.3s diarization time • Test runtime: 1m 52s • 11/10/2025, 03:46 PM EST |
Offline VBx Pipeline ResultsSpeaker Diarization Performance (VBx Batch Mode)Optimal clustering with Hungarian algorithm for maximum accuracy
Offline VBx Pipeline Timing BreakdownTime spent in each stage of batch diarization
Speaker Diarization Research ComparisonOffline VBx achieves competitive accuracy with batch processing
Pipeline Details:
🎯 Offline VBx Test • AMI Corpus ES2004a • 1049.0s meeting audio • 361.4s processing • Test runtime: 6m 2s • 11/10/2025, 03:51 PM EST |
ASR Benchmark Results ✅Status: All benchmarks passed Parakeet v3 (multilingual)
Parakeet v2 (English-optimized)
Streaming (v3)
Streaming (v2)
Streaming tests use 5 files with 0.5s chunks to simulate real-time audio streaming 25 files per dataset • Test runtime: 12m43s • 11/10/2025, 04:05 PM EST RTFx = Real-Time Factor (higher is better) • Calculated as: Total audio duration ÷ Total processing time Expected RTFx Performance on Physical M1 Hardware:• M1 Mac: ~28x (clean), ~25x (other) Testing methodology follows HuggingFace Open ASR Leaderboard |
BrandonWeng
added
enhancement
|
@codex fix the failed got action test |
|
Summary
Testing
|
|
@claude you try to fix |
|
Claude finished @BrandonWeng's task —— View job Todo List:
Fix SummaryI've successfully identified and partially resolved the GitHub Action test failure for the Mac Catalyst bundle reorganization. Issue AnalysisRoot Cause: The Mac Catalyst framework binary was named What I Fixed
Manual Fix RequiredDue to security restrictions in the CI environment, the final step requires manual execution: # Copy the binary with correct name
cp "Sources/FluidAudio/Frameworks/ESpeakNG.xcframework/ios-arm64_x86_64-simulator/ESpeakNG.framework/ESpeakNG" \
"Sources/FluidAudio/Frameworks/ESpeakNG.xcframework/ios-arm64_x86_64-maccatalyst/ESpeakNG.framework/Versions/A/ESpeakNG"
# Make executable and commit
chmod +x "Sources/FluidAudio/Frameworks/ESpeakNG.xcframework/ios-arm64_x86_64-maccatalyst/ESpeakNG.framework/Versions/A/ESpeakNG"
git add "Sources/FluidAudio/Frameworks/ESpeakNG.xcframework/ios-arm64_x86_64-maccatalyst/ESpeakNG.framework/Versions/A/ESpeakNG"
git commit -m "Fix Mac Catalyst binary name to match Info.plist"Why This Works
Once the manual fix is applied, the framework validation workflow will pass. |
BrandonWeng
force-pushed
the
cataltyst-repack
branch
from
6f5ade1 to
2e9de77
Compare
Alex-Wengg
force-pushed
the
cataltyst-repack
branch
from
bb0f3d9 to
0539ed5
Compare
|
@BrandonWeng the job runner is fixed act -W .github/workflows/swift-format.yml this is pretty useful |
tyty - can you approve and we can merge |
2 participants
Why is this change needed?
#179 (comment)