refactor: Clean up logging, error handling, and AVFoundation helpers#11
Open
refactor: Clean up logging, error handling, and AVFoundation helpers#11
Conversation
Replace 21 print() call sites with a typed os.Logger wrapper (Log.analysis,
Log.media, Log.parsing, Log.server, Log.ui, Log.cli) so log output flows
through the system unified logging facility instead of stdout.
Drop 9 DispatchQueue.main.async usages in favor of Task { @mainactor in } and
Task.detached, aligning with AGENTS.md's Swift Concurrency mandate.
Eliminate force unwraps in production code paths: replace .first! / .last!
sequences with guard-let bindings on first/last; resolve TimelineView's
double-evaluated nil-check pattern; add Application Support fallbacks in
JobQueue/ServerManager; convert HTTPField.Name(...)! and .data(using:)!
to safe forms; surface previously-silent JSON load errors in JobHistory and
ServerManager.
Add FramePeek/Utils/Media/AVAssetLoader.swift with typed helpers (durationSeconds, firstTrack, tracks, nominalFrameRate, estimatedDataRate, formatDescriptions, firstFormatDescription) and route the highest-frequency asset.load(.X) / loadTracks(withMediaType:) call sites through it. Replaces ~17 instances of (try? await ...) ?? default scattered across analyzers, extractors, and info loaders, removing one of the more painful sources of duplication and giving us a single seam where AVFoundation lookups can later be mocked or instrumented.
Per AGENTS.md, .help and .accessibilityLabel modifiers need explicit String(localized:) since they don't get SwiftUI's auto-localization that Text(...) gets. The five modifier-strings that were missing this wrap are now covered. Sweeping the rest of the codebase requires first adding a Strings catalog (.xcstrings); that infrastructure work is out of scope for this pass.
Add LRUCache<Key, Value> to CacheManager.swift, then collapse the three properties (gopFrameDetailsCache dictionary, gopCacheAccessOrder array, gopCacheMaxSize constant) plus the cache/get/clear functions on MediaInspectorViewModel down to a single LRUCache<UUID, [FrameInfo]>. Eliminates the duplicated bookkeeping between dictionary and access-order array, and gives waveform/sync caches the same primitive to use later if they want in-memory layers.
The analyzer files already compile into FramePeekCore (via the 'membershipExceptions' overrides on project.pbxproj's synced FramePeek folder), and the CLI consumes them through the framework. Update the docstring to describe how the build is wired today instead of instructing the next reader to do the migration that has already shipped.
Replace the placeholder FramePeekCoreTests target with four real suites covering: CSVExporter (header rows, empty data, file-info preamble, roundtrip), JSONExporter (round-trip decode, pretty-print sorted keys, multi-file wrapper), formatting utilities (formatDuration, timecode, channel layout, aspect ratio matching with PAR, resolution category, gcd, isVerticalResolution), and the new LRUCache (insertion, eviction, get-promotes-MRU, re-insert, clear, keys). 30 tests, all passing. Establishes the test foundation that future analyzer/engine tests can build on.
oscnord
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Branch-scoped cleanup pass tackling 8 of the 10 items from the codebase audit. Focused on the high-value, low-risk items; the two architecturally invasive items (god-object ViewModel split, long-view decomposition) are flagged for dedicated follow-up PRs.
459145aos.Logger(21 prints removed); GCD → Swift Concurrency (9 sites)459145aappSupportfallback; logged the previously-silent JSON load errors459231eAVAssetLoaderconsolidating ~17 duplicated AVFoundation lookup patterns1814a0dString(localized:)ca58a6fLRUCache<Key, Value>replacing the hand-rolled GOP cache plumbing on the ViewModel33d053bAnalysisEnginemigration TODO removed; docstring now describes the actual build wiring64f98c4CSVExporter,JSONExporter, formatting utils, andLRUCacheDeferred for follow-up PRs
.font(.system(size:weight:design:))calls — best handled together with P8.MediaInspectorViewModelintoAnalysisCoordinator+SettingsStore+TabContext— multi-day refactor across 11 files; should be phased with UI smoke-tests at each phase.VideoPlayerView(1,220),TimelineView(1,061),GOPHeatmapView(969) — each warrants its own PR with screenshot/UI verification.Test plan
xcodebuild -project FramePeek/FramePeek.xcodeproj -scheme FramePeek build(passes)xcodebuild -project FramePeek/FramePeek.xcodeproj -scheme FramePeekCLI build(passes)xcodebuild -project FramePeek/FramePeek.xcodeproj -scheme FramePeek test -only-testing:FramePeekCoreTests(30/30 pass)framepeek-cli analyze <file> --jsonagainst MP4, MPEG-TS, fragmented MP4/healthresponds