mirror of
https://github.com/screentinker/screentinker.git
synced 2026-08-13 13:53:12 -06:00
* fix(content): rotation-aware media dimensions — portrait no longer stored landscape (#170) Ingest recorded CODED width/height and ignored rotation, so a portrait phone video (coded 1920x1080 + 90° Display-Matrix) or a portrait photo (EXIF orientation 6) was stored LANDSCAPE. The player then rendered it wrong-aspect and letterboxed — the "portrait content degraded + blue bar at the bottom" symptom in #170. The reporter's workaround (pre-rotate + mark Landscape) is exactly what this bug forces. - lib/media-orientation.js (new): pure, unit-tested display-dimension helpers = single source of truth for ingest AND the backfill. videoDisplayDims() reads the modern Display-Matrix side_data rotation (falls back to the legacy tags.rotate, sign-normalized); imageDisplayDims() honors EXIF orientation 5..8. Odd quarter-turns swap W/H. - lib/content-ingest.js: use the helpers for stored dims; add sharp .rotate() so image THUMBNAILS are auto-oriented too (video thumbs were already auto-rotated by ffmpeg). - scripts/backfill-rotation-dims.js (new): idempotent, dry-run-by-default maintenance to correct already-uploaded portrait media (re-probe -> fix dims -> regenerate image thumbs). - test/media-orientation.test.js: 5 bites (tag + Display-Matrix, sign/normalize, EXIF 5..8, the blue-bar landscape->portrait case, null-safety). Scopes #170 to its residual-on-1.9.4 issues; the 1.9.3 "never displays" slice was #162 + the remote_url-null download fix, already shipped in 1.9.4. The slow low-res/orientation- cycling first load is tracked separately in #170 pending repro data. Refs #170. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(android): honor EXIF orientation in ImageLoader so portrait photos render upright (#170) Completes the rotation-aware media fix on the PLAYER side. The server ingest fix (this branch) corrects stored dimensions + auto-orients the thumbnail, but the panel draws the full-res original via BitmapFactory, which ignores EXIF — so a portrait photo (landscape pixels tagged "rotate 90") still rendered sideways on the screen. QA root-cause pass on #170 caught this gap: the Android player reads no stored dims and applied no EXIF. ImageLoader now reads the EXIF orientation (from the file for cached content, from the byte stream for remote_url images — ExifInterface(stream) is API 24+, minSdk is 24) and rotates/ flips the decoded bitmap via a Matrix (all 8 orientations). NORMAL/UNDEFINED is a no-op (no extra allocation); a transformed copy recycles the source; OOM falls back to the source rather than crashing. Videos were already correct (ExoPlayer honors the rotation matrix). Verified: :app:compileDebugKotlin clean. Refs #170. Rides with the server rotation-dims fix on this branch. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
parent
2f3dd80881
commit
837f65e634
|
|
@ -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
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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 {
|
||||
|
|
|
|||
55
server/lib/media-orientation.js
Normal file
55
server/lib/media-orientation.js
Normal file
|
|
@ -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 };
|
||||
80
server/scripts/backfill-rotation-dims.js
Normal file
80
server/scripts/backfill-rotation-dims.js
Normal file
|
|
@ -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 <container> 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.');
|
||||
})();
|
||||
57
server/test/media-orientation.test.js
Normal file
57
server/test/media-orientation.test.js
Normal file
|
|
@ -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 });
|
||||
});
|
||||
Loading…
Reference in a new issue