mirror of
https://github.com/screentinker/screentinker.git
synced 2026-08-17 07:34:15 -06:00
Merge #237: default a playlist item's duration to the video's own length
This commit is contained in:
commit
63b9de1329
|
|
@ -673,7 +673,11 @@ paths:
|
||||||
widget_id: { type: string }
|
widget_id: { type: string }
|
||||||
zone_id: { type: string }
|
zone_id: { type: string }
|
||||||
sort_order: { type: integer }
|
sort_order: { type: integer }
|
||||||
duration_sec: { type: integer }
|
duration_sec:
|
||||||
|
type: integer
|
||||||
|
description: >
|
||||||
|
Omit to let the server choose: video content defaults to the clip's own
|
||||||
|
length (rounded up to a whole second), anything else to 10s.
|
||||||
responses:
|
responses:
|
||||||
'201': { description: Created playlist item. }
|
'201': { description: Created playlist item. }
|
||||||
/playlists/{id}/items/reorder:
|
/playlists/{id}/items/reorder:
|
||||||
|
|
@ -815,7 +819,11 @@ paths:
|
||||||
content_id: { type: string }
|
content_id: { type: string }
|
||||||
widget_id: { type: string }
|
widget_id: { type: string }
|
||||||
zone_id: { type: string }
|
zone_id: { type: string }
|
||||||
duration_sec: { type: integer }
|
duration_sec:
|
||||||
|
type: integer
|
||||||
|
description: >
|
||||||
|
Omit to let the server choose: video content defaults to the clip's own
|
||||||
|
length (rounded up to a whole second), anything else to 10s.
|
||||||
sort_order: { type: integer }
|
sort_order: { type: integer }
|
||||||
responses:
|
responses:
|
||||||
'201': { description: Created item. }
|
'201': { description: Created item. }
|
||||||
|
|
@ -1242,7 +1250,11 @@ paths:
|
||||||
required: [content_id]
|
required: [content_id]
|
||||||
properties:
|
properties:
|
||||||
content_id: { type: string }
|
content_id: { type: string }
|
||||||
duration_sec: { type: integer }
|
duration_sec:
|
||||||
|
type: integer
|
||||||
|
description: >
|
||||||
|
Omit to let the server choose: video content defaults to the clip's own
|
||||||
|
length (rounded up to a whole second), anything else to 10s.
|
||||||
responses:
|
responses:
|
||||||
'200': { description: '{ success, devices_updated }.' }
|
'200': { description: '{ success, devices_updated }.' }
|
||||||
/groups/{id}/assign-playlist:
|
/groups/{id}/assign-playlist:
|
||||||
|
|
|
||||||
|
|
@ -1561,7 +1561,9 @@ async function setupPlaylistActions(device) {
|
||||||
</div>
|
</div>
|
||||||
<div class="form-group">
|
<div class="form-group">
|
||||||
<label>${t('device.assign.duration_label')}</label>
|
<label>${t('device.assign.duration_label')}</label>
|
||||||
<input type="number" id="assignDuration" class="input" value="10" min="1" max="3600">
|
<!-- max is the server's absurd-duration ceiling (12h): a feature-length clip
|
||||||
|
pre-filled from its own length must not land in an out-of-range field. -->
|
||||||
|
<input type="number" id="assignDuration" class="input" value="10" min="1" max="43200">
|
||||||
</div>
|
</div>
|
||||||
<!-- Tabs -->
|
<!-- Tabs -->
|
||||||
<div style="display:flex;gap:0;border-bottom:1px solid var(--border);margin-bottom:12px">
|
<div style="display:flex;gap:0;border-bottom:1px solid var(--border);margin-bottom:12px">
|
||||||
|
|
@ -1572,7 +1574,7 @@ async function setupPlaylistActions(device) {
|
||||||
<!-- Media grid -->
|
<!-- Media grid -->
|
||||||
<div class="assign-content-grid" id="assignMedia">
|
<div class="assign-content-grid" id="assignMedia">
|
||||||
${content.map(c => `
|
${content.map(c => `
|
||||||
<div class="assign-content-item" data-content-id="${c.id}" data-type="content">
|
<div class="assign-content-item" data-content-id="${c.id}" data-type="content" data-duration="${Number(c.duration_sec) > 0 ? Math.ceil(c.duration_sec) : ''}">
|
||||||
${c.thumbnail_path
|
${c.thumbnail_path
|
||||||
? `<img data-auth-src="/api/content/${c.id}/thumbnail" alt="">`
|
? `<img data-auth-src="/api/content/${c.id}/thumbnail" alt="">`
|
||||||
: c.remote_url
|
: c.remote_url
|
||||||
|
|
@ -1632,12 +1634,21 @@ async function setupPlaylistActions(device) {
|
||||||
|
|
||||||
let selectedId = null;
|
let selectedId = null;
|
||||||
let selectedType = null;
|
let selectedType = null;
|
||||||
|
// #237: this modal always SENDS a duration, so the server's "default a video to its own
|
||||||
|
// length" rule can never fire here — the field has to carry the clip length itself, or
|
||||||
|
// picking a 32s video silently assigns a 10s item that cuts off. Anything the operator
|
||||||
|
// typed is theirs and is never overwritten.
|
||||||
|
const durInput = modal.querySelector('#assignDuration');
|
||||||
|
let durationTouched = false;
|
||||||
|
durInput?.addEventListener('input', () => { durationTouched = true; });
|
||||||
modal.querySelectorAll('.assign-content-item').forEach(item => {
|
modal.querySelectorAll('.assign-content-item').forEach(item => {
|
||||||
item.addEventListener('click', () => {
|
item.addEventListener('click', () => {
|
||||||
modal.querySelectorAll('.assign-content-item').forEach(i => i.classList.remove('selected'));
|
modal.querySelectorAll('.assign-content-item').forEach(i => i.classList.remove('selected'));
|
||||||
item.classList.add('selected');
|
item.classList.add('selected');
|
||||||
selectedId = item.dataset.contentId;
|
selectedId = item.dataset.contentId;
|
||||||
selectedType = item.dataset.type;
|
selectedType = item.dataset.type;
|
||||||
|
const clip = parseInt(item.dataset.duration || '', 10);
|
||||||
|
if (durInput && !durationTouched) durInput.value = clip > 0 ? clip : 10;
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -219,7 +219,10 @@ export function render(container) {
|
||||||
await fetch(`/api/assignments/device/${pairedDeviceId}`, {
|
await fetch(`/api/assignments/device/${pairedDeviceId}`, {
|
||||||
method: 'POST',
|
method: 'POST',
|
||||||
headers: { 'Content-Type': 'application/json', Authorization: `Bearer ${token}` },
|
headers: { 'Content-Type': 'application/json', Authorization: `Bearer ${token}` },
|
||||||
body: JSON.stringify({ content_id: content.id, duration_sec: 10 })
|
// #237: no duration_sec — onboarding never asked the operator for one, and a
|
||||||
|
// hardcoded 10 cut their very first video off mid-play. Omitting it lets the
|
||||||
|
// server default to the clip's own length (still 10s for a photo).
|
||||||
|
body: JSON.stringify({ content_id: content.id })
|
||||||
});
|
});
|
||||||
} catch {}
|
} catch {}
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -735,7 +735,12 @@ async function showAddItemModal(playlistId, opts = {}) {
|
||||||
list.innerHTML = filtered.map(item => {
|
list.innerHTML = filtered.map(item => {
|
||||||
const isWidget = activeTab === 'widgets';
|
const isWidget = activeTab === 'widgets';
|
||||||
const name = item.filename || item.name || t('common.unknown');
|
const name = item.filename || item.name || t('common.unknown');
|
||||||
const sub = isWidget ? (item.widget_type || t('playlist.item_widget')) : (item.mime_type || '');
|
// #237: the server gives a video item the clip's own length instead of the 10s default.
|
||||||
|
// Show that length here so the duration the item lands with is something the operator
|
||||||
|
// saw coming, rather than a number that appears in the list after the fact.
|
||||||
|
const clipSec = !isWidget && Number(item.duration_sec) > 0 ? Math.ceil(item.duration_sec) : 0;
|
||||||
|
const clip = clipSec ? ` · ${Math.floor(clipSec / 60)}:${String(clipSec % 60).padStart(2, '0')}` : '';
|
||||||
|
const sub = isWidget ? (item.widget_type || t('playlist.item_widget')) : ((item.mime_type || '') + clip);
|
||||||
const thumb = item.thumbnail_path ? `/api/content/${esc(item.id)}/thumbnail` : null;
|
const thumb = item.thumbnail_path ? `/api/content/${esc(item.id)}/thumbnail` : null;
|
||||||
return `
|
return `
|
||||||
<div class="add-item-row" data-id="${esc(item.id)}" data-type="${isWidget ? 'widget' : 'content'}" style="display:flex;align-items:center;gap:12px;padding:10px;border-radius:var(--radius);cursor:pointer;transition:background 0.1s">
|
<div class="add-item-row" data-id="${esc(item.id)}" data-type="${isWidget ? 'widget' : 'content'}" style="display:flex;align-items:center;gap:12px;padding:10px;border-radius:var(--radius);cursor:pointer;transition:background 0.1s">
|
||||||
|
|
|
||||||
39
server/lib/item-duration.js
Normal file
39
server/lib/item-duration.js
Normal file
|
|
@ -0,0 +1,39 @@
|
||||||
|
'use strict';
|
||||||
|
|
||||||
|
// #237: a video added to a playlist got the flat 10s default, so a 32s clip was cut off at
|
||||||
|
// 10s unless the operator looked up the runtime and typed it in — per item, every time. The
|
||||||
|
// content row already carries the probed length, so that becomes the default. Shared by
|
||||||
|
// every playlist_items insert path (dashboard, device assign, group assign, agency portal,
|
||||||
|
// schedules, public API) because the operator sees one product, not six routes.
|
||||||
|
|
||||||
|
const DEFAULT_ITEM_DURATION = 10;
|
||||||
|
|
||||||
|
// A probe that reports longer than this is a broken container (streams and truncated files
|
||||||
|
// report absurd or near-infinite lengths), not a clip anyone means to schedule — honoring it
|
||||||
|
// would park a display on one item for days with no obvious cause. 12h.
|
||||||
|
const MAX_CONTENT_DURATION = 43200;
|
||||||
|
|
||||||
|
// The content's own length, or null when there isn't a trustworthy one: images, widgets,
|
||||||
|
// YouTube and remote-URL rows carry no duration, and a failed ffprobe leaves null/0. Rounded
|
||||||
|
// UP so a 31.7s clip gets 32 and not a 31 that clips the tail; whole seconds because the
|
||||||
|
// Android player reads duration_sec with optInt (a fractional value silently truncates) and
|
||||||
|
// an operator expects to see a round number in the duration box.
|
||||||
|
function contentDefaultDuration(content) {
|
||||||
|
const n = Number(content && content.duration_sec);
|
||||||
|
if (!Number.isFinite(n) || n <= 0 || n > MAX_CONTENT_DURATION) return null;
|
||||||
|
return Math.max(1, Math.ceil(n));
|
||||||
|
}
|
||||||
|
|
||||||
|
// The duration to STORE on a new playlist_item. An explicit operator value always wins; the
|
||||||
|
// content's own length is only a default for when none was given.
|
||||||
|
//
|
||||||
|
// Anything that isn't a usable number falls back rather than reaching the DB: a duration of
|
||||||
|
// 0 (or NaN, from a client that sent a string) makes the players schedule a 0ms advance,
|
||||||
|
// which self-loops and black-screens the TV (#widget zero-duration loop).
|
||||||
|
function resolveItemDuration(requested, content) {
|
||||||
|
const n = Number(requested);
|
||||||
|
if (Number.isFinite(n) && n >= 1) return Math.floor(n);
|
||||||
|
return contentDefaultDuration(content) ?? DEFAULT_ITEM_DURATION;
|
||||||
|
}
|
||||||
|
|
||||||
|
module.exports = { resolveItemDuration, contentDefaultDuration, DEFAULT_ITEM_DURATION, MAX_CONTENT_DURATION };
|
||||||
|
|
@ -17,6 +17,7 @@ const { listDesignatedPlaylists, isZonedPlaylist, folderSubtree } = require('../
|
||||||
const { listLayoutGeometry } = require('../lib/agency-layouts');
|
const { listLayoutGeometry } = require('../lib/agency-layouts');
|
||||||
const { publishPlaylist } = require('./playlists'); // #73: shared publish path for auto-publish
|
const { publishPlaylist } = require('./playlists'); // #73: shared publish path for auto-publish
|
||||||
const { isConfigured } = require('../services/email'); // #73: gate digest enqueue on SMTP being set
|
const { isConfigured } = require('../services/email'); // #73: gate digest enqueue on SMTP being set
|
||||||
|
const { resolveItemDuration } = require('../lib/item-duration');
|
||||||
|
|
||||||
const TIME_RE = /^([01]\d|2[0-3]):[0-5]\d$/;
|
const TIME_RE = /^([01]\d|2[0-3]):[0-5]\d$/;
|
||||||
const DATE_RE = /^\d{4}-\d{2}-\d{2}$/;
|
const DATE_RE = /^\d{4}-\d{2}-\d{2}$/;
|
||||||
|
|
@ -112,7 +113,9 @@ router.post('/playlists/:playlistId/items', (req, res) => {
|
||||||
if (duration_sec != null && (typeof duration_sec !== 'number' || duration_sec < 1)) {
|
if (duration_sec != null && (typeof duration_sec !== 'number' || duration_sec < 1)) {
|
||||||
return res.status(400).json({ error: 'duration_sec must be a positive integer' });
|
return res.status(400).json({ error: 'duration_sec must be a positive integer' });
|
||||||
}
|
}
|
||||||
duration_sec = duration_sec || content.duration_sec || 10;
|
// #237: the raw content duration used to be stored as probed (31.7s), and a fraction is
|
||||||
|
// truncated by the Android player's optInt read — so round to whole seconds here too.
|
||||||
|
duration_sec = resolveItemDuration(duration_sec, content);
|
||||||
|
|
||||||
const sd = start_date ?? null, ed = end_date ?? null;
|
const sd = start_date ?? null, ed = end_date ?? null;
|
||||||
for (const [k, v] of [['start_date', sd], ['end_date', ed]]) {
|
for (const [k, v] of [['start_date', sd], ['end_date', ed]]) {
|
||||||
|
|
|
||||||
|
|
@ -8,21 +8,15 @@ const { PLATFORM_ROLES, ELEVATED_ROLES } = require('../middleware/auth');
|
||||||
// though playlists.js itself isn't yet workspace-filtered.
|
// though playlists.js itself isn't yet workspace-filtered.
|
||||||
const { accessContext } = require('../lib/tenancy');
|
const { accessContext } = require('../lib/tenancy');
|
||||||
const { zoneInLayout } = require('../lib/zone-validate');
|
const { zoneInLayout } = require('../lib/zone-validate');
|
||||||
|
// #237 + #widget zero-duration loop: one place decides what duration a new item gets —
|
||||||
|
// explicit value, else the content's own length, else the 10s default (and never a 0).
|
||||||
|
const { resolveItemDuration } = require('../lib/item-duration');
|
||||||
|
|
||||||
// Mark playlist as draft (called after any item mutation)
|
// Mark playlist as draft (called after any item mutation)
|
||||||
function markDraft(playlistId) {
|
function markDraft(playlistId) {
|
||||||
db.prepare("UPDATE playlists SET status = 'draft', updated_at = strftime('%s','now') WHERE id = ?").run(playlistId);
|
db.prepare("UPDATE playlists SET status = 'draft', updated_at = strftime('%s','now') WHERE id = ?").run(playlistId);
|
||||||
}
|
}
|
||||||
|
|
||||||
// Hardening (#widget zero-duration loop): a non-positive duration — especially
|
|
||||||
// duration_sec=0 on a widget — makes the player schedule a 0ms auto-advance, which
|
|
||||||
// self-loops and black-screens the TV. Never STORE a bad value: floor any missing/
|
|
||||||
// invalid/<1 duration to the 10s default so it can't reach a device.
|
|
||||||
function normalizeDuration(v) {
|
|
||||||
const n = Number(v);
|
|
||||||
return Number.isFinite(n) && n >= 1 ? Math.floor(n) : 10;
|
|
||||||
}
|
|
||||||
|
|
||||||
// Hardening (#zone-orphan): a zone_id only renders if it belongs to the layout the
|
// Hardening (#zone-orphan): a zone_id only renders if it belongs to the layout the
|
||||||
// device is actually showing. Assigning a zone from a DIFFERENT layout (e.g. after a
|
// device is actually showing. Assigning a zone from a DIFFERENT layout (e.g. after a
|
||||||
// layout switch/duplicate) creates an item that the players can't place. We CLEAR a
|
// layout switch/duplicate) creates an item that the players can't place. We CLEAR a
|
||||||
|
|
@ -105,17 +99,20 @@ router.post('/device/:deviceId', (req, res) => {
|
||||||
const access = checkDeviceAccess(req, res, 'deviceId', true);
|
const access = checkDeviceAccess(req, res, 'deviceId', true);
|
||||||
if (!access) return;
|
if (!access) return;
|
||||||
const { content_id, widget_id, zone_id, sort_order } = req.body;
|
const { content_id, widget_id, zone_id, sort_order } = req.body;
|
||||||
const duration_sec = normalizeDuration(req.body.duration_sec);
|
|
||||||
|
|
||||||
if (!content_id && !widget_id) return res.status(400).json({ error: 'content_id or widget_id required' });
|
if (!content_id && !widget_id) return res.status(400).json({ error: 'content_id or widget_id required' });
|
||||||
|
|
||||||
|
let content = null;
|
||||||
if (content_id) {
|
if (content_id) {
|
||||||
const content = db.prepare('SELECT id, workspace_id FROM content WHERE id = ?').get(content_id);
|
content = db.prepare('SELECT id, workspace_id, duration_sec FROM content WHERE id = ?').get(content_id);
|
||||||
if (!content) return res.status(404).json({ error: 'Content not found' });
|
if (!content) return res.status(404).json({ error: 'Content not found' });
|
||||||
if (content.workspace_id && content.workspace_id !== access.device.workspace_id) {
|
if (content.workspace_id && content.workspace_id !== access.device.workspace_id) {
|
||||||
return res.status(403).json({ error: 'Content is not in this device\'s workspace' });
|
return res.status(403).json({ error: 'Content is not in this device\'s workspace' });
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
// #237: pushing a video straight at a display is the shortest path in the product, so it
|
||||||
|
// has to default to the clip's length too — not the 10s that cut it off mid-play.
|
||||||
|
const duration_sec = resolveItemDuration(req.body.duration_sec, content);
|
||||||
if (widget_id) {
|
if (widget_id) {
|
||||||
const widget = db.prepare('SELECT id, workspace_id FROM widgets WHERE id = ?').get(widget_id);
|
const widget = db.prepare('SELECT id, workspace_id FROM widgets WHERE id = ?').get(widget_id);
|
||||||
if (!widget) return res.status(404).json({ error: 'Widget not found' });
|
if (!widget) return res.status(404).json({ error: 'Widget not found' });
|
||||||
|
|
@ -234,7 +231,7 @@ router.put('/:id', (req, res) => {
|
||||||
const values = [];
|
const values = [];
|
||||||
|
|
||||||
if (sort_order !== undefined) { updates.push('sort_order = ?'); values.push(sort_order); }
|
if (sort_order !== undefined) { updates.push('sort_order = ?'); values.push(sort_order); }
|
||||||
if (duration_sec !== undefined) { updates.push('duration_sec = ?'); values.push(normalizeDuration(duration_sec)); }
|
if (duration_sec !== undefined) { updates.push('duration_sec = ?'); values.push(resolveItemDuration(duration_sec, null)); }
|
||||||
// zone_id can be null (clear the zone) - treat undefined as "no change",
|
// zone_id can be null (clear the zone) - treat undefined as "no change",
|
||||||
// any other value (including null) as "write this".
|
// any other value (including null) as "write this".
|
||||||
if (zone_id !== undefined) {
|
if (zone_id !== undefined) {
|
||||||
|
|
@ -337,7 +334,7 @@ router.post('/device/:deviceId/copy-to/:targetDeviceId', (req, res) => {
|
||||||
|
|
||||||
const transaction = db.transaction(() => {
|
const transaction = db.transaction(() => {
|
||||||
sourceItems.forEach((a, i) => {
|
sourceItems.forEach((a, i) => {
|
||||||
stmt.run(targetPlaylistId, a.content_id, a.widget_id, a.zone_id || null, maxOrder + i + 1, normalizeDuration(a.duration_sec));
|
stmt.run(targetPlaylistId, a.content_id, a.widget_id, a.zone_id || null, maxOrder + i + 1, resolveItemDuration(a.duration_sec, null));
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
transaction();
|
transaction();
|
||||||
|
|
|
||||||
|
|
@ -10,6 +10,7 @@ const { accessContext } = require('../lib/tenancy');
|
||||||
const { requireScope } = require('../middleware/apiToken');
|
const { requireScope } = require('../middleware/apiToken');
|
||||||
const { resolveSyncBackend, BACKENDS } = require('../lib/sync-backend');
|
const { resolveSyncBackend, BACKENDS } = require('../lib/sync-backend');
|
||||||
const playerCapabilities = require('../lib/player-capabilities');
|
const playerCapabilities = require('../lib/player-capabilities');
|
||||||
|
const { resolveItemDuration } = require('../lib/item-duration');
|
||||||
|
|
||||||
const VALID_COLOR = /^#[0-9A-Fa-f]{6}$/;
|
const VALID_COLOR = /^#[0-9A-Fa-f]{6}$/;
|
||||||
const ALLOWED_COMMANDS = [
|
const ALLOWED_COMMANDS = [
|
||||||
|
|
@ -326,11 +327,14 @@ router.post('/:id/assign-content', requireGroupWrite, (req, res) => {
|
||||||
|
|
||||||
// Verify content lives in the same workspace as the group (or is a
|
// Verify content lives in the same workspace as the group (or is a
|
||||||
// platform-template row).
|
// platform-template row).
|
||||||
const content = db.prepare('SELECT id, workspace_id FROM content WHERE id = ?').get(content_id);
|
const content = db.prepare('SELECT id, workspace_id, duration_sec FROM content WHERE id = ?').get(content_id);
|
||||||
if (!content) return res.status(404).json({ error: 'Content not found' });
|
if (!content) return res.status(404).json({ error: 'Content not found' });
|
||||||
if (content.workspace_id && content.workspace_id !== req.group.workspace_id) {
|
if (content.workspace_id && content.workspace_id !== req.group.workspace_id) {
|
||||||
return res.status(403).json({ error: 'Content is not in this group\'s workspace' });
|
return res.status(403).json({ error: 'Content is not in this group\'s workspace' });
|
||||||
}
|
}
|
||||||
|
// #237: same duration rule as every other add path — a video defaults to its own length,
|
||||||
|
// and here a wrong default would be wrong on every screen in the group at once.
|
||||||
|
const itemDuration = resolveItemDuration(duration_sec, content);
|
||||||
|
|
||||||
const members = db.prepare('SELECT device_id FROM device_group_members WHERE group_id = ?').all(req.params.id);
|
const members = db.prepare('SELECT device_id FROM device_group_members WHERE group_id = ?').all(req.params.id);
|
||||||
|
|
||||||
|
|
@ -339,7 +343,7 @@ router.post('/:id/assign-content', requireGroupWrite, (req, res) => {
|
||||||
const playlistId = ensureDevicePlaylist(m.device_id, req.user.id);
|
const playlistId = ensureDevicePlaylist(m.device_id, req.user.id);
|
||||||
const max = db.prepare('SELECT COALESCE(MAX(sort_order),0)+1 as next FROM playlist_items WHERE playlist_id = ?').get(playlistId);
|
const max = db.prepare('SELECT COALESCE(MAX(sort_order),0)+1 as next FROM playlist_items WHERE playlist_id = ?').get(playlistId);
|
||||||
db.prepare('INSERT INTO playlist_items (playlist_id, content_id, sort_order, duration_sec) VALUES (?, ?, ?, ?)')
|
db.prepare('INSERT INTO playlist_items (playlist_id, content_id, sort_order, duration_sec) VALUES (?, ?, ?, ?)')
|
||||||
.run(playlistId, content_id, max.next, duration_sec || 10);
|
.run(playlistId, content_id, max.next, itemDuration);
|
||||||
markDraft(playlistId);
|
markDraft(playlistId);
|
||||||
}
|
}
|
||||||
});
|
});
|
||||||
|
|
|
||||||
|
|
@ -7,6 +7,7 @@ const config = require('../config');
|
||||||
// Phase 2.2k: workspace-aware access. requirePlaylistOwnership is replaced
|
// Phase 2.2k: workspace-aware access. requirePlaylistOwnership is replaced
|
||||||
// by read/write helpers gated on the playlist's workspace_id.
|
// by read/write helpers gated on the playlist's workspace_id.
|
||||||
const { accessContext } = require('../lib/tenancy');
|
const { accessContext } = require('../lib/tenancy');
|
||||||
|
const { resolveItemDuration } = require('../lib/item-duration');
|
||||||
|
|
||||||
// Re-probe video duration with ffprobe if content.duration_sec is missing
|
// Re-probe video duration with ffprobe if content.duration_sec is missing
|
||||||
async function probeAndUpdateDuration(content) {
|
async function probeAndUpdateDuration(content) {
|
||||||
|
|
@ -450,18 +451,20 @@ router.post('/:id/items', requirePlaylistWrite, async (req, res) => {
|
||||||
return res.status(400).json({ error: 'duration_sec must be a positive integer' });
|
return res.status(400).json({ error: 'duration_sec must be a positive integer' });
|
||||||
}
|
}
|
||||||
|
|
||||||
|
let content = null;
|
||||||
if (content_id) {
|
if (content_id) {
|
||||||
const content = db.prepare('SELECT id, workspace_id, duration_sec, mime_type, filepath FROM content WHERE id = ?').get(content_id);
|
content = db.prepare('SELECT id, workspace_id, duration_sec, mime_type, filepath FROM content WHERE id = ?').get(content_id);
|
||||||
if (!content) return res.status(404).json({ error: 'Content not found' });
|
if (!content) return res.status(404).json({ error: 'Content not found' });
|
||||||
if (content.workspace_id && content.workspace_id !== req.playlist.workspace_id) {
|
if (content.workspace_id && content.workspace_id !== req.playlist.workspace_id) {
|
||||||
return res.status(403).json({ error: 'Content is not in this playlist\'s workspace' });
|
return res.status(403).json({ error: 'Content is not in this playlist\'s workspace' });
|
||||||
}
|
}
|
||||||
|
// Rows ingested before the probe existed (or while ffprobe was missing) have no stored
|
||||||
|
// duration; re-probe once so this add still gets the clip's length, and backfill the row.
|
||||||
if (duration_sec === undefined || duration_sec === null) {
|
if (duration_sec === undefined || duration_sec === null) {
|
||||||
const contentDur = await probeAndUpdateDuration(content);
|
content.duration_sec = await probeAndUpdateDuration(content);
|
||||||
if (contentDur) duration_sec = Math.ceil(contentDur);
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
if (duration_sec === undefined || duration_sec === null) duration_sec = 10;
|
duration_sec = resolveItemDuration(duration_sec, content);
|
||||||
if (widget_id) {
|
if (widget_id) {
|
||||||
const widget = db.prepare('SELECT id, workspace_id FROM widgets WHERE id = ?').get(widget_id);
|
const widget = db.prepare('SELECT id, workspace_id FROM widgets WHERE id = ?').get(widget_id);
|
||||||
if (!widget) return res.status(404).json({ error: 'Widget not found' });
|
if (!widget) return res.status(404).json({ error: 'Widget not found' });
|
||||||
|
|
|
||||||
|
|
@ -9,6 +9,7 @@ const { db } = require('../db/database');
|
||||||
// payload refs with no ownership check at all (only the target was checked).
|
// payload refs with no ownership check at all (only the target was checked).
|
||||||
const { accessContext } = require('../lib/tenancy');
|
const { accessContext } = require('../lib/tenancy');
|
||||||
const { effectiveDeviceTz } = require('../lib/device-timezone');
|
const { effectiveDeviceTz } = require('../lib/device-timezone');
|
||||||
|
const { resolveItemDuration } = require('../lib/item-duration');
|
||||||
|
|
||||||
// Helper: build the expanded schedule query for a device (device-level + group-level)
|
// Helper: build the expanded schedule query for a device (device-level + group-level)
|
||||||
function getDeviceSchedulesQuery() {
|
function getDeviceSchedulesQuery() {
|
||||||
|
|
@ -271,12 +272,14 @@ router.post('/', (req, res) => {
|
||||||
// playlist. That reuses the whole published/assign/push pipeline as-is.
|
// playlist. That reuses the whole published/assign/push pipeline as-is.
|
||||||
let effectivePlaylistId = playlist_id || null;
|
let effectivePlaylistId = playlist_id || null;
|
||||||
if (!effectivePlaylistId && content_id) {
|
if (!effectivePlaylistId && content_id) {
|
||||||
const c = db.prepare('SELECT filename FROM content WHERE id = ?').get(content_id);
|
const c = db.prepare('SELECT filename, duration_sec FROM content WHERE id = ?').get(content_id);
|
||||||
const genId = uuidv4();
|
const genId = uuidv4();
|
||||||
db.prepare('INSERT INTO playlists (id, name, workspace_id, user_id, status) VALUES (?, ?, ?, ?, ?)')
|
db.prepare('INSERT INTO playlists (id, name, workspace_id, user_id, status) VALUES (?, ?, ?, ?, ?)')
|
||||||
.run(genId, `Scheduled: ${(c && c.filename) || 'item'}`, targetWorkspaceId, req.user.id, 'published');
|
.run(genId, `Scheduled: ${(c && c.filename) || 'item'}`, targetWorkspaceId, req.user.id, 'published');
|
||||||
db.prepare('INSERT INTO playlist_items (playlist_id, content_id, sort_order, duration_sec) VALUES (?, ?, 0, 10)')
|
// #237: a scheduled video is a one-item playlist with nowhere to edit the duration, so a
|
||||||
.run(genId, content_id);
|
// flat 10 here would cut the clip off with no visible knob to fix it.
|
||||||
|
db.prepare('INSERT INTO playlist_items (playlist_id, content_id, sort_order, duration_sec) VALUES (?, ?, 0, ?)')
|
||||||
|
.run(genId, content_id, resolveItemDuration(null, c));
|
||||||
// Publish through the shared path rather than hand-rolling the snapshot: players read
|
// Publish through the shared path rather than hand-rolling the snapshot: players read
|
||||||
// denormalized fields (filename, mime_type, filepath, remote_url, schedules...) out of
|
// denormalized fields (filename, mime_type, filepath, remote_url, schedules...) out of
|
||||||
// published_snapshot, and duplicating that shape here would rot the moment it changes.
|
// published_snapshot, and duplicating that shape here would rot the moment it changes.
|
||||||
|
|
|
||||||
189
server/test/item-duration-default.test.js
Normal file
189
server/test/item-duration-default.test.js
Normal file
|
|
@ -0,0 +1,189 @@
|
||||||
|
'use strict';
|
||||||
|
|
||||||
|
// #237: adding a video to a playlist gave the item the flat 10s default, so a 32s clip was
|
||||||
|
// cut off at 10s unless the operator looked up the runtime and typed it in. These tests pin
|
||||||
|
// the rule that replaces it — and the ways it must NOT misfire:
|
||||||
|
// - a video defaults to its own probed length, rounded UP to whole seconds;
|
||||||
|
// - content with no trustworthy duration (image, widget, YouTube/remote, failed probe)
|
||||||
|
// keeps the 10s default, and a failed re-probe must not break the add;
|
||||||
|
// - an explicit duration from the operator always wins;
|
||||||
|
// - degenerate stored values (0, negative, NaN, absurd) never reach a device — a 0 makes
|
||||||
|
// the players schedule a 0ms advance, which self-loops and black-screens the TV.
|
||||||
|
// Unit-tests the shared helper (every insert path uses it), then proves it end to end on the
|
||||||
|
// two routes an operator actually clicks: playlist add and assign-to-display.
|
||||||
|
|
||||||
|
const { test, before, after } = require('node:test');
|
||||||
|
const assert = require('node:assert/strict');
|
||||||
|
const { spawn } = require('node:child_process');
|
||||||
|
const path = require('node:path');
|
||||||
|
const os = require('node:os');
|
||||||
|
const fs = require('node:fs');
|
||||||
|
const crypto = require('node:crypto');
|
||||||
|
const Database = require('better-sqlite3');
|
||||||
|
|
||||||
|
const { resolveItemDuration, contentDefaultDuration, DEFAULT_ITEM_DURATION, MAX_CONTENT_DURATION } = require('../lib/item-duration');
|
||||||
|
|
||||||
|
const { freePort } = require('./helpers/free-port');
|
||||||
|
let PORT, BASE;
|
||||||
|
const DATA_DIR = path.join(os.tmpdir(), 'st-itemdur-test-' + crypto.randomBytes(4).toString('hex'));
|
||||||
|
const LOG = path.join(os.tmpdir(), 'st-itemdur-' + crypto.randomBytes(4).toString('hex') + '.log');
|
||||||
|
const PW = 'Passw0rd123';
|
||||||
|
let proc, db;
|
||||||
|
const S = {};
|
||||||
|
|
||||||
|
async function jfetch(p, opts = {}) {
|
||||||
|
const res = await fetch(BASE + p, opts);
|
||||||
|
let body = null; try { body = await res.json(); } catch { /* non-JSON */ }
|
||||||
|
return { status: res.status, body };
|
||||||
|
}
|
||||||
|
const auth = (tok) => ({ headers: { Authorization: 'Bearer ' + tok, 'Content-Type': 'application/json' } });
|
||||||
|
const post = (tok, obj) => ({ method: 'POST', ...auth(tok), body: JSON.stringify(obj || {}) });
|
||||||
|
|
||||||
|
// ---------------------------------------------------------------- unit: the shared helper
|
||||||
|
|
||||||
|
test('a video defaults to its own length, rounded UP (31.7s -> 32, never a clipped 31)', () => {
|
||||||
|
assert.equal(resolveItemDuration(undefined, { duration_sec: 31.7 }), 32);
|
||||||
|
assert.equal(resolveItemDuration(null, { duration_sec: 32 }), 32);
|
||||||
|
assert.equal(contentDefaultDuration({ duration_sec: 31.7 }), 32);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('content with no known duration keeps the 10s default', () => {
|
||||||
|
for (const c of [null, undefined, {}, { duration_sec: null }, { duration_sec: undefined }]) {
|
||||||
|
assert.equal(resolveItemDuration(undefined, c), DEFAULT_ITEM_DURATION);
|
||||||
|
assert.equal(contentDefaultDuration(c), null);
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
|
test('an explicit duration always wins over the content length', () => {
|
||||||
|
assert.equal(resolveItemDuration(5, { duration_sec: 31.7 }), 5);
|
||||||
|
assert.equal(resolveItemDuration(600, { duration_sec: 31.7 }), 600);
|
||||||
|
// whole seconds: the Android player reads duration_sec with optInt, which truncates
|
||||||
|
assert.equal(resolveItemDuration(12.9, null), 12);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('degenerate stored durations never produce a zero-length item', () => {
|
||||||
|
for (const bad of [0, -5, NaN, Infinity, 'abc', {}]) {
|
||||||
|
assert.equal(resolveItemDuration(undefined, { duration_sec: bad }), DEFAULT_ITEM_DURATION, `content duration ${String(bad)}`);
|
||||||
|
assert.equal(resolveItemDuration(bad, null), DEFAULT_ITEM_DURATION, `requested ${String(bad)}`);
|
||||||
|
}
|
||||||
|
// sub-second clip: honest about its length, but still at least a whole second
|
||||||
|
assert.equal(resolveItemDuration(undefined, { duration_sec: 0.4 }), 1);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('an absurd probe result is treated as a broken probe, not a schedule', () => {
|
||||||
|
assert.equal(resolveItemDuration(undefined, { duration_sec: 1e9 }), DEFAULT_ITEM_DURATION);
|
||||||
|
assert.equal(resolveItemDuration(undefined, { duration_sec: MAX_CONTENT_DURATION }), MAX_CONTENT_DURATION);
|
||||||
|
assert.equal(resolveItemDuration(undefined, { duration_sec: MAX_CONTENT_DURATION + 1 }), DEFAULT_ITEM_DURATION);
|
||||||
|
// an operator who deliberately types a very long dwell is still obeyed
|
||||||
|
assert.equal(resolveItemDuration(1e6, { duration_sec: 30 }), 1e6);
|
||||||
|
});
|
||||||
|
|
||||||
|
// -------------------------------------------------------------- end to end on the routes
|
||||||
|
|
||||||
|
before(async () => {
|
||||||
|
PORT = await freePort();
|
||||||
|
BASE = `http://127.0.0.1:${PORT}`;
|
||||||
|
const logFd = fs.openSync(LOG, 'w');
|
||||||
|
proc = spawn('node', ['server.js'], {
|
||||||
|
cwd: path.join(__dirname, '..'),
|
||||||
|
env: { ...process.env, DATA_DIR, SELF_HOSTED: 'true', PORT: String(PORT), NODE_ENV: 'test' },
|
||||||
|
stdio: ['ignore', logFd, logFd],
|
||||||
|
});
|
||||||
|
let up = false;
|
||||||
|
for (let i = 0; i < 80; i++) {
|
||||||
|
try { const r = await fetch(BASE + '/api/status'); if (r.ok) { up = true; break; } } catch { /* not yet */ }
|
||||||
|
await new Promise((r) => setTimeout(r, 250));
|
||||||
|
}
|
||||||
|
if (!up) throw new Error('server did not boot:\n' + fs.readFileSync(LOG, 'utf8').slice(-2000));
|
||||||
|
|
||||||
|
const reg = await jfetch('/api/auth/register', post(null, { email: 'd' + crypto.randomBytes(4).toString('hex') + '@x.local', password: PW }));
|
||||||
|
S.jwt = reg.body.token;
|
||||||
|
S.userId = reg.body.user.id;
|
||||||
|
S.wsA = reg.body.current_workspace_id;
|
||||||
|
|
||||||
|
const pl = await jfetch('/api/playlists', post(S.jwt, { name: 'dur-pl' }));
|
||||||
|
S.playlistId = pl.body.id;
|
||||||
|
|
||||||
|
// Seed the content library on one connection (FK off, as in mute.test.js) so each row can
|
||||||
|
// carry exactly the duration_sec shape under test without needing a real media file.
|
||||||
|
db = new Database(path.join(DATA_DIR, 'db', 'remote_display.db'), { timeout: 5000 });
|
||||||
|
db.pragma('foreign_keys = OFF');
|
||||||
|
const mkContent = (name, mime, duration, filepath = '') => {
|
||||||
|
const id = crypto.randomUUID();
|
||||||
|
db.prepare('INSERT INTO content (id, filename, filepath, mime_type, file_size, duration_sec, workspace_id) VALUES (?,?,?,?,0,?,?)')
|
||||||
|
.run(id, name, filepath, mime, duration, S.wsA);
|
||||||
|
return id;
|
||||||
|
};
|
||||||
|
S.cVideo = mkContent('clip.mp4', 'video/mp4', 31.7);
|
||||||
|
S.cVideoExplicit = mkContent('clip2.mp4', 'video/mp4', 31.7);
|
||||||
|
S.cImage = mkContent('poster.png', 'image/png', null);
|
||||||
|
S.cZero = mkContent('zero.mp4', 'video/mp4', 0);
|
||||||
|
S.cAbsurd = mkContent('absurd.mp4', 'video/mp4', 1e9);
|
||||||
|
// No stored duration AND a filepath that doesn't exist -> the re-probe runs and fails.
|
||||||
|
S.cUnprobeable = mkContent('missing.mp4', 'video/mp4', null, 'does-not-exist-' + crypto.randomBytes(4).toString('hex') + '.mp4');
|
||||||
|
S.cAssign = mkContent('assign.mp4', 'video/mp4', 44.2);
|
||||||
|
|
||||||
|
S.deviceId = crypto.randomUUID();
|
||||||
|
db.prepare('INSERT INTO devices (id, name, status, workspace_id, user_id) VALUES (?,?,?,?,?)')
|
||||||
|
.run(S.deviceId, 'DurDev', 'online', S.wsA, S.userId);
|
||||||
|
});
|
||||||
|
|
||||||
|
after(() => {
|
||||||
|
try { db?.close(); } catch { /* */ }
|
||||||
|
try { proc?.kill('SIGKILL'); } catch { /* */ }
|
||||||
|
for (const f of [DATA_DIR, LOG]) { try { fs.rmSync(f, { recursive: true, force: true }); } catch { /* */ } }
|
||||||
|
});
|
||||||
|
|
||||||
|
test('POST /playlists/:id/items gives a video the clip length', async () => {
|
||||||
|
const r = await jfetch(`/api/playlists/${S.playlistId}/items`, post(S.jwt, { content_id: S.cVideo }));
|
||||||
|
assert.equal(r.status, 201);
|
||||||
|
assert.equal(r.body.duration_sec, 32, '31.7s clip -> a 32s item, not the 10s default');
|
||||||
|
});
|
||||||
|
|
||||||
|
test('POST /playlists/:id/items honors an explicit duration on the same video', async () => {
|
||||||
|
const r = await jfetch(`/api/playlists/${S.playlistId}/items`, post(S.jwt, { content_id: S.cVideoExplicit, duration_sec: 5 }));
|
||||||
|
assert.equal(r.status, 201);
|
||||||
|
assert.equal(r.body.duration_sec, 5, 'the operator asked for 5s and gets 5s');
|
||||||
|
});
|
||||||
|
|
||||||
|
test('content with no duration (image) still gets the 10s default', async () => {
|
||||||
|
const r = await jfetch(`/api/playlists/${S.playlistId}/items`, post(S.jwt, { content_id: S.cImage }));
|
||||||
|
assert.equal(r.status, 201);
|
||||||
|
assert.equal(r.body.duration_sec, DEFAULT_ITEM_DURATION);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('a stored 0 or absurd duration falls back to the default instead of reaching a device', async () => {
|
||||||
|
const zero = await jfetch(`/api/playlists/${S.playlistId}/items`, post(S.jwt, { content_id: S.cZero }));
|
||||||
|
assert.equal(zero.status, 201);
|
||||||
|
assert.equal(zero.body.duration_sec, DEFAULT_ITEM_DURATION, 'a 0 would be a 0ms self-advancing loop');
|
||||||
|
|
||||||
|
const absurd = await jfetch(`/api/playlists/${S.playlistId}/items`, post(S.jwt, { content_id: S.cAbsurd }));
|
||||||
|
assert.equal(absurd.status, 201);
|
||||||
|
assert.equal(absurd.body.duration_sec, DEFAULT_ITEM_DURATION, 'a nonsense probe must not park the screen for years');
|
||||||
|
});
|
||||||
|
|
||||||
|
test('a failed re-probe degrades to the default — the add still succeeds', async () => {
|
||||||
|
const r = await jfetch(`/api/playlists/${S.playlistId}/items`, post(S.jwt, { content_id: S.cUnprobeable }));
|
||||||
|
assert.equal(r.status, 201, 'ffprobe failing (or missing) must not break adding an item');
|
||||||
|
assert.equal(r.body.duration_sec, DEFAULT_ITEM_DURATION);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('assigning a video straight to a display uses the clip length too, and it reaches the snapshot', async () => {
|
||||||
|
const r = await jfetch(`/api/assignments/device/${S.deviceId}`, post(S.jwt, { content_id: S.cAssign }));
|
||||||
|
assert.equal(r.status, 201);
|
||||||
|
assert.equal(r.body.duration_sec, 45, '44.2s clip -> a 45s item');
|
||||||
|
|
||||||
|
const explicit = await jfetch(`/api/assignments/device/${S.deviceId}`, post(S.jwt, { content_id: S.cVideo, duration_sec: 7 }));
|
||||||
|
assert.equal(explicit.status, 201);
|
||||||
|
assert.equal(explicit.body.duration_sec, 7, 'an explicit duration still wins on the assign path');
|
||||||
|
|
||||||
|
// The device only ever plays the published snapshot, so the defaulted value has to survive
|
||||||
|
// the publish — a correct playlist_items row that never reaches the player is no fix.
|
||||||
|
const playlistId = db.prepare('SELECT playlist_id FROM devices WHERE id = ?').get(S.deviceId).playlist_id;
|
||||||
|
const pub = await jfetch(`/api/playlists/${playlistId}/publish`, post(S.jwt, {}));
|
||||||
|
assert.equal(pub.status, 200);
|
||||||
|
const snap = JSON.parse(db.prepare('SELECT published_snapshot FROM playlists WHERE id = ?').get(playlistId).published_snapshot);
|
||||||
|
const item = snap.find((i) => i.content_id === S.cAssign);
|
||||||
|
assert.ok(item, 'the assigned clip is in the published snapshot');
|
||||||
|
assert.equal(item.duration_sec, 45, 'the device payload carries the clip length');
|
||||||
|
});
|
||||||
Loading…
Reference in a new issue