fix(playback): prevent transcode resolution upscaling (#503)

* docs(playback): design transcode resolution clamp

* docs(playback): plan transcode resolution clamp

* fix(playback): prevent transcode resolution upscaling

* refactor(playback): share transcode resolution tiers

---------

Co-authored-by: rxwatcher <rxwatcher@users.noreply.github.com>
Co-authored-by: Quick <31828688+Quick104@users.noreply.github.com>
This commit is contained in:
RXWatcher
2026-07-28 13:07:05 -04:00
committed by GitHub
co-authored by rxwatcher Quick
parent 5d6f6323d5
commit 63e18cf37f
4 changed files with 503 additions and 13 deletions
@@ -0,0 +1,206 @@
# Transcode Resolution Clamp Implementation Plan
> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.
**Goal:** Prevent encoded-video transports from targeting a resolution above the final effective source file.
**Architecture:** Add one pure resolution-normalization helper in the playback API handler and invoke it exactly once after alternate-file selection. Mutate the request's target resolution at that boundary so remote transport, local transport, persistence, activity reporting, and reconstruction consume the same normalized recipe.
**Tech Stack:** Go, `net/http` handler tests, Silo playback session and transcode-node test doubles.
## Global Constraints
- Do not apply the new clamp to video-copy requests; preserve their existing
downstream recipe normalization.
- Preserve empty or unrecognized requested/source resolutions unchanged.
- Clamp only recognized encoded-video targets above the recognized effective source.
- Do not alter bitrate, codecs, API shapes, status codes, settings, migrations, clients, or production configuration.
- Prove the regression RED before writing production code.
---
### Task 1: Normalize the Effective Transcode Recipe
**Files:**
- Modify: `internal/api/handlers/playback.go`
- Test: `internal/api/handlers/playback_test.go`
**Interfaces:**
- Consumes: `transcodeStartRequest.TargetResolution`, `transcodeStartRequest.TargetCodecVideo`, and `models.MediaFile.Resolution` after alternate-file selection.
- Produces: `clampEncodedTargetResolution(requestedResolution, sourceResolution string) string`.
- Produces: one normalized `req.TargetResolution` consumed unchanged by remote/local transport construction and `buildTranscodeSessionReplacement`.
- [ ] **Step 1: Strengthen the existing remote fallback test**
Change `TestHandleStartTranscode_PreservesRecomputedBaseMethodAfterFallback` to
request 2160p at 10000 kbps. Assert that the captured
`transcodenode.TranscodeStartRequest.TargetResolution` is `1080p`, while the
effective alternate remains file 99 and the resulting playback session stores
`TargetResolution == "1080p"`.
```go
strings.NewReader(`{"session_id":"` + startResp.SessionID +
`","seek_seconds":0,"target_resolution":"2160p",` +
`"target_codec_video":"h264","target_codec_audio":"aac",` +
`"target_bitrate_kbps":10000,"segment_duration":2,` +
`"subtitle_track_index":-1,"subtitle_burn_in":false}`)
if remoteStartReq.TargetResolution != "1080p" {
t.Fatalf("remote target resolution = %q, want 1080p", remoteStartReq.TargetResolution)
}
if session.TargetResolution != "1080p" {
t.Fatalf("session target resolution = %q, want 1080p", session.TargetResolution)
}
```
- [ ] **Step 2: Run the regression and verify RED**
Run:
```bash
go test ./internal/api/handlers -run TestHandleStartTranscode_PreservesRecomputedBaseMethodAfterFallback -count=1
```
Expected: FAIL because the captured remote request still contains `2160p`.
- [ ] **Step 3: Add pure normalization table tests**
Add `TestClampEncodedTargetResolution` beside the handler tests. Cover:
```go
tests := []struct {
name, requested, source, want string
}{
{"clamps 2160p to 1080p", "2160p", "1080p", "1080p"},
{"keeps lower target", "720p", "1080p", "720p"},
{"keeps equal target", "1080p", "1080p", "1080p"},
{"keeps empty target", "", "1080p", ""},
{"keeps unknown target", "source", "1080p", "source"},
{"keeps target for unknown source", "2160p", "native", "2160p"},
{"supports low tiers", "480p", "420p", "420p"},
}
```
- [ ] **Step 4: Add the minimal helper and post-selection call**
Add a package-level recognized-height map/helper near `resolutionRank`:
```go
func clampEncodedTargetResolution(requestedResolution, sourceResolution string) string {
requestedHeight, requestedKnown := transcodeResolutionHeight(requestedResolution)
sourceHeight, sourceKnown := transcodeResolutionHeight(sourceResolution)
if !requestedKnown || !sourceKnown || requestedHeight <= sourceHeight {
return requestedResolution
}
return sourceResolution
}
```
After the 4K alternate block and before transport planning:
```go
if !videoCopy {
req.TargetResolution = clampEncodedTargetResolution(
req.TargetResolution,
file.Resolution,
)
}
```
Use an exhaustive switch for the six supported transcode tiers. Do not modify
the FFmpeg filter builder or web client.
- [ ] **Step 5: Verify focused GREEN**
Run:
```bash
go test ./internal/api/handlers \
-run 'TestHandleStartTranscode_PreservesRecomputedBaseMethodAfterFallback|TestClampEncodedTargetResolution' \
-count=1
```
Expected: PASS.
- [ ] **Step 6: Add copy and local-boundary regression coverage**
Add focused tests proving:
- a copy-video request bypasses the new clamp and retains its existing
downstream empty-resolution recipe;
- a local encoded-video request receives the clamped target in the persisted
session and generated FFmpeg recipe boundary;
- a requested 720p encode for a 1080p effective file remains 720p.
Use the existing local fake-FFmpeg and session-manager patterns in
`playback_test.go`; do not introduce production test hooks.
- [ ] **Step 7: Run focused and package verification**
Run:
```bash
gofmt -w internal/api/handlers/playback.go internal/api/handlers/playback_test.go
go test ./internal/api/handlers -count=1
go test ./internal/playback -count=1
go test ./internal/api/handlers ./internal/playback -race -count=1
git diff --check
```
Expected: all commands exit 0.
- [ ] **Step 8: Commit implementation**
```bash
git add internal/api/handlers/playback.go internal/api/handlers/playback_test.go
git commit -m "fix(playback): prevent transcode resolution upscaling"
```
### Task 2: Independent Review and Final Verification
**Files:**
- Review: `docs/superpowers/specs/2026-07-28-transcode-resolution-clamp-design.md`
- Review: `internal/api/handlers/playback.go`
- Review: `internal/api/handlers/playback_test.go`
**Interfaces:**
- Consumes: committed spec and implementation diff from `upstream/main`.
- Produces: reviewer verdict and a clean, verified branch suitable for a separate pull request.
- [ ] **Step 1: Request focused correctness/security review**
Review the change against these invariants:
- no encoded output exceeds the effective source tier;
- copy and unknown-resolution compatibility is preserved;
- local, offloaded, persisted, and reconstruction paths share one value;
- no target-bitrate or API behavior was broadened;
- tests fail without the production correction.
- [ ] **Step 2: Address Critical or Important findings test-first**
For each accepted behavioral finding, add or strengthen a failing test, observe
RED, make the smallest production correction, and rerun focused GREEN. Do not
bundle unrelated cleanup.
- [ ] **Step 3: Run final verification**
Run:
```bash
go test ./internal/api/handlers ./internal/playback -count=1
go test ./internal/api/handlers ./internal/playback -race -count=1
go test ./... -count=1
make verify-local-paths
git diff --check
git status --short --branch
```
Expected: tests and repository policy checks exit 0; status contains no
uncommitted implementation changes.
- [ ] **Step 4: Prepare handoff**
Summarize the production evidence, exact normalization boundary, RED/GREEN
proof, focused/full verification, reviewer verdict, and deployment caveat.
Do not deploy or mutate production as part of this plan.
@@ -0,0 +1,86 @@
# Transcode Resolution Clamp Design
## Problem
When video encoding is requested for a 2160p file while 4K transcoding is
disabled, the playback handler selects a lower-resolution alternate file.
Today it continues using the target resolution supplied for the original file.
That lets a 1080p alternate reach FFmpeg with a 2160p target, causing an
unnecessary upscale and making the persisted playback activity claim that
2160p was delivered.
Production logs confirmed this exact path with a 1080p H.264 alternate and a
QSV `scale_vaapi` filter targeting 2160 lines.
## Decision
The server owns the output-resolution invariant. After all alternate-file and
probe selection has completed, and before planning or starting any local or
remote transport, it will normalize an encoded-video target so it cannot
exceed the effective source file's resolution.
The normalization rules are:
- Do not apply the new clamp to video-copy requests; preserve their existing
downstream recipe normalization.
- Preserve an empty or unrecognized requested resolution unchanged.
- Preserve an empty or unrecognized effective source resolution unchanged.
- Preserve targets at or below the effective source resolution.
- Clamp a recognized target above a recognized effective source resolution to
the effective source resolution.
- Do not alter target bitrate. The observed 10 Mbps value is already a valid
1080p-high encoding target and is not the demonstrated defect.
Recognized resolutions are the same tiers already supported by the transcode
filter builders: 2160p, 1080p, 720p, 480p, 420p, and 328p.
## Architecture and Data Flow
A small pure helper in `internal/api/handlers/playback.go` compares requested
and effective-source resolution tiers. `HandleStartTranscode` calls it once
after the 4K alternate guard has selected and probed the effective file.
The normalized value replaces `req.TargetResolution`. Existing downstream
paths then automatically use one consistent value for:
- transcode-node planning and requests;
- integrated/local `playback.TranscodeOpts`;
- playback-session replacement and activity reporting;
- later transport reconstruction and restart recipes.
This boundary is preferred over a web-only correction because every client can
call the endpoint, and over an FFmpeg-only correction because session state and
remote transcode requests must remain truthful.
## Compatibility and Error Handling
The change is additive enforcement inside the existing `/api/v1` endpoint. It
does not change request or response shapes, status codes, codec selection,
alternate-file selection, or copy-video behavior. Unsupported resolution
strings retain their existing behavior rather than being silently reinterpreted.
No server settings, migrations, client changes, or production deployment
changes are part of this work.
## Verification
Tests will establish:
1. A 2160p encode request switched to a 1080p alternate reaches the remote
transcode node as 1080p and persists 1080p in the playback session.
2. The same normalized request reaches the local transcode option boundary.
3. Targets already below the source remain unchanged.
4. Copy-video and unknown-resolution requests remain unchanged.
5. Existing playback-handler and playback-package suites remain green.
The regression must fail against unmodified main before the implementation is
added.
## Security and Operational Impact
The server currently permits an authenticated user with transcoding permission
to amplify compute and bandwidth by requesting a recognized output larger than
the effective source. The clamp removes that unnecessary amplification while
retaining all authorized transcoding behavior.
Rollback is a single code commit. No data rollback is required.
+53 -10
View File
@@ -3149,6 +3149,9 @@ func (h *PlaybackHandler) HandleStartTranscode(w http.ResponseWriter, r *http.Re
switchedFileID = &alt.ID
}
}
if !videoCopy {
req.TargetResolution = clampEncodedTargetResolution(req.TargetResolution, file.Resolution)
}
if requestedFile != nil && file != nil && requestedFile.ID != file.ID {
if err := preflightPlaybackFile(r.Context(), requestedFile, h.MissingMarker, h.EventsHub); err != nil && !isPlaybackFileMissing(err) {
slog.WarnContext(r.Context(), "requested transcode file preflight failed; continuing with alternate file", "component", "api",
@@ -3936,20 +3939,60 @@ func (h *PlaybackHandler) findAlternateFile(ctx context.Context, source *models.
return candidates[0], nil
}
const (
transcodeResolution2160p = "2160p"
transcodeResolution1080p = "1080p"
transcodeResolution720p = "720p"
transcodeResolution480p = "480p"
transcodeResolution420p = "420p"
transcodeResolution328p = "328p"
)
// resolutionRank returns a numeric rank for resolution sorting.
func resolutionRank(res string) int {
switch res {
case "2160p":
return 4
case "1080p":
return 3
case "720p":
return 2
case "480p":
return 1
case "328p":
height, known := transcodeResolutionHeight(res)
if !known {
return 0
}
switch {
case height >= 2160:
return 4
case height >= 1080:
return 3
case height >= 720:
return 2
case height >= 480:
return 1
default:
return 0
}
}
func clampEncodedTargetResolution(requestedResolution, sourceResolution string) string {
requestedHeight, requestedKnown := transcodeResolutionHeight(requestedResolution)
sourceHeight, sourceKnown := transcodeResolutionHeight(sourceResolution)
if !requestedKnown || !sourceKnown || requestedHeight <= sourceHeight {
return requestedResolution
}
return sourceResolution
}
func transcodeResolutionHeight(resolution string) (int, bool) {
switch resolution {
case transcodeResolution2160p:
return 2160, true
case transcodeResolution1080p:
return 1080, true
case transcodeResolution720p:
return 720, true
case transcodeResolution480p:
return 480, true
case transcodeResolution420p:
return 420, true
case transcodeResolution328p:
return 328, true
default:
return 0, false
}
}
+158 -3
View File
@@ -1343,7 +1343,7 @@ func TestHandleStartTranscode_PreservesRecomputedBaseMethodAfterFallback(t *test
transcodeReq := httptest.NewRequest(
"POST",
"/api/v1/playback/transcode/start",
strings.NewReader(`{"session_id":"`+startResp.SessionID+`","seek_seconds":0,"target_resolution":"720p","target_codec_video":"h264","target_codec_audio":"aac","target_bitrate_kbps":2000,"segment_duration":2,"subtitle_track_index":-1,"subtitle_burn_in":false}`),
strings.NewReader(`{"session_id":"`+startResp.SessionID+`","seek_seconds":0,"target_resolution":"2160p","target_codec_video":"h264","target_codec_audio":"aac","target_bitrate_kbps":10000,"segment_duration":2,"subtitle_track_index":-1,"subtitle_burn_in":false}`),
)
transcodeReq = transcodeReq.WithContext(newAuthorizedPlaybackContext())
@@ -1359,6 +1359,12 @@ func TestHandleStartTranscode_PreservesRecomputedBaseMethodAfterFallback(t *test
if remoteStartReq.AudioTrackIndex != 1 {
t.Fatalf("remote audio_track_index = %d, want 1", remoteStartReq.AudioTrackIndex)
}
if remoteStartReq.TargetResolution != transcodeResolution1080p {
t.Fatalf("remote target resolution = %q, want 1080p", remoteStartReq.TargetResolution)
}
if remoteStartReq.TargetBitrateKbps != 10000 {
t.Fatalf("remote target bitrate = %d, want unchanged 10000", remoteStartReq.TargetBitrateKbps)
}
session, err := sessionMgr.GetSession(startResp.SessionID)
if err != nil {
@@ -1370,6 +1376,12 @@ func TestHandleStartTranscode_PreservesRecomputedBaseMethodAfterFallback(t *test
if session.BasePlayMethod != playback.PlayRemux {
t.Fatalf("session.BasePlayMethod = %q, want %q", session.BasePlayMethod, playback.PlayRemux)
}
if session.TargetResolution != transcodeResolution1080p {
t.Fatalf("session target resolution = %q, want 1080p", session.TargetResolution)
}
if session.TargetBitrateKbps != 10000 {
t.Fatalf("session target bitrate = %d, want unchanged 10000", session.TargetBitrateKbps)
}
}
// newRemoteAudioSwitchSession builds a session that is already an offloaded
@@ -2010,7 +2022,7 @@ func TestHandleStartTranscode_LocalPathPropagatesSelectedAudioTrack(t *testing.T
transcodeReq := httptest.NewRequest(
"POST",
"/api/v1/playback/transcode/start",
strings.NewReader(`{"session_id":"`+startResp.SessionID+`","seek_seconds":0,"target_resolution":"720p","target_codec_video":"h264","target_codec_audio":"aac","target_bitrate_kbps":2000,"segment_duration":2,"subtitle_track_index":-1,"subtitle_burn_in":false}`),
strings.NewReader(`{"session_id":"`+startResp.SessionID+`","seek_seconds":0,"target_resolution":"2160p","target_codec_video":"h264","target_codec_audio":"aac","target_bitrate_kbps":10000,"segment_duration":2,"subtitle_track_index":-1,"subtitle_burn_in":false}`),
)
transcodeReq = transcodeReq.WithContext(newAuthorizedPlaybackContext())
@@ -2030,6 +2042,22 @@ func TestHandleStartTranscode_LocalPathPropagatesSelectedAudioTrack(t *testing.T
if got := transcodeSession.Opts().AudioTrackIndex; got != 1 {
t.Fatalf("local transcode audio track index = %d, want 1", got)
}
if got := transcodeSession.Opts().TargetResolution; got != transcodeResolution1080p {
t.Fatalf("local target resolution = %q, want 1080p", got)
}
if got := transcodeSession.Opts().TargetBitrateKbps; got != 10000 {
t.Fatalf("local target bitrate = %d, want unchanged 10000", got)
}
session, err := sessionMgr.GetSession(startResp.SessionID)
if err != nil {
t.Fatalf("GetSession: %v", err)
}
if session.TargetResolution != transcodeResolution1080p {
t.Fatalf("session target resolution = %q, want 1080p", session.TargetResolution)
}
if session.TargetBitrateKbps != 10000 {
t.Fatalf("session target bitrate = %d, want unchanged 10000", session.TargetBitrateKbps)
}
}
func TestHandleChangeAudioTrack_LocalCopyRestartUsesFreshSeekAnchor(t *testing.T) {
@@ -2400,7 +2428,7 @@ func TestHandleStartTranscode_SeekedCopyPreservedOnRemoteNode(t *testing.T) {
transcodeReq := httptest.NewRequest(
http.MethodPost,
"/api/v1/playback/transcode/start",
strings.NewReader(`{"session_id":"`+session.ID+`","seek_seconds":18.261,"target_resolution":"","target_codec_video":"copy","target_codec_audio":"aac","target_bitrate_kbps":0,"segment_duration":2,"subtitle_track_index":-1,"subtitle_burn_in":false}`),
strings.NewReader(`{"session_id":"`+session.ID+`","seek_seconds":18.261,"target_resolution":"2160p","target_codec_video":"copy","target_codec_audio":"aac","target_bitrate_kbps":0,"segment_duration":2,"subtitle_track_index":-1,"subtitle_burn_in":false}`),
).WithContext(newAuthorizedPlaybackContext())
transcodeRR := httptest.NewRecorder()
handler.HandleStartTranscode(transcodeRR, transcodeReq)
@@ -2411,6 +2439,9 @@ func TestHandleStartTranscode_SeekedCopyPreservedOnRemoteNode(t *testing.T) {
remoteStartReq.StreamOriginSeconds != 16 || !remoteStartReq.CopySeekAnchorResolved || remoteStartReq.StartSegmentNumber != 8 {
t.Fatalf("remote copy recipe = codec %q seek %v origin %v segment %d", remoteStartReq.TargetCodecVideo, remoteStartReq.SeekSeconds, remoteStartReq.StreamOriginSeconds, remoteStartReq.StartSegmentNumber)
}
if remoteStartReq.TargetResolution != transcodeResolution2160p {
t.Fatalf("remote copy target resolution = %q, want unchanged 2160p", remoteStartReq.TargetResolution)
}
if !strings.HasPrefix(remoteStartReq.SessionID, session.ID+legacyTransportMarker) {
t.Fatalf("remote SessionID = %q, want legacy transport for %q", remoteStartReq.SessionID, session.ID)
}
@@ -2904,6 +2935,9 @@ func TestHandleStartTranscode_SeekedCopyAllowedWhenVideoTranscodingDisabled(t *t
if got := transcodeSession.Opts().TargetCodecVideo; got != "copy" {
t.Fatalf("target video codec = %q, want copy", got)
}
if got := transcodeSession.Opts().TargetResolution; got != "" {
t.Fatalf("copy-video target resolution = %q, want existing empty recipe", got)
}
}
func TestHandleStartTranscode_SubtitleBurnInRechecksVideoTranscodePermission(t *testing.T) {
@@ -3369,6 +3403,127 @@ func TestFindAlternateFile_DoesNotCrossEdition(t *testing.T) {
}
}
func TestClampEncodedTargetResolution(t *testing.T) {
tests := []struct {
name string
requested string
source string
want string
}{
{
name: "clamps 2160p to 1080p",
requested: transcodeResolution2160p,
source: transcodeResolution1080p,
want: transcodeResolution1080p,
},
{
name: "keeps lower target",
requested: transcodeResolution720p,
source: transcodeResolution1080p,
want: transcodeResolution720p,
},
{
name: "keeps equal target",
requested: transcodeResolution1080p,
source: transcodeResolution1080p,
want: transcodeResolution1080p,
},
{
name: "keeps empty target",
requested: "",
source: transcodeResolution1080p,
want: "",
},
{
name: "keeps unknown target",
requested: "unrecognized-target",
source: transcodeResolution1080p,
want: "unrecognized-target",
},
{
name: "keeps target for unknown source",
requested: transcodeResolution2160p,
source: "native",
want: transcodeResolution2160p,
},
{
name: "supports low tiers",
requested: transcodeResolution480p,
source: transcodeResolution420p,
want: transcodeResolution420p,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if got := clampEncodedTargetResolution(tt.requested, tt.source); got != tt.want {
t.Fatalf(
"clampEncodedTargetResolution(%q, %q) = %q, want %q",
tt.requested,
tt.source,
got,
tt.want,
)
}
})
}
}
func TestTranscodeResolutionHeight(t *testing.T) {
tests := []struct {
resolution string
wantHeight int
wantKnown bool
}{
{resolution: transcodeResolution2160p, wantHeight: 2160, wantKnown: true},
{resolution: transcodeResolution1080p, wantHeight: 1080, wantKnown: true},
{resolution: transcodeResolution720p, wantHeight: 720, wantKnown: true},
{resolution: transcodeResolution480p, wantHeight: 480, wantKnown: true},
{resolution: transcodeResolution420p, wantHeight: 420, wantKnown: true},
{resolution: transcodeResolution328p, wantHeight: 328, wantKnown: true},
{resolution: "unrecognized-tier", wantHeight: 0, wantKnown: false},
}
for _, tt := range tests {
t.Run(tt.resolution, func(t *testing.T) {
gotHeight, gotKnown := transcodeResolutionHeight(tt.resolution)
if gotHeight != tt.wantHeight || gotKnown != tt.wantKnown {
t.Fatalf(
"transcodeResolutionHeight(%q) = (%d, %t), want (%d, %t)",
tt.resolution,
gotHeight,
gotKnown,
tt.wantHeight,
tt.wantKnown,
)
}
})
}
}
func TestResolutionRank(t *testing.T) {
tests := []struct {
resolution string
want int
}{
{resolution: transcodeResolution2160p, want: 4},
{resolution: transcodeResolution1080p, want: 3},
{resolution: transcodeResolution720p, want: 2},
{resolution: transcodeResolution480p, want: 1},
{resolution: transcodeResolution420p, want: 0},
{resolution: transcodeResolution328p, want: 0},
{resolution: "unrecognized-tier", want: 0},
}
for _, tt := range tests {
t.Run(tt.resolution, func(t *testing.T) {
if got := resolutionRank(tt.resolution); got != tt.want {
t.Fatalf("resolutionRank(%q) = %d, want %d", tt.resolution, got, tt.want)
}
})
}
}
func TestAlignedSeekSeconds(t *testing.T) {
tests := []struct {
name string