diff --git a/android/app/src/main/java/com/remotedisplay/player/util/ImageLoader.kt b/android/app/src/main/java/com/remotedisplay/player/util/ImageLoader.kt index 66b3733..dc84453 100644 --- a/android/app/src/main/java/com/remotedisplay/player/util/ImageLoader.kt +++ b/android/app/src/main/java/com/remotedisplay/player/util/ImageLoader.kt @@ -3,7 +3,10 @@ package com.remotedisplay.player.util import android.content.Context import android.graphics.Bitmap import android.graphics.BitmapFactory +import android.graphics.Matrix +import android.media.ExifInterface import android.util.Log +import java.io.ByteArrayInputStream import java.io.File import java.net.URL @@ -13,6 +16,12 @@ import java.net.URL * A 4K source image on a 1080p screen ends up as 1920x1080, not 3840x2160 — keeps * the bitmap under ~8 MB instead of ~33 MB. * + * #170: BitmapFactory ignores EXIF orientation, so a portrait photo (landscape pixels + * tagged "rotate 90") would render sideways. We apply the EXIF rotation after decode. + * (The server-side ingest fix corrects stored dimensions + the thumbnail; this is the + * companion so the panel itself draws the photo upright — videos are already handled by + * ExoPlayer's rotation matrix.) + * * All exceptions, including OutOfMemoryError, return null so the caller can skip the * item rather than crashing the whole app. */ @@ -33,7 +42,12 @@ object ImageLoader { val opts = BitmapFactory.Options().apply { inSampleSize = calcSampleSize(bounds.outWidth, bounds.outHeight, maxW, maxH) } - BitmapFactory.decodeFile(file.absolutePath, opts) + val bmp = BitmapFactory.decodeFile(file.absolutePath, opts) ?: return null + // #170: honor EXIF orientation (read from the file; JPEGs from phones carry it). + val orientation = try { + ExifInterface(file.absolutePath).getAttributeInt(ExifInterface.TAG_ORIENTATION, ExifInterface.ORIENTATION_NORMAL) + } catch (e: Throwable) { ExifInterface.ORIENTATION_NORMAL } + applyExifOrientation(bmp, orientation) } catch (e: OutOfMemoryError) { Log.e(TAG, "OOM decoding ${file.name}: ${e.message}") null @@ -75,7 +89,12 @@ object ImageLoader { val opts = BitmapFactory.Options().apply { inSampleSize = calcSampleSize(bounds.outWidth, bounds.outHeight, maxW, maxH) } - BitmapFactory.decodeByteArray(bytes, 0, bytes.size, opts) + val bmp = BitmapFactory.decodeByteArray(bytes, 0, bytes.size, opts) ?: return null + // #170: honor EXIF orientation for remote images too (ExifInterface(stream) is API 24+). + val orientation = try { + ExifInterface(ByteArrayInputStream(bytes)).getAttributeInt(ExifInterface.TAG_ORIENTATION, ExifInterface.ORIENTATION_NORMAL) + } catch (e: Throwable) { ExifInterface.ORIENTATION_NORMAL } + applyExifOrientation(bmp, orientation) } catch (e: OutOfMemoryError) { Log.e(TAG, "OOM decoding ${bytes.size} bytes: ${e.message}") null @@ -91,4 +110,30 @@ object ImageLoader { while (srcW / sample > maxW || srcH / sample > maxH) sample *= 2 return sample } + + // #170: rotate/flip a just-decoded bitmap per its EXIF orientation so portrait photos + // render upright. Returns the input unchanged for NORMAL/UNDEFINED (no allocation), and + // recycles the source once a transformed copy is made. Falls back to the source on OOM + // rather than crashing — a sideways image beats a dead player. + private fun applyExifOrientation(bitmap: Bitmap, orientation: Int): Bitmap { + val m = Matrix() + when (orientation) { + ExifInterface.ORIENTATION_ROTATE_90 -> m.setRotate(90f) + ExifInterface.ORIENTATION_ROTATE_180 -> m.setRotate(180f) + ExifInterface.ORIENTATION_ROTATE_270 -> m.setRotate(270f) + ExifInterface.ORIENTATION_FLIP_HORIZONTAL -> m.setScale(-1f, 1f) + ExifInterface.ORIENTATION_FLIP_VERTICAL -> m.setScale(1f, -1f) + ExifInterface.ORIENTATION_TRANSPOSE -> { m.setRotate(90f); m.postScale(-1f, 1f) } + ExifInterface.ORIENTATION_TRANSVERSE -> { m.setRotate(270f); m.postScale(-1f, 1f) } + else -> return bitmap // NORMAL, UNDEFINED, or unknown -> leave as-is + } + return try { + val rotated = Bitmap.createBitmap(bitmap, 0, 0, bitmap.width, bitmap.height, m, true) + if (rotated != bitmap) bitmap.recycle() + rotated + } catch (e: OutOfMemoryError) { + Log.e(TAG, "OOM applying EXIF orientation $orientation: ${e.message}") + bitmap + } + } } diff --git a/server/lib/content-ingest.js b/server/lib/content-ingest.js index 83e68bc..f11c16a 100644 --- a/server/lib/content-ingest.js +++ b/server/lib/content-ingest.js @@ -11,6 +11,7 @@ const { v4: uuidv4 } = require('uuid'); const { db } = require('../db/database'); const config = require('../config'); const { sanitizeString } = require('../middleware/sanitize'); +const { videoDisplayDims, imageDisplayDims } = require('./media-orientation'); // Multer takes file.originalname from the multipart header, bypassing sanitizeBody, so // HTML-escape here (renders as text in every UI sink). .normalize('NFC') first: macOS @@ -32,10 +33,11 @@ async function ingestUploadedFile({ file, userId, workspaceId, folderId = null } if (file.mimetype.startsWith('image/')) { const sharp = require('sharp'); const metadata = await sharp(file.path).metadata(); - width = metadata.width; - height = metadata.height; + // #170: honor EXIF orientation so a portrait photo isn't stored as landscape. + ({ width, height } = imageDisplayDims(metadata)); thumbnailPath = `thumb_${filepath}`; await sharp(file.path) + .rotate() // #170: auto-orient per EXIF (and strip the tag) so the thumbnail matches .resize(config.thumbnailWidth) .jpeg({ quality: 70 }) .toFile(path.join(config.contentDir, thumbnailPath)); @@ -49,8 +51,9 @@ async function ingestUploadedFile({ file, userId, workspaceId, folderId = null } if (info.format?.duration) durationSec = parseFloat(info.format.duration); const videoStream = info.streams?.find(s => s.codec_type === 'video'); if (videoStream) { - width = videoStream.width; - height = videoStream.height; + // #170: honor the rotation/Display-Matrix so a portrait video isn't stored landscape. + // (ffmpeg auto-rotates the thumbnail below by default, so only the dims need fixing.) + ({ width, height } = videoDisplayDims(videoStream)); } thumbnailPath = `thumb_${filepath.replace(/\.[^.]+$/, '.jpg')}`; try { diff --git a/server/lib/media-orientation.js b/server/lib/media-orientation.js new file mode 100644 index 0000000..5eb130a --- /dev/null +++ b/server/lib/media-orientation.js @@ -0,0 +1,55 @@ +'use strict'; + +// #170: rotation-aware media dimensions. Both ffprobe (video) and sharp/EXIF (image) report the +// CODED/stored width×height, which ignore how the asset is meant to be DISPLAYED. A portrait +// phone video (coded 1920×1080 with a 90° display matrix) or a portrait photo (EXIF orientation +// 6) is therefore stored as LANDSCAPE, so the player renders it wrong-aspect and letterboxed +// (the blue-bar symptom). These pure helpers normalize to DISPLAY dimensions and are the single +// source of truth for both fresh ingest (content-ingest.js) and the backfill script. Unit-tested +// in test/media-orientation.test.js. + +// Does a rotation of `degrees` swap width and height? True only for odd quarter-turns. +function rotationSwapsWH(degrees) { + const d = (((Number(degrees) || 0) % 360) + 360) % 360; + return d === 90 || d === 270; +} + +// Rotation (degrees, normalized 0..359) a video should be displayed at. ffprobe exposes this two +// ways: the legacy `tags.rotate` string, and the modern Display Matrix side_data `rotation` (an +// int, often NEGATIVE, e.g. -90). Prefer the Display Matrix when present (what modern muxers +// write); fall back to the tag. The sign is irrelevant to the W/H swap decision either way. +function videoRotationDegrees(videoStream) { + if (!videoStream) return 0; + let deg = null; + const dm = (videoStream.side_data_list || []).find(s => /display\s*matrix/i.test(s.side_data_type || '')); + if (dm && dm.rotation != null && !Number.isNaN(parseInt(dm.rotation, 10))) { + deg = parseInt(dm.rotation, 10); + } else if (videoStream.tags && videoStream.tags.rotate != null && !Number.isNaN(parseInt(videoStream.tags.rotate, 10))) { + deg = parseInt(videoStream.tags.rotate, 10); + } + return (((Number(deg) || 0) % 360) + 360) % 360; +} + +// Display dimensions for a video stream, honoring rotation. Null-safe. +function videoDisplayDims(videoStream) { + if (!videoStream || videoStream.width == null || videoStream.height == null) return { width: null, height: null }; + return rotationSwapsWH(videoRotationDegrees(videoStream)) + ? { width: videoStream.height, height: videoStream.width } + : { width: videoStream.width, height: videoStream.height }; +} + +// EXIF orientation 5..8 encode a 90°/270° rotation (W/H swap); 1..4 do not. +function exifSwapsWH(orientation) { + const o = Number(orientation); + return o >= 5 && o <= 8; +} + +// Display dimensions for an image, honoring EXIF orientation. Null-safe. +function imageDisplayDims(metadata) { + if (!metadata || metadata.width == null || metadata.height == null) return { width: null, height: null }; + return exifSwapsWH(metadata.orientation) + ? { width: metadata.height, height: metadata.width } + : { width: metadata.width, height: metadata.height }; +} + +module.exports = { rotationSwapsWH, videoRotationDegrees, videoDisplayDims, exifSwapsWH, imageDisplayDims }; diff --git a/server/scripts/backfill-rotation-dims.js b/server/scripts/backfill-rotation-dims.js new file mode 100644 index 0000000..3a2e3e0 --- /dev/null +++ b/server/scripts/backfill-rotation-dims.js @@ -0,0 +1,80 @@ +'use strict'; + +// #170 backfill: correct stored width/height for already-uploaded portrait media that was +// ingested before the rotation-aware fix (coded dims, rotation ignored -> stored landscape). +// Re-probes each content file on disk, recomputes DISPLAY dims via lib/media-orientation, and +// (with --apply) updates the row + regenerates mis-oriented IMAGE thumbnails (old image thumbs +// were written without EXIF auto-orient; old video thumbs were already auto-rotated by ffmpeg). +// +// Idempotent: a second run finds nothing to change. Dry-run by default. +// node scripts/backfill-rotation-dims.js # report only +// node scripts/backfill-rotation-dims.js --apply # write corrections +// node scripts/backfill-rotation-dims.js --apply --limit 50 +// +// In Docker: docker exec node scripts/backfill-rotation-dims.js --apply + +const path = require('path'); +const fs = require('fs'); +const { execFileSync } = require('child_process'); +const { db } = require('../db/database'); +const config = require('../config'); +const { videoDisplayDims, imageDisplayDims } = require('../lib/media-orientation'); + +const APPLY = process.argv.includes('--apply'); +const limitArg = process.argv.indexOf('--limit'); +const LIMIT = limitArg >= 0 ? parseInt(process.argv[limitArg + 1], 10) || 0 : 0; + +function probeVideoDims(filePath) { + const out = execFileSync('ffprobe', ['-v', 'quiet', '-print_format', 'json', '-show_streams', filePath], { timeout: 15000 }).toString(); + const stream = (JSON.parse(out).streams || []).find(s => s.codec_type === 'video'); + return videoDisplayDims(stream); +} + +async function probeImageDims(filePath) { + const sharp = require('sharp'); + return imageDisplayDims(await sharp(filePath).metadata()); +} + +async function regenImageThumb(filePath, thumbName) { + const sharp = require('sharp'); + await sharp(filePath).rotate().resize(config.thumbnailWidth).jpeg({ quality: 70 }).toFile(path.join(config.contentDir, thumbName)); +} + +(async () => { + const rows = db.prepare( + `SELECT id, filepath, thumbnail_path, mime_type, width, height FROM content + WHERE filepath IS NOT NULL AND (mime_type LIKE 'image/%' OR mime_type LIKE 'video/%') + ${LIMIT ? 'LIMIT ' + LIMIT : ''}` + ).all(); + + let checked = 0, corrected = 0, missing = 0, probeErr = 0, thumbRegen = 0; + const changes = []; + + for (const row of rows) { + const filePath = path.join(config.contentDir, row.filepath); + if (!fs.existsSync(filePath)) { missing++; continue; } + checked++; + let dims; + try { + dims = row.mime_type.startsWith('image/') ? await probeImageDims(filePath) : probeVideoDims(filePath); + } catch (e) { probeErr++; continue; } + if (dims.width == null || dims.height == null) continue; + if (dims.width === row.width && dims.height === row.height) continue; // already correct + + corrected++; + changes.push({ id: row.id, from: `${row.width}x${row.height}`, to: `${dims.width}x${dims.height}`, mime: row.mime_type }); + if (APPLY) { + db.prepare('UPDATE content SET width = ?, height = ? WHERE id = ?').run(dims.width, dims.height, row.id); + // Regenerate the image thumbnail (video thumbs were already auto-rotated at ingest). + if (row.mime_type.startsWith('image/') && row.thumbnail_path) { + try { await regenImageThumb(filePath, row.thumbnail_path); thumbRegen++; } catch (e) { /* best-effort */ } + } + } + } + + console.log(`${APPLY ? 'APPLIED' : 'DRY-RUN'} — content rows: ${rows.length} | on-disk checked: ${checked} | missing file: ${missing} | probe errors: ${probeErr}`); + console.log(`dimension corrections ${APPLY ? 'written' : 'needed'}: ${corrected}${APPLY ? ` | image thumbnails regenerated: ${thumbRegen}` : ''}`); + for (const c of changes.slice(0, 40)) console.log(` ${c.id.slice(0, 8)} ${c.mime} ${c.from} -> ${c.to}`); + if (changes.length > 40) console.log(` … and ${changes.length - 40} more`); + if (!APPLY && corrected) console.log('\nRe-run with --apply to write these corrections.'); +})(); diff --git a/server/test/media-orientation.test.js b/server/test/media-orientation.test.js new file mode 100644 index 0000000..765d526 --- /dev/null +++ b/server/test/media-orientation.test.js @@ -0,0 +1,57 @@ +'use strict'; + +// #170: portrait media was stored with swapped W/H because ingest read CODED dimensions and +// ignored rotation (video Display-Matrix / rotate tag; image EXIF orientation) -> wrong aspect +// + blue letterbox bar on the player. These bites pin the display-dimension logic that fixes it. + +const { test } = require('node:test'); +const assert = require('node:assert/strict'); +const { + rotationSwapsWH, videoRotationDegrees, videoDisplayDims, exifSwapsWH, imageDisplayDims, +} = require('../lib/media-orientation'); + +test('#170 rotationSwapsWH: only odd quarter-turns swap', () => { + assert.equal(rotationSwapsWH(0), false); + assert.equal(rotationSwapsWH(90), true); + assert.equal(rotationSwapsWH(180), false); + assert.equal(rotationSwapsWH(270), true); + assert.equal(rotationSwapsWH(360), false); + assert.equal(rotationSwapsWH(-90), true); // normalizes to 270 + assert.equal(rotationSwapsWH(450), true); // normalizes to 90 + assert.equal(rotationSwapsWH(91), false); // not a clean quarter-turn + assert.equal(rotationSwapsWH(undefined), false); +}); + +test('#170 videoRotationDegrees: reads tag and Display Matrix, normalizes sign', () => { + assert.equal(videoRotationDegrees({ tags: { rotate: '90' } }), 90); + assert.equal(videoRotationDegrees({ tags: { rotate: '-90' } }), 270, 'negative tag normalized'); + assert.equal(videoRotationDegrees({ side_data_list: [{ side_data_type: 'Display Matrix', rotation: -90 }] }), 270); + assert.equal(videoRotationDegrees({ side_data_list: [{ side_data_type: 'Display Matrix', rotation: 90 }] }), 90); + // Display Matrix wins over a (possibly stale) legacy tag + assert.equal(videoRotationDegrees({ tags: { rotate: '0' }, side_data_list: [{ side_data_type: 'Display Matrix', rotation: -90 }] }), 270); + assert.equal(videoRotationDegrees({}), 0, 'no rotation info -> 0'); + assert.equal(videoRotationDegrees(null), 0); +}); + +test('#170 videoDisplayDims: portrait video (coded landscape + 90 rotation) reads portrait', () => { + assert.deepEqual( + videoDisplayDims({ width: 1920, height: 1080, side_data_list: [{ side_data_type: 'Display Matrix', rotation: -90 }] }), + { width: 1080, height: 1920 }, 'the blue-bar case: stored landscape -> display portrait'); + assert.deepEqual(videoDisplayDims({ width: 1920, height: 1080 }), { width: 1920, height: 1080 }, 'no rotation -> unchanged'); + assert.deepEqual(videoDisplayDims({ width: 1080, height: 1920, tags: { rotate: '180' } }), { width: 1080, height: 1920 }, '180 does not swap'); + assert.deepEqual(videoDisplayDims(null), { width: null, height: null }); + assert.deepEqual(videoDisplayDims({ width: null, height: null }), { width: null, height: null }); +}); + +test('#170 exifSwapsWH: EXIF 5..8 imply a quarter-turn', () => { + for (const o of [1, 2, 3, 4]) assert.equal(exifSwapsWH(o), false, `orientation ${o} no swap`); + for (const o of [5, 6, 7, 8]) assert.equal(exifSwapsWH(o), true, `orientation ${o} swaps`); + assert.equal(exifSwapsWH(undefined), false, 'no EXIF -> no swap'); +}); + +test('#170 imageDisplayDims: portrait photo with EXIF 6 reads portrait', () => { + assert.deepEqual(imageDisplayDims({ width: 4032, height: 3024, orientation: 6 }), { width: 3024, height: 4032 }); + assert.deepEqual(imageDisplayDims({ width: 3024, height: 4032, orientation: 1 }), { width: 3024, height: 4032 }); + assert.deepEqual(imageDisplayDims({ width: 3024, height: 4032 }), { width: 3024, height: 4032 }, 'no orientation tag -> unchanged'); + assert.deepEqual(imageDisplayDims(null), { width: null, height: null }); +});