From 706cd22b057943b074a61d87d7cbc767c780551e Mon Sep 17 00:00:00 2001 From: Esa Kataja Date: Sat, 16 May 2026 20:31:27 +0300 Subject: [PATCH] fix: isolate per-job intermediates in paths.work and harden failure cleanup MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds a per-job scratch directory under the new `paths.work` config (default `./work`) so audio..wav, audio..opus and output.mkv no longer live beside the user's sources in paths.input. The work subdir is named after the input basename and unconditionally removed when processFile returns (success or failure), which kills bug #4 (intermediates leaking into the user-owned input folder; output.mkv getting re-picked by the 15-second watcher tick on rename failure; fixed-name collision risk for any future concurrency). Tightens the failure paths in main.processFile too (bug #25): - mover.MoveToFailed return values are now surfaced via a new failToFailed helper. - The helper os.Stats the source first; missing -> log and skip instead of the previous silent no-op when MoveToFailed was called on output.mkv before it existed. - On rename-output failure, the source is now routed to paths.failed (it was previously left in paths.input, causing an infinite re-encode loop on the next watcher tick). The old MoveToFailed on the work-dir output is dropped — the deferred RemoveAll covers it. Mechanical changes: - PathsConfig gains `Work string \`yaml:"work"\`` with default ./work, included in EnsureDirs. - Encoder.Transcode signature now takes workDir; extractAudio, encodeOpus and encodeVideo all write into workDir. The internal cleanupWavs/cleanupOpus defers are gone (RemoveAll in main is the one cleanup path). - MANUAL.md updated: example config, field reference, §6 pipeline step wording, §9 failure handling description, §11 runtime directories block. --- MANUAL.md | 32 +++++++++++++++++-------- cmd/videnc/main.go | 47 ++++++++++++++++++++++++++++++------- internal/config/config.go | 5 +++- internal/encoder/encoder.go | 42 ++++++++++----------------------- pkg/types/types.go | 1 + 5 files changed, 79 insertions(+), 48 deletions(-) diff --git a/MANUAL.md b/MANUAL.md index ce3178b..e3de395 100644 --- a/MANUAL.md +++ b/MANUAL.md @@ -60,6 +60,7 @@ paths: output: "./output" originals: "./originals" failed: "./failed" + work: "./work" ``` ### Field reference @@ -78,9 +79,10 @@ paths: | `paths.input` | Folder polled for new `.mkv` files | `./input` | | `paths.output` | Destination for finished encodes | `./output` | | `paths.originals` | Where source files are moved on success (unless `-d`) | `./originals` | -| `paths.failed` | Where source + partial output go on failure | `./failed` | +| `paths.failed` | Where source files go on failure | `./failed` | +| `paths.work` | Scratch directory for per-job intermediates (wav/opus/output.mkv); deleted after every job | `./work` | -All four directories are created on startup if they don't exist. +All five directories are created on startup if they don't exist. --- @@ -166,7 +168,7 @@ If no token is present, the program falls back to guessing from pixel count: `wi ## 6. The processing pipeline -For every `.mkv` found in `paths.input`, the program runs these steps in order. Any error sends the source file (and the partial `output.mkv`, if any) to `paths.failed`. +For every `.mkv` found in `paths.input`, the program runs these steps in order. A per-job scratch subdirectory under `paths.work` (named after the input base name without `.mkv`) holds all intermediates, and is deleted unconditionally at the end of the job. Any error sends the source file to `paths.failed`; the work subdirectory is wiped regardless of outcome. 1. **Parse filename** → determines whether this is a movie or series, and what IDs to use. 2. **Probe video** with `ffprobe`: @@ -179,8 +181,8 @@ For every `.mkv` found in `paths.input`, the program runs these steps in order. - Otherwise, fall back to pixel count: `width × height < 600,000` → DVD, else Blu-ray. The chosen profile selects which `encoding..crf` / `encoding..preset` pair from the config to use, and is written into the `ORIGINAL_MEDIA_TYPE` metadata tag. 5. **Fetch metadata** from OMDb or TVmaze depending on the parsed filename. Failures here are logged but do not abort the encode — the file is just encoded without metadata. -6. **Extract audio** — one PCM wav per source audio stream, written to the input directory as `audio.0.wav`, `audio.1.wav`, … in source order (PCM s16le, 48 kHz). Errors out if the source has no audio streams. -7. **Encode audio** — each wav is converted with `opusenc --bitrate 128k` to a matching `audio..opus`. +6. **Extract audio** — one PCM wav per source audio stream, written to the per-job work directory as `audio.0.wav`, `audio.1.wav`, … in source order (PCM s16le, 48 kHz). Errors out if the source has no audio streams. +7. **Encode audio** — each wav is converted with `opusenc --bitrate 128k` to a matching `audio..opus` in the same work directory. 8. **Calculate display width** from SAR. The width is rescaled so the output has square pixels only when there's actually work to do — `zscale` is skipped entirely for square-pixel sources (SAR `1:1`, `N/A`, empty, `0:N`), and for any SAR whose calculated width rounds to the source width. When rescaling, the width is rounded to the nearest even number (mod-2, preferred by AV1). 9. **Encode video** with FFmpeg: - Video filter chain is built conditionally. `bwdif=mode=0:par=-1:-1` is prepended when the source is interlaced; the `zscale` step is appended only when a rescale is actually needed (see step 8). If neither applies, `-vf` is omitted entirely. @@ -191,9 +193,9 @@ For every `.mkv` found in `paths.input`, the program runs these steps in order. - Audio re-muxed (`-c:a copy` — copies the already-Opus-encoded streams), subtitles copied (`-c:s copy`). - `-map_metadata -1` strips global metadata; per-stream language tags are then re-applied. Audio language tags are indexed by **output position** (the position in the opus-file list), not by counting source streams, so missing-language streams don't shift the index. - Container metadata written: `TITLE`, `DATE_RELEASED`, `IMDBID`, `ORIGINAL_MEDIA_TYPE`. For series: also `COLLECTION`, `SEASON`, `EPISODE`, `TVMAZE_ID`. Values are unquoted (literal value, no wrapping `"…"`). - - Output written to `output.mkv` in the input directory. -10. **Clean up** intermediate `audio.*.wav` and `audio.*.opus` files. -11. **Rename and move** `output.mkv` to `paths.output` with a final name: + - Output written to `output.mkv` in the per-job work directory. +10. **Clean up** the entire per-job work directory (intermediates and `output.mkv` together) once the rename/move below succeeds — or, on any failure, when the deferred cleanup runs. +11. **Rename and move** `output.mkv` from the work directory to `paths.output` with a final name: - Series with a known show name → `.SE.mkv` (the series no longer needs a populated IMDb mapping — a TVmaze show with no external IMDb link still gets a useful filename). - Movie with a known title and IMDb ID → `..mkv`. - Anything else → `<8-hex-chars>.nometadata.mkv`. @@ -241,8 +243,8 @@ Note: a number of `DEBUG` lines are printed to stdout/stderr (ffprobe output, ca If any step from probing through encoding through renaming fails: -- The source `.mkv` is moved to `paths.failed`. -- Any partial `output.mkv` left in the input directory is also moved to `paths.failed`. +- The source `.mkv` is moved to `paths.failed` (the move itself is `os.Stat`-guarded — if the source is already gone, the move is skipped and logged; if the move itself errors, that error is logged too). This guarantees the source leaves `paths.input` on every failure path, so the watcher doesn't retry the same file on the next tick. +- The per-job work directory under `paths.work` (containing partial wav/opus/output.mkv) is deleted unconditionally. - The error is logged to `error_*.log` and `structured.json` with the source file path. The watcher continues with the next file; one bad rip won't stop the daemon. @@ -270,3 +272,13 @@ internal/mover/ File rename/move/delete helpers internal/logger/ Plain + JSON logging pkg/types/types.go Shared structs (Config, Job, Metadata, …) ``` + +Runtime directories (from `paths.*` in the config): + +``` +paths.input User drops new .mkv files here; watcher polls every 15 s +paths.output Finished encodes land here under their final name +paths.originals Successfully-encoded sources end up here (unless -d) +paths.failed Sources of failed jobs end up here +paths.work Per-job scratch subdir (basename of input); wiped per job +``` diff --git a/cmd/videnc/main.go b/cmd/videnc/main.go index c4874c8..89a8c8e 100644 --- a/cmd/videnc/main.go +++ b/cmd/videnc/main.go @@ -9,6 +9,7 @@ import ( "os/signal" "path/filepath" "regexp" + "strings" "syscall" "videnc-vibe/internal/config" @@ -83,10 +84,23 @@ func processFile(inputPath string, cfg *types.Config, enc *encoder.Encoder, meta filename := filepath.Base(inputPath) isSeries, imdbID, tvmazeID, season, episode := metadata.ParseFilename(filename) + // Per-job work subdirectory under paths.work. Uses the source base name + // (without ".mkv") as the subdir name. MkdirAll is idempotent, so a + // leftover dir from a crashed previous run is harmless to overwrite. + workDirName := strings.TrimSuffix(filename, filepath.Ext(filename)) + workDir := filepath.Join(cfg.Paths.Work, workDirName) + if err := os.MkdirAll(workDir, 0755); err != nil { + log.ErrorFile(inputPath, "Creating work directory", err.Error()) + failToFailed(inputPath, cfg.Paths.Failed, log) + return err + } + // Cleanup the work directory on every exit path, success or failure. + defer os.RemoveAll(workDir) + width, height, interlaced, err := enc.GetMediaInfo(inputPath) if err != nil { log.ErrorFile(inputPath, "Getting media info", err.Error()) - mover.MoveToFailed(inputPath, cfg.Paths.Failed) + failToFailed(inputPath, cfg.Paths.Failed, log) return err } @@ -145,16 +159,13 @@ func processFile(inputPath string, cfg *types.Config, enc *encoder.Encoder, meta DeleteOrigin: deleteOrigin, } - if err := enc.Transcode(inputPath, job, meta, interlaced, streamLangs); err != nil { + if err := enc.Transcode(inputPath, workDir, job, meta, interlaced, streamLangs); err != nil { log.ErrorFile(inputPath, "Transcoding", err.Error()) - mover.MoveToFailed(inputPath, cfg.Paths.Failed) - outputPath := filepath.Join(filepath.Dir(inputPath), "output.mkv") - mover.MoveToFailed(outputPath, cfg.Paths.Failed) + failToFailed(inputPath, cfg.Paths.Failed, log) return err } - outputDir := filepath.Dir(inputPath) - outputPath := filepath.Join(outputDir, "output.mkv") + outputPath := filepath.Join(workDir, "output.mkv") var outFilename string switch { @@ -168,8 +179,11 @@ func processFile(inputPath string, cfg *types.Config, enc *encoder.Encoder, meta finalOutput := filepath.Join(cfg.Paths.Output, outFilename) if err := mover.Rename(outputPath, finalOutput); err != nil { + // Move the source to failed/ so it doesn't get re-encoded on the next + // tick. The deferred RemoveAll(workDir) takes care of the partial + // output.mkv left in the work dir. log.ErrorFile(inputPath, "Moving output", err.Error()) - mover.MoveToFailed(outputPath, cfg.Paths.Failed) + failToFailed(inputPath, cfg.Paths.Failed, log) return err } @@ -185,6 +199,23 @@ func processFile(inputPath string, cfg *types.Config, enc *encoder.Encoder, meta return nil } +// failToFailed stats path before invoking MoveToFailed: if missing, logs and +// skips; if present, surfaces any move error to the logger. This avoids the +// silent no-op pattern where MoveToFailed was called on a non-existent file. +func failToFailed(path, failedDir string, log *logger.Logger) { + if _, err := os.Stat(path); err != nil { + if os.IsNotExist(err) { + log.Info(fmt.Sprintf("Skip move to failed; not present: %s", path)) + return + } + log.ErrorFile(path, "Stat before move to failed", err.Error()) + return + } + if err := mover.MoveToFailed(path, failedDir); err != nil { + log.ErrorFile(path, "Moving to failed", err.Error()) + } +} + func generateRandomString(length int) string { bytes := make([]byte, length/2+1) rand.Read(bytes) diff --git a/internal/config/config.go b/internal/config/config.go index 72dc11e..89f3ec5 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -49,6 +49,9 @@ func Load(configPath string) (*types.Config, error) { if cfg.Paths.Failed == "" { cfg.Paths.Failed = "./failed" } + if cfg.Paths.Work == "" { + cfg.Paths.Work = "./work" + } if cfg.Encoding.DVD.CRF == 0 { cfg.Encoding.DVD.CRF = 30 } @@ -78,7 +81,7 @@ func Load(configPath string) (*types.Config, error) { } func EnsureDirs(cfg *types.Config) error { - dirs := []string{cfg.Paths.Input, cfg.Paths.Output, cfg.Paths.Originals, cfg.Paths.Failed} + dirs := []string{cfg.Paths.Input, cfg.Paths.Output, cfg.Paths.Originals, cfg.Paths.Failed, cfg.Paths.Work} for _, dir := range dirs { if err := os.MkdirAll(dir, 0755); err != nil { return fmt.Errorf("creating directory %s: %w", dir, err) diff --git a/internal/encoder/encoder.go b/internal/encoder/encoder.go index 600deba..a3bf803 100644 --- a/internal/encoder/encoder.go +++ b/internal/encoder/encoder.go @@ -3,7 +3,6 @@ package encoder import ( "encoding/json" "fmt" - "os" "os/exec" "path/filepath" "regexp" @@ -222,21 +221,18 @@ func (e *Encoder) calculateZscaleWidth(path string, originalHeight int) (string, return zscale, newWidth, nil } -func (e *Encoder) Transcode(input string, job *types.Job, metadata *types.Metadata, interlaced bool, streamLangs []StreamMetadata) error { - dir := filepath.Dir(input) - audioWavs, err := e.extractAudio(input, dir, streamLangs) +func (e *Encoder) Transcode(input, workDir string, job *types.Job, metadata *types.Metadata, interlaced bool, streamLangs []StreamMetadata) error { + audioWavs, err := e.extractAudio(input, workDir, streamLangs) if err != nil { return fmt.Errorf("extracting audio: %w", err) } - defer e.cleanupWavs(audioWavs) - opusFiles, err := e.encodeOpus(audioWavs, dir) + opusFiles, err := e.encodeOpus(audioWavs, workDir) if err != nil { return fmt.Errorf("encoding opus: %w", err) } - defer e.cleanupOpus(opusFiles) - if err := e.encodeVideo(input, opusFiles, job, metadata, interlaced, streamLangs); err != nil { + if err := e.encodeVideo(input, workDir, opusFiles, job, metadata, interlaced, streamLangs); err != nil { return fmt.Errorf("encoding video: %w", err) } @@ -257,7 +253,7 @@ func audioStreamsInSourceOrder(streamLangs []StreamMetadata) []StreamMetadata { return audio } -func (e *Encoder) extractAudio(input, dir string, streamLangs []StreamMetadata) ([]string, error) { +func (e *Encoder) extractAudio(input, workDir string, streamLangs []StreamMetadata) ([]string, error) { audio := audioStreamsInSourceOrder(streamLangs) if len(audio) == 0 { return nil, fmt.Errorf("no audio streams found") @@ -265,7 +261,7 @@ func (e *Encoder) extractAudio(input, dir string, streamLangs []StreamMetadata) wavs := make([]string, 0, len(audio)) for srcAudioIndex := range audio { - wavPath := filepath.Join(dir, fmt.Sprintf("audio.%d.wav", srcAudioIndex)) + wavPath := filepath.Join(workDir, fmt.Sprintf("audio.%d.wav", srcAudioIndex)) cmd := exec.Command(e.ffmpegPath, "-i", input, "-map", fmt.Sprintf("0:a:%d", srcAudioIndex), "-vn", "-c:a", "pcm_s16le", "-ar", "48000", @@ -278,22 +274,22 @@ func (e *Encoder) extractAudio(input, dir string, streamLangs []StreamMetadata) return wavs, nil } -func (e *Encoder) encodeOpus(wavs []string, dir string) ([]string, error) { +func (e *Encoder) encodeOpus(wavs []string, workDir string) ([]string, error) { var opusFiles []string for _, wav := range wavs { - out := strings.Replace(wav, ".wav", ".opus", 1) + base := filepath.Base(strings.Replace(wav, ".wav", ".opus", 1)) + out := filepath.Join(workDir, base) cmd := exec.Command(e.opusencPath, "--bitrate", "128k", wav, out) - if out, err := cmd.CombinedOutput(); err != nil { - return nil, fmt.Errorf("opusenc: %s %w", out, err) + if logOut, err := cmd.CombinedOutput(); err != nil { + return nil, fmt.Errorf("opusenc: %s %w", logOut, err) } opusFiles = append(opusFiles, out) } return opusFiles, nil } -func (e *Encoder) encodeVideo(input string, opusFiles []string, job *types.Job, metadata *types.Metadata, interlaced bool, streamLangs []StreamMetadata) error { - dir := filepath.Dir(input) - outFile := filepath.Join(dir, "output.mkv") +func (e *Encoder) encodeVideo(input, workDir string, opusFiles []string, job *types.Job, metadata *types.Metadata, interlaced bool, streamLangs []StreamMetadata) error { + outFile := filepath.Join(workDir, "output.mkv") svtParams := fmt.Sprintf("film-grain=10:film-grain-denoise=1:scd=1:qm-min=4:qm-max=15:keyint=10s") @@ -398,15 +394,3 @@ func (e *Encoder) encodeVideo(input string, opusFiles []string, job *types.Job, return nil } - -func (e *Encoder) cleanupWavs(files []string) { - for _, f := range files { - os.Remove(f) - } -} - -func (e *Encoder) cleanupOpus(files []string) { - for _, f := range files { - os.Remove(f) - } -} diff --git a/pkg/types/types.go b/pkg/types/types.go index db22642..1103ea3 100644 --- a/pkg/types/types.go +++ b/pkg/types/types.go @@ -58,6 +58,7 @@ type PathsConfig struct { Output string `yaml:"output"` Originals string `yaml:"originals"` Failed string `yaml:"failed"` + Work string `yaml:"work"` } type LogEntry struct {