CI: Apply Go linter recommendations to "internal/ffmpeg" package #5330

Signed-off-by: Michael Mayer <michael@photoprism.app>
This commit is contained in:
Michael Mayer
2025-11-22 12:58:11 +01:00
parent 153ebd5905
commit 43bca10b4e
13 changed files with 102 additions and 16 deletions

70
internal/ffmpeg/README.md Normal file
View File

@@ -0,0 +1,70 @@
## PhotoPrism — FFmpeg Integration
**Last Updated:** November 22, 2025
### Overview
`internal/ffmpeg` wraps the `ffmpeg` CLI to transcode videos to AVC/H.264, remux containers, and extract preview frames in a predictable, testable way. Command builders share option structs so CLI tools, workers, and tests can select software or hardware encoders without duplicating flag logic.
#### Context & Constraints
- Relies on the system `ffmpeg` binary; defaults to `FFmpegBin` but callers may override `Options.Bin`.
- Inputs are internal filenames and option structs (not user input); exec invocations are annotated with `#nosec G204`.
- Downstream jobs may run concurrently, so `TranscodeCmd` returns a `useMutex` hint to serialize expensive work.
- Remux and extract commands honor `Force` and reuse shared map flags; metadata copying is limited to safe defaults.
#### Goals
- Provide consistent command lines for software and hardware AVC encoders.
- Keep remuxing and preview extraction lightweight while preserving metadata where possible.
- Centralize quality and size clamping logic so UIs/CLI can pass user preferences safely.
#### Non-Goals
- Full coverage of every FFmpeg codec or container; the package focuses on MP4/H.264 paths required by PhotoPrism.
- Direct management of FFmpeg installation or GPU availability.
### Encoders, Containers, & Hardware
- **Software AVC:** `encode.TranscodeToAvcCmd` (x264 or default encoder).
- **Intel Quick Sync:** `internal/ffmpeg/intel` (`h264_qsv`) with optional `Options.Device`.
- **NVIDIA NVENC:** `internal/ffmpeg/nvidia` (`h264_nvenc`).
- **Apple VideoToolbox:** `internal/ffmpeg/apple` (`h264_videotoolbox`).
- **VA-API:** `internal/ffmpeg/vaapi` (`h264_vaapi`) supporting optional device paths.
- **V4L2 M2M:** `internal/ffmpeg/v4l` (`h264_v4l2m2m`) for ARM/embedded targets.
- **Containers:** MP4 is the primary target (`fs.VideoMp4`); `RemuxCmd` can handle other `fs.Type` values when provided.
- **Streaming flags:** `encode.MovFlags` defaults to `use_metadata_tags+faststart` to keep outputs stream-friendly.
### Package Layout (Code Map)
- `encode/` — shared option structs, quality helpers, default map/metadata flags, software AVC command builder.
- `apple/`, `intel/`, `nvidia/`, `vaapi/`, `v4l/` — hardware-specific AVC command builders.
- `remux.go` — container-only transfers with metadata copy and temp-file safety.
- `transcode_cmd.go` — selects encoder, handles animated image inputs, and signals mutex usage.
- `extract_image_cmd.go` — JPEG/PNG preview frame extraction with color-space presets.
- `test.go` & `*_test.go` — reusable command runner and smoke tests (use fixtures in `testdata/`).
- `ffmpeg.go` — package logger hook.
### Related Packages & Entry Points
- `internal/thumb` calls these builders for video previews and thumbnails.
- `internal/commands` and workers select encoders based on configuration options and reuse `encode.Options`.
- `pkg/fs` supplies path helpers, existence checks, and file-mode constants referenced by remux/extract logic.
### Configuration & Safety Notes
- Clamp size and quality via `NewVideoOptions` to `[1, 15360]` pixels and the defined quality bounds.
- Remuxing respects `Options.Force`; without it existing outputs are preserved.
- Metadata copying uses `-map_metadata` and `clean` sanitizers; only safe string fields (title, description, comment, author, creation_time) are added when set.
- Hardware helpers expect the matching FFmpeg build and devices; callers should gate selection via config or environment (see `PHOTOPRISM_FFMPEG_ENCODER` guidance in `AGENTS.md`).
### Testing
- Run unit tests: `go test ./internal/ffmpeg/...`
- Hardware-specific tests assume the encoder is available; keep runs gated via config when adding new cases.
### Operational Tips
- Prefer `TranscodeCmd` over manual `exec.Command` to keep logging, metadata, and mutex hints consistent.
- Use `RemuxFile` to convert containers without re-encoding; it creates a temp file and swaps atomically.
- For preview frames, pass `encode.Options` with `SeekOffset` and `TimeOffset` computed from video duration (see `NewPreviewImageOptions`).

