feat: let a subscription carry clip bounds (closes #1049)

A subscription already carries every other download option and applies it to
each video it queues. Clip bounds were the one exception: 4f83174 added them
for one-off downloads and carved subscriptions out, rejecting the fields in the
subscribe route and stripping them in the UI before the request was built. So
the fields sat visible in the shared advanced-options panel while quietly doing
nothing for a subscription.

Carry them like the rest: two fields on SubscriptionInfo, threaded through the
check flow to add_entry. Stored records that predate the fields take the
defaults, since _from_stored filters by field name.

One thing does not carry over. parse_download_options reads a YouTube t=
timestamp from the URL and turns it into a clip start, which is right when you
paste a link to a moment in a video you want. A subscription URL is a channel
or a playlist, so a timestamp left on it says nothing about the videos that
feed will yield, and honouring it would silently truncate every future
download. The subscribe route therefore takes clip bounds only when the caller
sent the fields explicitly; the t= param is still stripped from the stored URL.

Worth knowing when using this: the range is a fixed offset applied to every
video, so it suits feeds with a consistent shape - a standing intro, a fixed
sponsor read - and will cut in the wrong place on a feed without one.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
Alex Shnitman
2026-08-16 09:25:03 +02:00
parent 3444b1605b
commit aac9c63a36
8 changed files with 201 additions and 37 deletions
+15 -3
View File
@@ -917,9 +917,6 @@ async def subscribe(request):
raise web.HTTPBadRequest(reason='check_interval_minutes must be an integer') from exc raise web.HTTPBadRequest(reason='check_interval_minutes must be an integer') from exc
if cic < 1: if cic < 1:
raise web.HTTPBadRequest(reason='check_interval_minutes must be at least 1') raise web.HTTPBadRequest(reason='check_interval_minutes must be at least 1')
if o.get('clip_start') is not None or o.get('clip_end') is not None:
raise web.HTTPBadRequest(reason='clip options are not supported for subscriptions')
try: try:
skip_subscriber_only = coerce_optional_bool( skip_subscriber_only = coerce_optional_bool(
post.get('skip_subscriber_only'), post.get('skip_subscriber_only'),
@@ -929,6 +926,19 @@ async def subscribe(request):
except ValueError as exc: except ValueError as exc:
raise web.HTTPBadRequest(reason=str(exc)) from exc raise web.HTTPBadRequest(reason=str(exc)) from exc
# A t= timestamp in the URL means "start playing here" and parse_download_options
# turns it into a clip start, which is right for a one-off download of that video.
# A subscription URL is a channel or playlist, so a timestamp left on it says
# nothing about the videos it will yield — honour clip fields only when the
# caller supplied them explicitly, rather than silently clipping every future
# download. The t= param is still stripped from the stored URL.
clip_given = (
_clip_field_provided_in_post(post.get('clip_start'))
or _clip_field_provided_in_post(post.get('clip_end'))
)
sub_clip_start = o['clip_start'] if clip_given else None
sub_clip_end = o['clip_end'] if clip_given else None
result = await submgr.add_subscription( result = await submgr.add_subscription(
o['url'], o['url'],
check_interval_minutes=cic, check_interval_minutes=cic,
@@ -948,6 +958,8 @@ async def subscribe(request):
ytdl_options_overrides=o['ytdl_options_overrides'], ytdl_options_overrides=o['ytdl_options_overrides'],
title_regex=post.get('title_regex'), title_regex=post.get('title_regex'),
skip_subscriber_only=skip_subscriber_only, skip_subscriber_only=skip_subscriber_only,
clip_start=sub_clip_start,
clip_end=sub_clip_end,
) )
return web.Response(text=serializer.encode(result)) return web.Response(text=serializer.encode(result))
+23
View File
@@ -189,6 +189,13 @@ class SubscriptionInfo:
ytdl_options_overrides: dict[str, Any] = field(default_factory=dict) ytdl_options_overrides: dict[str, Any] = field(default_factory=dict)
title_regex: str = "" title_regex: str = ""
skip_subscriber_only: bool = False skip_subscriber_only: bool = False
# A fixed range applied to every video the subscription downloads. Only
# sensible for channels with a consistent format (a standing intro, a fixed
# sponsor read); left unset, videos download whole. Old stored records
# predate these fields and take the defaults — _from_stored filters by
# field name, so nothing needs migrating.
clip_start: Optional[float] = None
clip_end: Optional[float] = None
last_checked: Optional[float] = None last_checked: Optional[float] = None
seen_ids: list[str] = field(default_factory=list) seen_ids: list[str] = field(default_factory=list)
error: Optional[str] = None error: Optional[str] = None
@@ -211,6 +218,8 @@ class SubscriptionInfo:
"folder": self.folder, "folder": self.folder,
"title_regex": self.title_regex, "title_regex": self.title_regex,
"skip_subscriber_only": self.skip_subscriber_only, "skip_subscriber_only": self.skip_subscriber_only,
"clip_start": self.clip_start,
"clip_end": self.clip_end,
"last_checked": self.last_checked, "last_checked": self.last_checked,
"seen_count": len(self.seen_ids), "seen_count": len(self.seen_ids),
"error": self.error, "error": self.error,
@@ -240,6 +249,8 @@ def _subscription_to_record(sub: SubscriptionInfo) -> dict[str, Any]:
"ytdl_options_overrides": sub.ytdl_options_overrides, "ytdl_options_overrides": sub.ytdl_options_overrides,
"title_regex": sub.title_regex, "title_regex": sub.title_regex,
"skip_subscriber_only": sub.skip_subscriber_only, "skip_subscriber_only": sub.skip_subscriber_only,
"clip_start": sub.clip_start,
"clip_end": sub.clip_end,
"last_checked": sub.last_checked, "last_checked": sub.last_checked,
"seen_ids": list(sub.seen_ids), "seen_ids": list(sub.seen_ids),
"error": sub.error, "error": sub.error,
@@ -474,6 +485,8 @@ class SubscriptionManager:
subtitle_mode: str, subtitle_mode: str,
ytdl_options_presets: Optional[list[str]] = None, ytdl_options_presets: Optional[list[str]] = None,
ytdl_options_overrides: Optional[dict[str, Any]] = None, ytdl_options_overrides: Optional[dict[str, Any]] = None,
clip_start: Optional[float] = None,
clip_end: Optional[float] = None,
) -> tuple[list[str], list[str]]: ) -> tuple[list[str], list[str]]:
queued_ids: list[str] = [] queued_ids: list[str] = []
queue_errors: list[str] = [] queue_errors: list[str] = []
@@ -504,6 +517,8 @@ class SubscriptionManager:
subtitle_mode, subtitle_mode,
presets, presets,
ytdl_options_overrides, ytdl_options_overrides,
clip_start,
clip_end,
) )
if isinstance(result, dict) and result.get("status") == "error": if isinstance(result, dict) and result.get("status") == "error":
msg = str(result.get("msg") or f"Queueing failed for {vurl}") msg = str(result.get("msg") or f"Queueing failed for {vurl}")
@@ -593,6 +608,8 @@ class SubscriptionManager:
ytdl_options_overrides: Optional[dict[str, Any]] = None, ytdl_options_overrides: Optional[dict[str, Any]] = None,
title_regex: Any = None, title_regex: Any = None,
skip_subscriber_only: Any = None, skip_subscriber_only: Any = None,
clip_start: Optional[float] = None,
clip_end: Optional[float] = None,
) -> dict: ) -> dict:
url = self._normalize_url(url) url = self._normalize_url(url)
if not url: if not url:
@@ -679,6 +696,8 @@ class SubscriptionManager:
ytdl_options_overrides=dict(ytdl_options_overrides or {}), ytdl_options_overrides=dict(ytdl_options_overrides or {}),
title_regex=title_regex_stored, title_regex=title_regex_stored,
skip_subscriber_only=skip_so, skip_subscriber_only=skip_so,
clip_start=clip_start,
clip_end=clip_end,
last_checked=time.time(), last_checked=time.time(),
seen_ids=list(dict.fromkeys(all_ids)), seen_ids=list(dict.fromkeys(all_ids)),
error=None, error=None,
@@ -930,6 +949,8 @@ class SubscriptionManager:
dl_ytdl_overrides = dict(cur.ytdl_options_overrides) dl_ytdl_overrides = dict(cur.ytdl_options_overrides)
dl_title_regex = cur.title_regex or "" dl_title_regex = cur.title_regex or ""
dl_skip_subscriber_only = bool(cur.skip_subscriber_only) dl_skip_subscriber_only = bool(cur.skip_subscriber_only)
dl_clip_start = cur.clip_start
dl_clip_end = cur.clip_end
new_entries: list[dict] = [] new_entries: list[dict] = []
for ent in entries: for ent in entries:
@@ -994,6 +1015,8 @@ class SubscriptionManager:
subtitle_mode=dl_submode, subtitle_mode=dl_submode,
ytdl_options_presets=dl_ytdl_presets, ytdl_options_presets=dl_ytdl_presets,
ytdl_options_overrides=dl_ytdl_overrides, ytdl_options_overrides=dl_ytdl_overrides,
clip_start=dl_clip_start,
clip_end=dl_clip_end,
) )
log.info( log.info(
"Subscription check finished for %s: %d new, %d filtered, %d subscriber_skipped, %d queued, %d failed", "Subscription check finished for %s: %d new, %d filtered, %d subscriber_skipped, %d queued, %d failed",
+57 -4
View File
@@ -349,17 +349,70 @@ async def test_add_passes_clip_bounds_to_queue(mock_dqueue):
@pytest.mark.asyncio @pytest.mark.asyncio
async def test_subscribe_rejects_clip_options(mock_dqueue, monkeypatch): async def test_subscribe_passes_clip_bounds(mock_dqueue, monkeypatch):
monkeypatch.setattr(main.submgr, "add_subscription", AsyncMock()) """Issue #1049: a subscription's options apply to every future download, and
clip bounds were the one option carved out of that."""
monkeypatch.setattr(main.submgr, "add_subscription", AsyncMock(return_value={"status": "ok"}))
req = _json_request( req = _json_request(
{ {
**_valid_video_add_body(clip_start="10"), **_valid_video_add_body(clip_start="2:26", clip_end="3:24"),
"check_interval_minutes": 60, "check_interval_minutes": 60,
} }
) )
resp = await main.subscribe(req)
assert resp.status == 200
kwargs = main.submgr.add_subscription.await_args.kwargs
assert kwargs["clip_start"] == pytest.approx(146.0)
assert kwargs["clip_end"] == pytest.approx(204.0)
@pytest.mark.asyncio
async def test_subscribe_without_clip_fields_stores_none(mock_dqueue, monkeypatch):
monkeypatch.setattr(main.submgr, "add_subscription", AsyncMock(return_value={"status": "ok"}))
req = _json_request({**_valid_video_add_body(), "check_interval_minutes": 60})
await main.subscribe(req)
kwargs = main.submgr.add_subscription.await_args.kwargs
assert kwargs["clip_start"] is None
assert kwargs["clip_end"] is None
@pytest.mark.asyncio
async def test_subscribe_ignores_t_param_in_url(mock_dqueue, monkeypatch):
"""A t= timestamp means "start here" for a one-off download of that video.
On a channel or playlist URL it says nothing about the videos it yields, so
it must not silently clip every future download."""
monkeypatch.setattr(main.submgr, "add_subscription", AsyncMock(return_value={"status": "ok"}))
body = _valid_video_add_body()
# t= is only honoured on YouTube hosts, so this must be one to exercise it.
body["url"] = "https://www.youtube.com/@somechannel?t=90"
req = _json_request({**body, "check_interval_minutes": 60})
await main.subscribe(req)
kwargs = main.submgr.add_subscription.await_args.kwargs
assert kwargs["clip_start"] is None
assert kwargs["clip_end"] is None
# The timestamp is still stripped from the URL that gets stored.
assert "t=90" not in main.submgr.add_subscription.await_args.args[0]
@pytest.mark.asyncio
async def test_subscribe_explicit_clip_wins_over_t_param(mock_dqueue, monkeypatch):
monkeypatch.setattr(main.submgr, "add_subscription", AsyncMock(return_value={"status": "ok"}))
body = _valid_video_add_body(clip_start="30")
body["url"] = "https://www.youtube.com/@somechannel?t=90"
req = _json_request({**body, "check_interval_minutes": 60})
await main.subscribe(req)
kwargs = main.submgr.add_subscription.await_args.kwargs
assert kwargs["clip_start"] == pytest.approx(30.0)
@pytest.mark.asyncio
async def test_subscribe_still_rejects_clips_for_non_media(mock_dqueue, monkeypatch):
monkeypatch.setattr(main.submgr, "add_subscription", AsyncMock(return_value={"status": "ok"}))
body = _valid_video_add_body(clip_start="10")
body["download_type"] = "thumbnail"
req = _json_request({**body, "check_interval_minutes": 60})
with pytest.raises(web.HTTPBadRequest): with pytest.raises(web.HTTPBadRequest):
await main.subscribe(req) await main.subscribe(req)
main.submgr.add_subscription.assert_not_awaited()
@pytest.mark.asyncio @pytest.mark.asyncio
+70
View File
@@ -409,6 +409,76 @@ class SubscriptionPersistenceTests(unittest.IsolatedAsyncioTestCase):
self.assertEqual(sub.seen_ids[:2], ["v2", "v1"]) self.assertEqual(sub.seen_ids[:2], ["v2", "v1"])
self.assertEqual([entry["webpage_url"] for entry, _, _ in queue.entries], ["https://example.com/v2"]) self.assertEqual([entry["webpage_url"] for entry, _, _ in queue.entries], ["https://example.com/v2"])
async def test_check_now_applies_subscription_clip_bounds(self):
"""Issue #1049: clip bounds were the one download option a subscription
could not carry, so they must reach every entry it queues."""
with tempfile.TemporaryDirectory() as tmp:
queue = _Queue()
mgr = SubscriptionManager(_Config(tmp), queue, _Notifier())
with patch(
"subscriptions.extract_flat_playlist",
side_effect=[
(
{"_type": "channel", "title": "Channel"},
[{"id": "v1", "title": "One", "webpage_url": "https://example.com/v1"}],
),
(
{"_type": "channel", "title": "Channel"},
[
{"id": "v2", "title": "Two", "webpage_url": "https://example.com/v2"},
{"id": "v1", "title": "One", "webpage_url": "https://example.com/v1"},
],
),
],
):
result = await mgr.add_subscription(
"https://example.com/channel",
check_interval_minutes=60,
download_type="video",
codec="auto",
format="any",
quality="best",
folder="",
custom_name_prefix="",
auto_start=True,
playlist_item_limit=0,
split_by_chapters=False,
chapter_template="",
subtitle_language="en",
subtitle_mode="prefer_manual",
clip_start=30.0,
clip_end=90.0,
)
sub_id = result["subscription"]["id"]
self.assertEqual(mgr.get(sub_id).clip_start, 30.0)
self.assertEqual(mgr.get(sub_id).clip_end, 90.0)
await mgr.check_now([sub_id])
self.assertEqual(len(queue.entries), 1)
_entry, args, _kwargs = queue.entries[0]
# add_entry(entry, download_type, ..., ytdl_options_overrides, clip_start, clip_end)
self.assertEqual(args[-2], 30.0)
self.assertEqual(args[-1], 90.0)
async def test_clip_bounds_survive_reload_and_default_to_none(self):
with tempfile.TemporaryDirectory() as tmp:
cfg = _Config(tmp)
queue = _Queue()
mgr = SubscriptionManager(cfg, queue, _Notifier())
sub_id = await self._add_one_subscription(mgr)
# Records written before these fields existed simply take the defaults.
self.assertIsNone(mgr.get(sub_id).clip_start)
self.assertIsNone(mgr.get(sub_id).clip_end)
mgr.get(sub_id).clip_start = 12.5
async with mgr._lock:
mgr._save_locked()
reloaded = SubscriptionManager(cfg, _Queue(), _Notifier())
self.assertEqual(reloaded.get(sub_id).clip_start, 12.5)
self.assertIsNone(reloaded.get(sub_id).clip_end)
async def test_check_now_queues_subscriber_only_when_skip_disabled(self): async def test_check_now_queues_subscriber_only_when_skip_disabled(self):
with tempfile.TemporaryDirectory() as tmp: with tempfile.TemporaryDirectory() as tmp:
queue = _Queue() queue = _Queue()
+5 -3
View File
@@ -254,7 +254,9 @@ describe('App', () => {
expect(payload.skipSubscriberOnly).toBe(true); expect(payload.skipSubscriberOnly).toBe(true);
}); });
it('omits clip fields from subscribe payload', () => { it('passes clip fields through to the subscribe payload', () => {
// #1049: a subscription's options apply to all its future downloads, and
// clip bounds used to be stripped out on the way.
const fixture = TestBed.createComponent(App); const fixture = TestBed.createComponent(App);
const app = fixture.componentInstance; const app = fixture.componentInstance;
const subs = TestBed.inject(SubscriptionsService) as unknown as SubscriptionsServiceStub; const subs = TestBed.inject(SubscriptionsService) as unknown as SubscriptionsServiceStub;
@@ -264,8 +266,8 @@ describe('App', () => {
app.addSubscription(); app.addSubscription();
expect(subs.subscribeCalls.length).toBe(1); expect(subs.subscribeCalls.length).toBe(1);
const payload = subs.subscribeCalls[0] as Record<string, unknown>; const payload = subs.subscribeCalls[0] as Record<string, unknown>;
expect('clipStart' in payload).toBe(false); expect(payload['clipStart']).toBe('1:00');
expect('clipEnd' in payload).toBe(false); expect(payload['clipEnd']).toBe('2:00');
}); });
it('buildAddPayload includes clip times', () => { it('buildAddPayload includes clip times', () => {
+1 -5
View File
@@ -601,14 +601,10 @@ export class App implements AfterViewInit, OnInit, OnDestroy {
if (!this.validateYtdlOptionsOverrides(payload.ytdlOptionsOverrides)) { if (!this.validateYtdlOptionsOverrides(payload.ytdlOptionsOverrides)) {
return; return;
} }
// Subscriptions do not support clip ranges (backend rejects clip fields).
const { clipStart: _clipStart, clipEnd: _clipEnd, ...subscribeBase } = payload;
void _clipStart;
void _clipEnd;
this.subscribeInProgress = true; this.subscribeInProgress = true;
this.subscriptionsSvc this.subscriptionsSvc
.subscribe({ .subscribe({
...subscribeBase, ...payload,
checkIntervalMinutes: this.checkIntervalMinutes, checkIntervalMinutes: this.checkIntervalMinutes,
titleRegex: tr, titleRegex: tr,
skipSubscriberOnly: this.skipSubscriberOnly, skipSubscriberOnly: this.skipSubscriberOnly,
+2
View File
@@ -11,6 +11,8 @@ export interface SubscriptionRow {
folder: string; folder: string;
title_regex?: string; title_regex?: string;
skip_subscriber_only?: boolean; skip_subscriber_only?: boolean;
clip_start?: number | null;
clip_end?: number | null;
last_checked: number | null; last_checked: number | null;
seen_count: number; seen_count: number;
error: string | null; error: string | null;
+10 -4
View File
@@ -81,8 +81,7 @@ export class SubscriptionsService {
} }
subscribe(payload: SubscribePayload) { subscribe(payload: SubscribePayload) {
return this.http const body: Record<string, unknown> = {
.post<Status>('subscribe', {
url: payload.url, url: payload.url,
download_type: payload.downloadType, download_type: payload.downloadType,
codec: payload.codec, codec: payload.codec,
@@ -101,8 +100,15 @@ export class SubscriptionsService {
check_interval_minutes: payload.checkIntervalMinutes, check_interval_minutes: payload.checkIntervalMinutes,
title_regex: payload.titleRegex, title_regex: payload.titleRegex,
skip_subscriber_only: payload.skipSubscriberOnly, skip_subscriber_only: payload.skipSubscriberOnly,
}) };
.pipe(catchError((err) => this.handleHTTPError(err))); // Send the clip fields only when actually filled in. The backend treats an
// absent field as "not requested", which is what stops a t= timestamp on the
// subscribed URL from clipping every future download.
const cs = payload.clipStart?.trim();
const ce = payload.clipEnd?.trim();
if (cs) body['clip_start'] = cs;
if (ce) body['clip_end'] = ce;
return this.http.post<Status>('subscribe', body).pipe(catchError((err) => this.handleHTTPError(err)));
} }
delete(ids: string[]) { delete(ids: string[]) {