Merge bbe8fe164df8ba915d1cba17da9871c2fe435a53 into 74dc7678f1613aa38e3ab3466e6ed846ccb376dd

* ci: add migration timestamp validation to pipeline

Added a new migrations-lint job to the CI pipeline that validates new migrations
in PRs have timestamps greater than the latest migration on master. This prevents
migration ordering conflicts when multiple PRs add migrations concurrently.

The validation script compares timestamps using git ls-tree for master and
git diff for changed files, outputting GitHub Actions error annotations for
inline PR feedback. The job runs in parallel with other lint/test jobs and
only executes on pull requests.

* fix: only fail on new migrations, warn on modified ones

Updated the migration validation script to distinguish between new migrations
(which must have timestamps after master's latest) and modified existing
migrations (which only produce warnings). This allows fixing typos or issues
in existing migrations without breaking the pipeline.

* feat: post PR comment when migrations are modified

When modified migration files are detected, the script now posts a comment
on the PR to alert reviewers. This makes the warning more visible than just
the workflow annotation. The comment lists all modified migration files and
warns about potential issues for users who have already applied them.

* fix: deduplicate PR comments for modified migrations

Check if a warning comment already exists before posting a new one.
This prevents duplicate comments when multiple commits are pushed to the PR.

.github/workflows/pipeline.yml

@@ -159,6 +159,23 @@ jobs:
           done
       - run: ./.github/workflows/validate-translations.sh -v
 
+  migrations-lint:
+    name: Validate migrations
+    runs-on: ubuntu-latest
+    if: github.event_name == 'pull_request'
+    steps:
+      - uses: actions/checkout@v6
+        with:
+          fetch-depth: 0
+
+      - name: Fetch master branch
+        run: git fetch origin master
+
+      - name: Validate migration timestamps
+        env:
+          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+          PR_NUMBER: ${{ github.event.pull_request.number }}
+        run: ./.github/workflows/validate-migrations.sh
 
   check-push-enabled:
     name: Check Docker configuration
@@ -172,7 +189,8 @@ jobs:
 
   build:
     name: Build
-    needs: [js, go, go-lint, i18n-lint, git-version, check-push-enabled]
+    needs: [js, go, go-lint, i18n-lint, migrations-lint, git-version, check-push-enabled]
+    if: ${{ !failure() && !cancelled() }}
     strategy:
       matrix:
         platform: [ linux/amd64, linux/arm64, linux/arm/v5, linux/arm/v6, linux/arm/v7, linux/386, darwin/amd64, darwin/arm64, windows/amd64, windows/386 ]

.github/workflows/validate-migrations.sh

@@ -0,0 +1,103 @@
+#!/bin/bash
+# Validates that new migrations in a PR have timestamps greater than
+# the latest migration timestamp on the master branch.
+#
+# This prevents migration ordering conflicts when multiple PRs add migrations.
+# Modified existing migrations only produce warnings, not errors.
+
+set -e
+
+# Get the latest migration timestamp from master branch
+# Filter for files matching the pattern: 14-digit timestamp followed by _ and ending in .sql or .go
+MASTER_MIGRATIONS=$(git ls-tree --name-only origin/master -- db/migrations/ | grep -E '^db/migrations/[0-9]{14}_.*\.(sql|go)$' || true)
+
+if [ -z "$MASTER_MIGRATIONS" ]; then
+    echo "No migrations found on master branch"
+    exit 0
+fi
+
+MASTER_LATEST=$(echo "$MASTER_MIGRATIONS" | sed 's|db/migrations/||' | cut -c1-14 | sort -n | tail -1)
+
+# Get NEW migrations (added in this PR)
+NEW_MIGRATIONS=$(git diff --name-only --diff-filter=A origin/master -- db/migrations/ | grep -E '^db/migrations/[0-9]{14}_.*\.(sql|go)$' || true)
+
+# Get MODIFIED migrations (existing files that were changed)
+MODIFIED_MIGRATIONS=$(git diff --name-only --diff-filter=M origin/master -- db/migrations/ | grep -E '^db/migrations/[0-9]{14}_.*\.(sql|go)$' || true)
+
+if [ -z "$NEW_MIGRATIONS" ] && [ -z "$MODIFIED_MIGRATIONS" ]; then
+    echo "No new or modified migrations found in this PR"
+    exit 0
+fi
+
+echo "Latest migration on master: $MASTER_LATEST"
+
+HAS_ERRORS=false
+
+# Check NEW migrations - these MUST have valid timestamps (errors)
+if [ -n "$NEW_MIGRATIONS" ]; then
+    echo ""
+    echo "New migrations in this PR:"
+    for migration in $NEW_MIGRATIONS; do
+        TIMESTAMP=$(basename "$migration" | cut -c1-14)
+        echo "  - $migration (timestamp: $TIMESTAMP)"
+        
+        if [ "$TIMESTAMP" -le "$MASTER_LATEST" ]; then
+            echo "::error file=$migration::Migration timestamp $TIMESTAMP must be greater than latest master timestamp $MASTER_LATEST"
+            HAS_ERRORS=true
+        fi
+    done
+fi
+
+# Check MODIFIED migrations - only warn, don't fail
+if [ -n "$MODIFIED_MIGRATIONS" ]; then
+    echo ""
+    echo "Modified existing migrations in this PR:"
+    for migration in $MODIFIED_MIGRATIONS; do
+        TIMESTAMP=$(basename "$migration" | cut -c1-14)
+        echo "  - $migration (timestamp: $TIMESTAMP)"
+        echo "::warning file=$migration::Modifying existing migration files may cause issues for users who have already applied them"
+    done
+
+    # Post a PR review comment if running in GitHub Actions with a PR
+    if [ -n "$GITHUB_TOKEN" ] && [ -n "$GITHUB_REPOSITORY" ] && [ -n "$PR_NUMBER" ]; then
+        # Check if a warning comment already exists to avoid duplicates
+        EXISTING_COMMENT=$(curl -s \
+            -H "Authorization: token $GITHUB_TOKEN" \
+            -H "Accept: application/vnd.github.v3+json" \
+            "https://api.github.com/repos/$GITHUB_REPOSITORY/issues/$PR_NUMBER/comments" \
+            | jq -r '.[] | select(.body | startswith("### ⚠️ Modified Migration Files Detected")) | .id' | head -1)
+
+        if [ -n "$EXISTING_COMMENT" ]; then
+            echo "Warning comment already exists (comment ID: $EXISTING_COMMENT), skipping"
+        else
+            COMMENT_BODY="### ⚠️ Modified Migration Files Detected
+
+This PR modifies existing migration files that may have already been applied by users:
+
+$(for m in $MODIFIED_MIGRATIONS; do echo "- \`$m\`"; done)
+
+**Warning:** Modifying migrations that have already been applied can cause issues for existing users. Please ensure this change is intentional and consider the impact on users who have already run these migrations."
+
+            # Use GitHub API to post a PR comment
+            curl -s -X POST \
+                -H "Authorization: token $GITHUB_TOKEN" \
+                -H "Accept: application/vnd.github.v3+json" \
+                "https://api.github.com/repos/$GITHUB_REPOSITORY/issues/$PR_NUMBER/comments" \
+                -d "$(jq -n --arg body "$COMMENT_BODY" '{body: $body}')" > /dev/null
+            
+            echo "Posted PR comment about modified migrations"
+        fi
+    fi
+fi
+
+if [ "$HAS_ERRORS" = "true" ]; then
+    echo ""
+    echo "ERROR: One or more NEW migrations have timestamps that are not after the latest migration on master."
+    echo "Please regenerate the migration with a newer timestamp using:"
+    echo "  make migration-sql name=your_migration_name"
+    echo "  make migration-go name=your_migration_name"
+    exit 1
+fi
+
+echo ""
+echo "All migration timestamps are valid!"

core/media_streamer.go

@@ -130,58 +130,99 @@ func (s *Stream) EstimatedContentLength() int {
 	return int(s.mf.Duration * float32(s.bitRate) / 8 * 1024)
 }
 
-// TODO This function deserves some love (refactoring)
 func selectTranscodingOptions(ctx context.Context, ds model.DataStore, mf *model.MediaFile, reqFormat string, reqBitRate int) (format string, bitRate int) {
+	// Default case
 	format = "raw"
+	bitRate = 0
+
+	// If the client explicitly requests "raw"
+	// then always serve the original
 	if reqFormat == "raw" {
+		return format, bitRate
+	}
+
+	// If requested format matches the file’s suffix and
+	// no bitrate reduction is requested then
+	// stream the file without transcoding
+	if reqFormat == mf.Suffix && reqBitRate == 0 {
+		return format, mf.BitRate
+	}
+
+	targetFormat, targetBitRate := findTargetTranscodingOptions(ctx, mf, reqFormat, reqBitRate)
+
+	// If nothing was found then stream raw
+	if targetFormat == "" && targetBitRate == 0 {
 		return format, 0
 	}
-	if reqFormat == mf.Suffix && reqBitRate == 0 {
-		bitRate = mf.BitRate
-		return format, bitRate
+
+	t, err := ds.Transcoding(ctx).FindByFormat(targetFormat)
+	if err != nil {
+		// TODO: log error?
+		return format, 0
 	}
-	trc, hasDefault := request.TranscodingFrom(ctx)
-	var cFormat string
-	var cBitRate int
-	if reqFormat != "" {
-		cFormat = reqFormat
+
+	format = t.TargetFormat
+
+	// If no target bitrate was specified
+	// fall back to the transcoding’s configuration
+	// default bitrate
+	if targetBitRate == 0 {
+		bitRate = t.DefaultBitRate
 	} else {
-		if hasDefault {
-			cFormat = trc.TargetFormat
-			cBitRate = trc.DefaultBitRate
-			if p, ok := request.PlayerFrom(ctx); ok {
-				cBitRate = p.MaxBitRate
-			}
-		} else if reqBitRate > 0 && reqBitRate < mf.BitRate && conf.Server.DefaultDownsamplingFormat != "" {
-			// If no format is specified and no transcoding associated to the player, but a bitrate is specified,
-			// and there is no transcoding set for the player, we use the default downsampling format.
-			// But only if the requested bitRate is lower than the original bitRate.
-			log.Debug("Default Downsampling", "Using default downsampling format", conf.Server.DefaultDownsamplingFormat)
-			cFormat = conf.Server.DefaultDownsamplingFormat
-		}
-	}
-	if reqBitRate > 0 {
-		cBitRate = reqBitRate
-	}
-	if cBitRate == 0 && cFormat == "" {
-		return format, bitRate
-	}
-	t, err := ds.Transcoding(ctx).FindByFormat(cFormat)
-	if err == nil {
-		format = t.TargetFormat
-		if cBitRate != 0 {
-			bitRate = cBitRate
-		} else {
-			bitRate = t.DefaultBitRate
-		}
+		bitRate = targetBitRate
 	}
+
+	// If the final format is the same as the original
+	// and does not reduce bitrate
+	// there’s no reason to transcode
 	if format == mf.Suffix && bitRate >= mf.BitRate {
-		format = "raw"
-		bitRate = 0
+		return "raw", 0
 	}
+
 	return format, bitRate
 }
 
+func findTargetTranscodingOptions(ctx context.Context, mf *model.MediaFile, reqFormat string, reqBitRate int) (string, int) {
+	// If a format is requested use that
+	if reqFormat != "" {
+		return reqFormat, reqBitRate
+	}
+
+	// If a default transcoding configuration exists for this context
+	if trc, ok := request.TranscodingFrom(ctx); ok {
+		targetFormat := trc.TargetFormat
+		targetBitRate := trc.DefaultBitRate
+
+		// If a player is configured adjust bitrate based on
+		// user request or player limits
+		if p, hasPlayer := request.PlayerFrom(ctx); hasPlayer {
+			if reqBitRate > 0 {
+				targetBitRate = reqBitRate
+			} else if p.MaxBitRate > 0 {
+				targetBitRate = p.MaxBitRate
+			}
+		} else if reqBitRate > 0 {
+			targetBitRate = reqBitRate
+		}
+
+		return targetFormat, targetBitRate
+	}
+
+	// Use the default downsampling format the server is configured to but
+	// only if the requested bitrate is reduced
+	isBitrateReduced := reqBitRate > 0 && reqBitRate < mf.BitRate
+	hasDefaultDownsamplingFormat := conf.Server.DefaultDownsamplingFormat != ""
+
+	if isBitrateReduced && hasDefaultDownsamplingFormat {
+		log.Debug("Default Downsampling",
+			"Using default downsampling format",
+			conf.Server.DefaultDownsamplingFormat)
+		return conf.Server.DefaultDownsamplingFormat, reqBitRate
+	}
+
+	return "", 0
+}
+
 var (
 	onceTranscodingCache     sync.Once
 	instanceTranscodingCache TranscodingCache