diff --git a/CONVERT_MODULARIZATION_PLAN.md b/CONVERT_MODULARIZATION_PLAN.md new file mode 100644 index 0000000..9a23c56 --- /dev/null +++ b/CONVERT_MODULARIZATION_PLAN.md @@ -0,0 +1,109 @@ +# Convert Panel Modularization Plan (Dev24/25) + +## 🎯 Goal +Move Advanced Convert UI logic out of main.go into modular UI components, keeping main.go as glue only. + +## 📁 File Structure + +``` +internal/ui/ +├── convert_advanced.go # Advanced panel UI builder +├── convert_state.go # State manager + callbacks +├── convert_simple.go # Simple panel UI builder (future) +└── convert_types.go # Shared types and constants + +main.go (cleanup) +├── Keep encoding/FFmpeg logic in existing helpers +├── Import internal/ui package only +└── Replace UI blocks with module calls +``` + +## 🔧 What to Extract from main.go + +### 1. UI Builders (buildConvertView -> convert_advanced.go) +- Advanced panel dropdowns, sliders, toggles +- Layout containers and responsive sizing +- Quality presets and format selection +- Hardware acceleration controls +- Two-pass encoding interface +- Progress preview and command display + +### 2. State Management (convertUIState -> convert_state.go) +```go +type ConvertState struct { + // Current settings + Format formatOption + Quality string + Preset string + TwoPass bool + HardwareAccel bool + + // UI bindings + FormatList *widget.Select + QualitySelect *widget.Select + // ... etc +} + +type ConvertUIBindings struct { + // Controls accessible to main.go + StartConversion func() + StopConversion func() + ShowPreview func() + // ... etc +} +``` + +### 3. Callback Functions (applyQuality, setQuality, updateEncodingControls -> convert_state.go) +- State change management +- Validation and sanitization +- Settings persistence +- Progress update handling + +## 🔄 Integration Pattern + +### main.go Changes: +```go +import "git.leaktechnologies.dev/stu/VideoTools/internal/ui" + +// Replace giant UI block with: +if useAdvanced { + panel := ui.BuildConvertAdvancedPanel(state, src) + mainContent := container.NewVBox(panel) +} else { + panel := ui.BuildConvertSimplePanel(state, src) + mainContent := container.NewVBox(panel) +} +``` + +### Module Interface: +```go +func BuildConvertAdvancedPanel(state *appState, src *videoSource) (fyne.CanvasObject, *ConvertUIBindings) +func BuildConvertSimplePanel(state *appState, src *videoSource) (fyne.CanvasObject, *ConvertUIBindings) +func InitConvertState(state *appState, src *videoSource) *ConvertState +``` + +## 🎨 Benefits + +1. **Maintainable**: UI logic separated from core logic +2. **Testable**: UI components can be unit tested independently +3. **Reusability**: Simple/Advanced panels reused in other modules +4. **Clean Code**: main.go becomes readable and focused +5. **Future-Proof**: Easy to add new UI features without bloating main.go + +## 📋 Implementation Order + +1. **Phase 1**: Create convert_types.go with shared types +2. **Phase 2**: Extract state management into convert_state.go +3. **Phase 3**: Build convert_advanced.go with UI logic +4. **Phase 4**: Update main.go to use new modules +5. **Phase 5**: Test and iterate on modular interface + +## 🎯 Success Metrics + +✅ main.go reduced by 2000+ lines +✅ UI logic properly separated and testable +✅ Clean module boundaries with no circular deps +✅ Maintain existing functionality and user experience +✅ Foundation for future UI improvements in other modules + +This modularization will make the codebase much more maintainable and prepare us for advanced features in dev25. \ No newline at end of file diff --git a/TESTING_CHECKLIST.md b/TESTING_CHECKLIST.md new file mode 100644 index 0000000..5d4be3a --- /dev/null +++ b/TESTING_CHECKLIST.md @@ -0,0 +1,77 @@ +# UnifiedPlayer Testing Checklist + +## 🎬 Video Functionality Testing +- [ ] Basic video playback starts without crashing +- [ ] Video frames display correctly in Fyne canvas +- [ ] Frame rate matches source video (30fps, 24fps, 60fps) +- [ ] Resolution scaling works properly +- [ ] No memory leaks during video playback +- [ ] Clean video-only playback (no audio stream files) + +## 🔊 Audio Functionality Testing +- [ ] Audio plays through system speakers +- [ ] Audio volume controls work correctly (0-100%) +- [ ] Mute/unmute functionality works +- [ ] A/V synchronization stays in sync (no drift) +- [ ] Audio works with different sample rates +- [ ] Audio stops cleanly on Stop()/Pause() + +## ⚡ Play/Pause/Seek Controls +- [ ] Play() starts both video and audio immediately +- [ ] Pause() freezes both video and audio frames +- [ ] Seek() jumps to correct timestamp instantly +- [ ] Frame stepping works frame-by-frame +- [ ] Resume after pause continues from paused position + +## 🛠️ Error Handling & Edge Cases +- [ ] Missing video file shows user-friendly error +- [ ] Corrupted video file handles gracefully +- [ ] Unsupported format shows clear error message +- [ ] Audio-only files handled without crashes +- [ ] Resource cleanup on player.Close() + +## 📊 Performance & Resource Usage +- [ ] CPU usage is reasonable (<50% on modern hardware) +- [ ] Memory usage stays stable (no growing leaks) +- [ ] Smooth playback without stuttering +- [ ] Fast seeking without rebuffering delays +- [ ] Frame extraction is performant at target resolution + +## 📋 Cross-Platform Testing +- [ ] Works on different video codecs (H.264, H.265, VP9) +- [ ] Handles different container formats (MP4, MKV, AVI) +- [ ] Works with various resolutions (720p, 1080p, 4K) +- [ ] Audio works with stereo/mono sources +- [ ] No platform-specific crashes (Linux/Windows/Mac) + +## 🔧 Technical Validation +- [ ] FFmpeg process starts with correct args +- [ ] Pipe communication works (video + audio) +- [ ] RGB24 → RGBA conversion is correct +- [ ] oto audio context initializes successfully +- [ ] Frame display loop runs at correct timing +- [ ] A/V sync timing calculations are accurate + +## 🎯 Key Success Metrics +- [ ] Video plays without crashes +- [ ] Audio is audible and in sync +- [ ] Seeking is frame-accurate and responsive +- [ ] Frame stepping works perfectly +- [ ] Resource usage is optimal +- [ ] No memory leaks or resource issues + +## 📝 Testing Notes +- **File**: [Test video file used] +- **Duration**: [Video length tested] +- **Resolution**: [Input and output resolutions] +- **Issues Found**: [List any problems discovered] +- **Performance**: [CPU/Memory usage observations] +- **A/V Sync**: [Any sync issues noted] +- **Seek Accuracy**: [Seek performance observations] + +## 🔍 Debug Information +- **FFmpeg Args**: [Command line arguments used] +- **Audio Context**: [Sample rate, channels, format] +- **Buffer Sizes**: [Video frame and audio buffer sizes] +- **Error Logs**: [Any error messages during testing] +- **Pipe Status**: [Video/audio pipe communication status] \ No newline at end of file diff --git a/internal/logging/logging.go b/internal/logging/logging.go index 4651af8..011d332 100644 --- a/internal/logging/logging.go +++ b/internal/logging/logging.go @@ -4,7 +4,7 @@ import ( "fmt" "log" "os" - "runtime/debug" + "runtime" "time" ) @@ -33,9 +33,17 @@ const ( // Init initializes the logging system func Init() { + // Create logs directory if it doesn't exist + logsDir := "logs" + if err := os.MkdirAll(logsDir, 0755); err != nil { + fmt.Fprintf(os.Stderr, "videotools: cannot create logs directory: %v\n", err) + return + } + + // Use environment variable or default filePath = os.Getenv("VIDEOTOOLS_LOG_FILE") if filePath == "" { - filePath = "videotools.log" + filePath = "logs/videotools.log" } f, err := os.OpenFile(filePath, os.O_CREATE|os.O_APPEND|os.O_WRONLY, 0o644) if err != nil { @@ -65,6 +73,27 @@ func Debug(cat Category, format string, args ...interface{}) { if file != nil { fmt.Fprintf(file, "%s %s\n", timestamp, msg) } + logger.Printf("%s %s", timestamp, msg) +} + +// Crash logs a critical error with stack trace for debugging crashes +func Crash(cat Category, format string, args ...interface{}) { + msg := fmt.Sprintf("%s CRASH: %s", cat, fmt.Sprintf(format, args...)) + timestamp := time.Now().Format(time.RFC3339Nano) + + // Log to main log file + if file != nil { + fmt.Fprintf(file, "%s %s\n", timestamp, msg) + } + logger.Printf("%s %s", timestamp, msg) + + // Also log to dedicated crash log + if crashFile, err := os.OpenFile(GetCrashLogPath(), os.O_CREATE|os.O_APPEND|os.O_WRONLY, 0644); err == nil { + fmt.Fprintf(crashFile, "%s %s\n", timestamp, msg) + fmt.Fprintf(crashFile, "Stack trace:\n%s\n", timestamp, getStackTrace()) + crashFile.Sync() + } +} history = append(history, fmt.Sprintf("%s %s", timestamp, msg)) if len(history) > historyMax { history = history[len(history)-historyMax:] @@ -84,6 +113,13 @@ func History() []string { return history } +// getStackTrace returns current goroutine stack trace +func getStackTrace() string { + buf := make([]byte, 4096) + n := runtime.Stack(buf, false) + return string(buf[:n]) +} + // Error logs an error message with a category (always logged, even when debug is off) func Error(cat Category, format string, args ...interface{}) { msg := fmt.Sprintf("%s ERROR: %s", cat, fmt.Sprintf(format, args...)) @@ -98,6 +134,43 @@ func Error(cat Category, format string, args ...interface{}) { logger.Printf("%s %s", timestamp, msg) } +// Crash logs a critical error with stack trace for debugging crashes +func Crash(cat Category, format string, args ...interface{}) { + msg := fmt.Sprintf("%s CRASH: %s", cat, fmt.Sprintf(format, args...)) + timestamp := time.Now().Format(time.RFC3339Nano) + + // Log to main log file + if file != nil { + fmt.Fprintf(file, "%s %s\n", timestamp, msg) + } + logger.Printf("%s %s", timestamp, msg) + + // Also log to dedicated crash log + if crashFile, err := os.OpenFile(GetCrashLogPath(), os.O_CREATE|os.O_APPEND|os.O_WRONLY, 0644); err == nil { + fmt.Fprintf(crashFile, "%s %s\n", timestamp, msg) + fmt.Fprintf(crashFile, "Stack trace:\n%s\n", timestamp, getStackTrace()) + crashFile.Sync() + } +} + +// Fatal logs a fatal error and exits (always logged, even when debug is off) +func Fatal(cat Category, format string, args ...interface{}) { + msg := fmt.Sprintf("%s FATAL: %s", cat, fmt.Sprintf(format, args...)) + timestamp := time.Now().Format(time.RFC3339Nano) + if file != nil { + fmt.Fprintf(file, "%s %s\n", timestamp, msg) + file.Sync() + } + logger.Printf("%s %s", timestamp, msg) + os.Exit(1) +} + history = append(history, fmt.Sprintf("%s %s", timestamp, msg)) + if len(history) > historyMax { + history = history[len(history)-historyMax:] + } + logger.Printf("%s %s", timestamp, msg) +} + // Fatal logs a fatal error and exits (always logged) func Fatal(cat Category, format string, args ...interface{}) { msg := fmt.Sprintf("%s FATAL: %s", cat, fmt.Sprintf(format, args...)) diff --git a/internal/player/unified_ffmpeg_player.go b/internal/player/unified_ffmpeg_player.go index f3ff241..ae5928b 100644 --- a/internal/player/unified_ffmpeg_player.go +++ b/internal/player/unified_ffmpeg_player.go @@ -110,9 +110,22 @@ func (p *UnifiedPlayer) Load(path string, offset time.Duration) error { p.mu.Lock() defer p.mu.Unlock() + // Special handling for our test file + if strings.Contains(path, "bbb_sunflower_2160p_60fps_normal.mp4") { + logging.Debug(logging.CatPlayer, "Loading test video: Big Buck Bunny (%s)", path) + } + p.currentPath = path p.state = StateLoading + // Add panic recovery for crash safety + defer func() { + if r := recover(); r != nil { + logging.Crash(logging.CatPlayer, "Panic in Load(): %v", r) + return fmt.Errorf("panic during video loading: %v", r) + } + }() + // Create pipes for FFmpeg communication p.videoPipeReader, p.videoPipeWriter = io.Pipe() p.audioPipeReader, p.audioPipeWriter = io.Pipe() @@ -146,7 +159,8 @@ func (p *UnifiedPlayer) Load(path string, offset time.Duration) error { // Initialize audio context for playback sampleRate := 48000 channels := 2 - + bytesPerSample := 2 // 16-bit = 2 bytes + ctx, ready, err := oto.NewContext(&oto.NewContextOptions{ SampleRate: sampleRate, ChannelCount: channels, @@ -160,11 +174,9 @@ func (p *UnifiedPlayer) Load(path string, offset time.Duration) error { if ready != nil { <-ready } - + p.audioContext = ctx - - // Initialize audio buffer - p.audioBuffer = make([]byte, 0, 0) // Will grow as needed + logging.Info(logging.CatPlayer, "Audio context initialized successfully") // Start FFmpeg process for unified A/V output err = p.startVideoProcess() @@ -177,6 +189,7 @@ func (p *UnifiedPlayer) Load(path string, offset time.Duration) error { return nil } +} // SeekToTime seeks to a specific time without restarting processes func (p *UnifiedPlayer) SeekToTime(offset time.Duration) error { @@ -468,6 +481,13 @@ func (p *UnifiedPlayer) Play() error { p.mu.Lock() defer p.mu.Unlock() + // Add panic recovery for crash safety + defer func() { + if r := recover(); r != nil { + logging.Crash(logging.CatPlayer, "Panic in Play(): %v", r) + } + }() + if p.state == StateStopped { // Need to load first return fmt.Errorf("no video loaded") @@ -476,9 +496,9 @@ func (p *UnifiedPlayer) Play() error { p.paused = false p.state = StatePlaying p.syncClock = time.Now() - + logging.Debug(logging.CatPlayer, "UnifiedPlayer: Play() called, state=%v", p.state) - + if p.stateCallback != nil { p.stateCallback(p.state) } @@ -611,9 +631,60 @@ func (p *UnifiedPlayer) startVideoProcess() error { // readAudioStream reads and processes audio from the audio pipe func (p *UnifiedPlayer) readAudioStream() { - if p.audioContext == nil || p.audioPipeReader == nil { - return + // Add panic recovery for crash safety + defer func() { + if r := recover(); r != nil { + logging.Crash(logging.CatPlayer, "Panic in readAudioStream(): %v", r) + return + } + }() + + buffer := make([]byte, 4096) // 85ms chunks + + for { + select { + case <-p.ctx.Done(): + logging.Debug(logging.CatPlayer, "Audio reading goroutine stopped") + return + + default: + // Read from audio pipe + n, err := p.audioPipeReader.Read(buffer) + if err != nil && err.Error() != "EOF" { + logging.Error(logging.CatPlayer, "Audio read error: %v", err) + continue + } + + if n == 0 { + continue + } + + // Initialize audio player if needed + if p.audioPlayer == nil && p.audioContext != nil { + player, err := p.audioContext.NewPlayer(p.audioPipeReader) + if err != nil { + logging.Error(logging.CatPlayer, "Failed to create audio player: %v", err) + return + } + p.audioPlayer = player + logging.Info(logging.CatPlayer, "Audio player created successfully") + } + + // Write audio data to player buffer + if p.audioPlayer != nil { + p.audioPlayer.Write(buffer[:n]) + } + + // Buffer for sync monitoring (keep small to avoid memory issues) + if len(p.audioBuffer) > 32768 { // Max 1 second at 48kHz + p.audioBuffer = p.audioBuffer[len(p.audioBuffer)-16384:] // Keep half + } + + // Simple audio sync timing + p.updateAVSync() + } } +} p.mu.Lock() if p.audioPlayer == nil {