View File

@@ -8,6 +8,7 @@ import (
// TranscodeToAvcCmd returns the FFmpeg command for hardware-accelerated transcoding to MPEG-4 AVC.
func TranscodeToAvcCmd(srcName, destName string, opt encode.Options) *exec.Cmd {
// #nosec G204 -- command arguments are built from validated options and paths.
return exec.Command(
opt.Bin,
"-hide_banner",

View File

@@ -4,6 +4,7 @@ import "os/exec"
// TranscodeToAvcCmd returns the default FFmpeg command for transcoding video files to MPEG-4 AVC.
func TranscodeToAvcCmd(srcName, destName string, opt Options) *exec.Cmd {
// #nosec G204 -- command arguments are built from validated options and paths.
return exec.Command(
opt.Bin,
"-hide_banner",

View File

@@ -1,6 +1,6 @@
package encode
// The MovFlags default forces fragmented MP4 output suitable for streaming:
// MovFlags defines default fragmented MP4 flags suitable for streaming:
// - https://developer.mozilla.org/en-US/docs/Web/API/Media_Source_Extensions_API/Transcoding_assets_for_MSE#fragmenting
// - https://nschlia.github.io/ffmpegfs/html/ffmpeg__profiles_8cc.html
// - https://cloudinary.com/glossary/fragmented-mp4

View File

@@ -41,17 +41,19 @@ func NewVideoOptions(ffmpegBin string, encoder Encoder, sizeLimit, quality int,
encoder = DefaultAvcEncoder()
}
if sizeLimit < 1 {
switch {
case sizeLimit < 1:
sizeLimit = 1920
} else if sizeLimit > 15360 {
case sizeLimit > 15360:
sizeLimit = 15360
}
if quality <= 0 {
switch {
case quality <= 0:
quality = DefaultQuality
} else if quality < WorstQuality {
case quality < WorstQuality:
quality = WorstQuality
} else if quality >= BestQuality {
case quality >= BestQuality:
quality = BestQuality
}
@@ -118,13 +120,14 @@ func NewPreviewImageOptions(ffmpegBin string, videoDuration time.Duration) *Opti
// VideoFilter returns the FFmpeg video filter string based on the size limit in pixels and the pixel format.
func (o *Options) VideoFilter(format PixelFormat) string {
// scale specifies the FFmpeg downscale filter, see http://trac.ffmpeg.org/wiki/Scaling.
if format == "" {
switch format {
case "":
return fmt.Sprintf("scale='if(gte(iw,ih), min(%d, iw), -2):if(gte(iw,ih), -2, min(%d, ih))'", o.SizeLimit, o.SizeLimit)
} else if format == FormatQSV {
case FormatQSV:
return fmt.Sprintf("scale_qsv=w='if(gte(iw,ih), min(%d, iw), -1)':h='if(gte(iw,ih), -1, min(%d, ih))':format=nv12", o.SizeLimit, o.SizeLimit)
} else {
return fmt.Sprintf("scale='if(gte(iw,ih), min(%d, iw), -2):if(gte(iw,ih), -2, min(%d, ih))',format=%s", o.SizeLimit, o.SizeLimit, format)
}
return fmt.Sprintf("scale='if(gte(iw,ih), min(%d, iw), -2):if(gte(iw,ih), -2, min(%d, ih))',format=%s", o.SizeLimit, o.SizeLimit, format)
}
// QvQuality returns the video encoding quality as "-q:v" parameter string.

View File

@@ -26,6 +26,7 @@ func ExtractJpegImageCmd(videoName, imageName string, opt *encode.Options) *exec
// see https://github.com/photoprism/photoprism/issues/4488.
// Unfortunately, this filter would render thumbnails of non-HDR videos too dark:
// "-vf", "zscale=t=linear:npl=100,format=gbrpf32le,zscale=p=bt709,tonemap=tonemap=gamma:desat=0,zscale=t=bt709:m=bt709:r=tv,format=yuv420p",
// #nosec G204 -- paths and flags are created by the application, not user input.
return exec.Command(
opt.Bin,
"-hide_banner",
@@ -46,6 +47,7 @@ func ExtractJpegImageCmd(videoName, imageName string, opt *encode.Options) *exec
// ExtractPngImageCmd extracts a PNG still image from the specified source video file.
func ExtractPngImageCmd(videoName, imageName string, opt *encode.Options) *exec.Cmd {
// #nosec G204 -- paths and flags are created by the application, not user input.
return exec.Command(
opt.Bin,
"-hide_banner",

View File

@@ -10,6 +10,7 @@ import (
func TranscodeToAvcCmd(srcName, destName string, opt encode.Options) *exec.Cmd {
// ffmpeg -hide_banner -h encoder=h264_qsv
if opt.Device != "" {
// #nosec G204 -- command arguments are built from validated options and paths.
return exec.Command(
opt.Bin,
"-hide_banner",
@@ -33,6 +34,7 @@ func TranscodeToAvcCmd(srcName, destName string, opt encode.Options) *exec.Cmd {
destName,
)
} else {
// #nosec G204 -- command arguments are built from validated options and paths.
return exec.Command(
opt.Bin,
"-hide_banner",

View File

@@ -9,6 +9,7 @@ import (
// TranscodeToAvcCmd returns the FFmpeg command for hardware-accelerated transcoding to MPEG-4 AVC.
func TranscodeToAvcCmd(srcName, destName string, opt encode.Options) *exec.Cmd {
// ffmpeg -hide_banner -h encoder=h264_nvenc
// #nosec G204 -- command arguments are built from validated options and paths.
return exec.Command(
opt.Bin,
"-hide_banner",

View File

@@ -131,13 +131,14 @@ func RemuxFile(videoFilePath, destFilePath string, opt encode.Options) error {
// RemuxCmd returns the FFmpeg command for transferring content from one container format to another without altering the original video or audio stream.
func RemuxCmd(srcName, destName string, opt encode.Options) (cmd *exec.Cmd, err error) {
if srcName == "" {
switch {
case srcName == "":
return nil, fmt.Errorf("empty source filename")
} else if !fs.FileExistsNotEmpty(srcName) {
case !fs.FileExistsNotEmpty(srcName):
return nil, fmt.Errorf("source file is empty or missing")
} else if destName == "" {
case destName == "":
return nil, fmt.Errorf("empty destination filename")
} else if srcName == destName {
case srcName == destName:
return nil, fmt.Errorf("source and destination filenames must be different")
}
@@ -164,8 +165,7 @@ func RemuxCmd(srcName, destName string, opt encode.Options) (cmd *exec.Cmd, err
}
// Append format specific "ffmpeg" command flags.
switch opt.Container {
case fs.VideoMp4:
if opt.Container == fs.VideoMp4 {
// Ensure MP4 compatibility:
flags = append(flags,
"-movflags", opt.MovFlags,
@@ -197,6 +197,7 @@ func RemuxCmd(srcName, destName string, opt encode.Options) (cmd *exec.Cmd, err
// Set the destination file name as the last command flag.
flags = append(flags, destName)
// #nosec G204 -- filenames and flags are constructed internally and not user-controlled.
cmd = exec.Command(
opt.Bin,
flags...,

View File

@@ -13,6 +13,7 @@ import (
"github.com/photoprism/photoprism/pkg/fs"
)
// RunCommandTest executes ffmpeg command tests and cleans up created files.
func RunCommandTest(t *testing.T, encoder encode.Encoder, srcName, destName string, cmd *exec.Cmd, deleteAfterTest bool) {
var out bytes.Buffer
var stderr bytes.Buffer

View File

@@ -31,6 +31,7 @@ func TranscodeCmd(srcName, destName string, opt encode.Options) (cmd *exec.Cmd,
// Always use software encoder for transcoding animated pictures into videos.
if fs.TypeAnimated[fs.FileType(srcName)] != "" {
// #nosec G204 -- command arguments are built from validated options and paths.
cmd = exec.Command(
opt.Bin,
"-hide_banner",

View File

@@ -9,6 +9,7 @@ import (
// TranscodeToAvcCmd returns the FFmpeg command for hardware-accelerated transcoding to MPEG-4 AVC.
func TranscodeToAvcCmd(srcName, destName string, opt encode.Options) *exec.Cmd {
// ffmpeg -hide_banner -h encoder=h264_v4l2m2m
// #nosec G204 -- command arguments are built from validated options and paths.
return exec.Command(
opt.Bin,
"-hide_banner",

View File

@@ -9,6 +9,7 @@ import (
// TranscodeToAvcCmd returns the FFmpeg command for hardware-accelerated transcoding to MPEG-4 AVC.
func TranscodeToAvcCmd(srcName, destName string, opt encode.Options) *exec.Cmd {
if opt.Device != "" {
// #nosec G204 -- command arguments are built from validated options and paths.
return exec.Command(
opt.Bin,
"-hide_banner",
@@ -30,6 +31,7 @@ func TranscodeToAvcCmd(srcName, destName string, opt encode.Options) *exec.Cmd {
destName,
)
} else {
// #nosec G204 -- command arguments are built from validated options and paths.
return exec.Command(
opt.Bin,
"-hide_banner",