feat: Add crash-safe logging and panic recovery
- Create organized logging system with subfolders - Add dedicated crash log (logs/crashes.log) - Add specific log paths for different components - Implement panic recovery with stack traces - Add crash-safe logging functions - Update UnifiedPlayer with better error handling and recovery - Special handling for test video file - Add comprehensive testing checklist for Phase A This makes crashes much easier to diagnose and debug when testing the UnifiedPlayer implementation. Files: - internal/logging/logging.go (enhanced) - internal/player/unified_ffmpeg_player.go (crash-safe) - TESTING_CHECKLIST.md (comprehensive checklist) - CONVERSION_MODULARIZATION_PLAN.md (dev25 preparation)
This commit is contained in:
parent
91c6ab73c7
commit
3c6e57d0b4
109
CONVERT_MODULARIZATION_PLAN.md
Normal file
109
CONVERT_MODULARIZATION_PLAN.md
Normal file
|
|
@ -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.
|
||||
77
TESTING_CHECKLIST.md
Normal file
77
TESTING_CHECKLIST.md
Normal file
|
|
@ -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]
|
||||
|
|
@ -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...))
|
||||
|
|
|
|||
|
|
@ -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 {
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user