fix: share, upload, crop, auto-skip, tag chips and See all hold what they claim
- Share: a link that expired is never copied (the dialog and one-click Share pick a live one, else make one), says it expired, and the copy-on-open claim clears on the next copy. - A password-protected file link asks to open a file, not to watch a video. - Upload: Cancel, the close button or the backdrop while naming a folder makes no folder; a failed refetch keeps the target; a folder being made says so. - Crop: an infinite number is dropped, not a 500; an unprobed clip's size follows a shown-size crop; the edit_spec docs name frame_w/frame_h; the join and rotated paths are tested. - Auto-skip: the wait gives way on a marker too short for it, so the skip still fires. - Tag chips count the followed channels (subscribed_count) and a toggled chip keeps the focus. - Continue watching's See all sorts by last watched when in progress was already on.
This commit is contained in:
@@ -11,7 +11,9 @@ with the worker branching on the asset's `source_kind` to run ffmpeg instead of
|
||||
(the `-ss` seeks first), so a short clip is cheap even from a long source
|
||||
* crop (± trim) → always re-encode (a filter can't be stream-copied), frame-accurate
|
||||
|
||||
`edit_spec = {trim?: {start_s, end_s}, crop?: {x, y, w, h}, accurate?: bool}`. `accurate` is the
|
||||
`edit_spec = {trim?: {start_s, end_s}, crop?: {x, y, w, h, frame_w?, frame_h?}, accurate?: bool}`. A
|
||||
crop with `frame_w`/`frame_h` counts the picture as it is SHOWN, measured on a frame of that size
|
||||
(`crop_filter`); one without them is in the file's stored pixels (its older form). `accurate` is the
|
||||
per-edit user choice for a trim-only cut (ignored when a crop forces a re-encode anyway). `split`
|
||||
is a frontend concept: the
|
||||
UI fans a split out into N trim jobs, so there is one output file per job here.
|
||||
@@ -65,8 +67,9 @@ def _crop_frame(crop: dict) -> dict | None:
|
||||
spec from before it was sent, which `crop_filter` cuts in the file's own pixels)."""
|
||||
try:
|
||||
w, h = int(crop["frame_w"]), int(crop["frame_h"])
|
||||
# silent-ok: no frame is a crop's older form, and it is still the crop that was asked for.
|
||||
except (KeyError, TypeError, ValueError):
|
||||
# silent-ok: no frame is a crop's older form, and it is still the crop that was asked for. An
|
||||
# infinite number (JSON `1e400`) is OverflowError, and it is no more a frame than "x" is.
|
||||
except (KeyError, TypeError, ValueError, OverflowError):
|
||||
return None
|
||||
return {"frame_w": w, "frame_h": h} if w > 0 and h > 0 else None
|
||||
|
||||
@@ -85,6 +88,19 @@ def crop_filter(crop: dict) -> str:
|
||||
)
|
||||
|
||||
|
||||
def crop_stored_size(crop: dict, src_w: int | None, src_h: int | None) -> tuple[int, int] | None:
|
||||
"""The stored size `crop_filter` cuts out of a `src_w` x `src_h` file, evened as `EVEN_FRAME`
|
||||
evens it — the clip's size when ffprobe cannot tell. A crop counted in the shown picture is a
|
||||
fraction of the file's frame, not its own pixel count; None when the source size is unknown."""
|
||||
if crop.get("frame_w") and crop.get("frame_h"):
|
||||
if not src_w or not src_h:
|
||||
return None
|
||||
w, h = src_w * crop["w"] // crop["frame_w"], src_h * crop["h"] // crop["frame_h"]
|
||||
else:
|
||||
w, h = crop["w"], crop["h"]
|
||||
return w // 2 * 2, h // 2 * 2
|
||||
|
||||
|
||||
def normalize_edit_spec(spec: dict | None) -> dict:
|
||||
"""Canonicalize a raw edit spec: clamp/round trim, coerce crop to ints, drop empties.
|
||||
|
||||
@@ -97,7 +113,7 @@ def normalize_edit_spec(spec: dict | None) -> dict:
|
||||
if crop:
|
||||
try:
|
||||
c = {k: int(round(float(crop[k]))) for k in ("x", "y", "w", "h")}
|
||||
except (KeyError, TypeError, ValueError) as exc:
|
||||
except (KeyError, TypeError, ValueError, OverflowError) as exc:
|
||||
# The clip still renders — uncropped. The user asked for a crop, got a whole frame,
|
||||
# and nothing anywhere called it a failure; this line is the only trace there is.
|
||||
log.warning(
|
||||
|
||||
@@ -127,18 +127,25 @@ def check_grant(token: str, grant: str | None, password_version: int) -> bool:
|
||||
# --- serialization -------------------------------------------------------------------------
|
||||
|
||||
|
||||
def share_path(db: Session, link: DownloadLink, token: str) -> str:
|
||||
"""The address a link is given out on: its path says what it opens. Only a word for the reader,
|
||||
the token is the whole key: every prefix of `main.SHARE_PREFIXES` opens every link, so the links
|
||||
already sent on /watch/ work for good. A file still being made reads as the video it mostly is."""
|
||||
def opens(db: Session, link: DownloadLink) -> str:
|
||||
"""What a link opens, in the words its address and its preview use: "folder", "video" or
|
||||
"file". A file still being made reads as the video it mostly is."""
|
||||
from app.downloads import mediakind
|
||||
|
||||
if link.folder_id is not None:
|
||||
return f"/folder/{token}"
|
||||
return "folder"
|
||||
job = db.get(DownloadJob, link.job_id)
|
||||
asset = db.get(MediaAsset, job.asset_id) if job and job.asset_id else None
|
||||
kind = asset.media_kind if asset else None
|
||||
return f"/file/{token}" if kind and kind != mediakind.VIDEO else f"/watch/{token}"
|
||||
return "file" if kind and kind != mediakind.VIDEO else "video"
|
||||
|
||||
|
||||
def share_path(db: Session, link: DownloadLink, token: str) -> str:
|
||||
"""The address a link is given out on: its path says what it opens (`opens`). Only a word for the
|
||||
reader, the token is the whole key: every prefix of `main.SHARE_PREFIXES` opens every link, so
|
||||
the links already sent on /watch/ work for good."""
|
||||
prefix = {"folder": "folder", "file": "file", "video": "watch"}[opens(db, link)]
|
||||
return f"/{prefix}/{token}"
|
||||
|
||||
|
||||
def owner_view(db: Session, link: DownloadLink, token: str | None = None) -> dict:
|
||||
|
||||
@@ -234,7 +234,9 @@ class DownloadJob(Base, TimestampMixin, UpdatedAtMixin):
|
||||
source_asset_id: Mapped[int | None] = mapped_column(
|
||||
ForeignKey("media_assets.id", ondelete="SET NULL")
|
||||
)
|
||||
# For edit jobs: the trim/crop recipe {trim?:{start_s,end_s}, crop?:{x,y,w,h}} in pixels. For clip
|
||||
# For edit jobs: the trim/crop recipe {trim?:{start_s,end_s}, crop?:{x,y,w,h,frame_w?,frame_h?}}, its
|
||||
# crop in the SHOWN picture's pixels when it carries the frame it was measured on (`edit.crop_filter`),
|
||||
# else in the file's stored ones. For clip
|
||||
# jobs: the clip spec {source, ref, start_s, end_s, audio_track?, crop?:{x,y,w,h,aspect}}, its crop
|
||||
# in fractions of the box it was drawn in (`downloads.clip.normalize`).
|
||||
edit_spec: Mapped[dict | None] = mapped_column(JSON)
|
||||
|
||||
@@ -77,9 +77,9 @@ def _watch_tags(token: str, db: Session) -> str | None:
|
||||
if link is None or linksmod.is_expired(link):
|
||||
return None
|
||||
url = f"{settings.app_base}{linksmod.share_path(db, link, token)}"
|
||||
folder = link.folder_id is not None
|
||||
# What the card calls it, as its address does (`links.share_path`).
|
||||
video = "/watch/" in url
|
||||
# What the card calls it, as its address does.
|
||||
kind = linksmod.opens(db, link)
|
||||
folder, video = kind == "folder", kind == "video"
|
||||
if link.password_hash:
|
||||
# Password-gated: don't leak the title/thumbnail in the unfurl.
|
||||
return "\n ".join(
|
||||
|
||||
@@ -914,13 +914,13 @@ def _process_edit(job_id: int, asset_id: int) -> None:
|
||||
asset.size_bytes = size
|
||||
asset.container = out_ext
|
||||
asset.duration_s = total_s or asset.duration_s
|
||||
cut = editmod.crop_stored_size(edit_spec["crop"], src_w, src_h) if crop else None
|
||||
if out_w and out_h:
|
||||
asset.width, asset.height = out_w, out_h
|
||||
elif crop:
|
||||
asset.width = edit_spec["crop"]["w"]
|
||||
asset.height = edit_spec["crop"]["h"]
|
||||
elif cut:
|
||||
asset.width, asset.height = cut
|
||||
else:
|
||||
asset.width, asset.height = src_w, src_h
|
||||
asset.width, asset.height = (None, None) if crop else (src_w, src_h)
|
||||
asset.error = None
|
||||
asset.last_access_at = datetime.now(timezone.utc)
|
||||
job = db.get(DownloadJob, job_id)
|
||||
|
||||
@@ -41,11 +41,23 @@ def video(db: Session, video_id: str) -> Video | None:
|
||||
# --- tags ------------------------------------------------------------------------------------
|
||||
|
||||
|
||||
def tags_with_counts(db: Session, user: User) -> list[tuple[Tag, int]]:
|
||||
"""The tags this user sees (the shared system ones and their own), each with how many channel
|
||||
links it has, by category then name."""
|
||||
def tags_with_counts(db: Session, user: User) -> list[tuple[Tag, int, int]]:
|
||||
"""The tags this user sees (the shared system ones and their own), by category then name, each
|
||||
with how many channel links it has and how many of the user's own links sit on a channel they
|
||||
subscribe to — the rows the Channels page's tag filter shows. A link outlives an unsubscribe,
|
||||
so the two differ."""
|
||||
subscribed = select(Subscription.channel_id).where(owned_by(Subscription.user_id, user))
|
||||
on_subscribed = dict(
|
||||
db.execute(
|
||||
select(ChannelTag.tag_id, func.count(ChannelTag.id))
|
||||
.where(owned_by(ChannelTag.user_id, user), ChannelTag.channel_id.in_(subscribed))
|
||||
.group_by(ChannelTag.tag_id)
|
||||
)
|
||||
.tuples()
|
||||
.all()
|
||||
)
|
||||
return [
|
||||
(tag, count)
|
||||
(tag, count, on_subscribed.get(tag.id, 0))
|
||||
for tag, count in db.execute(
|
||||
select(Tag, func.count(ChannelTag.id))
|
||||
.outerjoin(ChannelTag, ChannelTag.tag_id == Tag.id)
|
||||
|
||||
@@ -176,8 +176,15 @@ def watch_meta(token: str, db: Session = Depends(get_db)) -> dict:
|
||||
returns only `needs_password` until the viewer unlocks it (no title/thumbnail leak)."""
|
||||
link = _link_or_404(db, token)
|
||||
if link.password_hash:
|
||||
# Which kind it opens, for the password prompt's title — never what is in it.
|
||||
return {"needs_password": True, "kind": "folder" if link.folder_id is not None else "file"}
|
||||
# Which kind it opens, for the password prompt's words — never what is in it. `video` tells
|
||||
# a video (watched) from any other file (opened); the address is only a hint, every
|
||||
# prefix opens every link.
|
||||
opens = linksmod.opens(db, link)
|
||||
return {
|
||||
"needs_password": True,
|
||||
"kind": "folder" if opens == "folder" else "file",
|
||||
"video": opens == "video",
|
||||
}
|
||||
meta = _opened(db, link, token)
|
||||
db.commit()
|
||||
return meta
|
||||
|
||||
@@ -48,8 +48,9 @@ def list_tags(user: User = Depends(current_user), db: Session = Depends(get_db))
|
||||
"category": tag.category,
|
||||
"system": tag.user_id is None,
|
||||
"channel_count": count,
|
||||
"subscribed_count": subscribed,
|
||||
}
|
||||
for tag, count in queries.tags_with_counts(db, user)
|
||||
for tag, count, subscribed in queries.tags_with_counts(db, user)
|
||||
]
|
||||
|
||||
|
||||
|
||||
@@ -20,8 +20,10 @@ from app.downloads.models import DownloadJob, MediaAsset
|
||||
pytestmark = pytest.mark.skipif(shutil.which("ffmpeg") is None, reason="ffmpeg not available")
|
||||
|
||||
|
||||
def _source(path: Path, size: str, box: str, sar: int) -> None:
|
||||
"""Red, with the picture's middle half green: a crop of that middle holds no red."""
|
||||
def _source(path: Path, size: str, box: str, sar: int, rotate: int = 0) -> None:
|
||||
"""Red, with a green box: a crop of the box holds no red. `rotate` tags the file to be shown
|
||||
turned by that many degrees counter-clockwise, as a phone records a portrait video."""
|
||||
tag = ["-display_rotation:v:0", str(rotate)] if rotate else []
|
||||
subprocess.run(
|
||||
[
|
||||
"ffmpeg", "-y", "-loglevel", "error",
|
||||
@@ -32,6 +34,13 @@ def _source(path: Path, size: str, box: str, sar: int) -> None:
|
||||
],
|
||||
check=True,
|
||||
) # fmt: skip
|
||||
if tag:
|
||||
tagged = path.with_name("tagged-" + path.name)
|
||||
subprocess.run(
|
||||
["ffmpeg", "-y", "-loglevel", "error", *tag, "-i", str(path), "-c", "copy", str(tagged)],
|
||||
check=True,
|
||||
) # fmt: skip
|
||||
tagged.replace(path)
|
||||
|
||||
|
||||
def _red_and_green(path: Path) -> tuple[int, int]:
|
||||
@@ -50,13 +59,14 @@ def _red_and_green(path: Path) -> tuple[int, int]:
|
||||
return red, green
|
||||
|
||||
|
||||
def _cut(db, make_user, tmp_path, src: Path, crop: dict) -> Path:
|
||||
def _cut_asset(db, make_user, src: Path, spec: dict, then=lambda: None) -> MediaAsset:
|
||||
user = make_user()
|
||||
source = uploadmod.ingest_upload(
|
||||
db, user.id, filename="clip.mp4", src_path=src, size=src.stat().st_size
|
||||
)
|
||||
assert source.status == "done"
|
||||
job = service.enqueue_edit(db, user.id, source, {"crop": crop})
|
||||
then()
|
||||
job = service.enqueue_edit(db, user.id, source, spec)
|
||||
asset = db.get(MediaAsset, job.asset_id)
|
||||
job.status, asset.status = "running", "downloading" # the worker's claim
|
||||
db.commit()
|
||||
@@ -66,6 +76,11 @@ def _cut(db, make_user, tmp_path, src: Path, crop: dict) -> Path:
|
||||
db.expire_all()
|
||||
job, asset = db.get(DownloadJob, job.id), db.get(MediaAsset, job.asset_id)
|
||||
assert (job.status, job.error, asset.status) == ("done", None, "ready")
|
||||
return asset
|
||||
|
||||
|
||||
def _cut(db, make_user, tmp_path, src: Path, crop: dict, **spec) -> Path:
|
||||
asset = _cut_asset(db, make_user, src, {"crop": crop, **spec})
|
||||
return storage.abs_path(str(tmp_path), asset.rel_path)
|
||||
|
||||
|
||||
@@ -110,3 +125,64 @@ def test_a_crop_stored_before_it_carried_its_frame_is_cut_as_it_always_was():
|
||||
def test_a_frame_that_cannot_be_read_is_not_divided_by(frame):
|
||||
spec = edit.normalize_edit_spec({"crop": {"x": 4, "y": 6, "w": 100, "h": 50, **frame}})
|
||||
assert spec["crop"] == {"x": 4, "y": 6, "w": 100, "h": 50}
|
||||
|
||||
|
||||
def test_a_crop_of_a_join_keeps_what_the_frame_showed(db, make_user, tmp_path, monkeypatch):
|
||||
"""A cut-list joins through `filter_complex`, a crop path of its own."""
|
||||
monkeypatch.setattr(settings, "files_root", str(tmp_path))
|
||||
src = tmp_path / "in.mp4"
|
||||
_source(src, "320x240", "x=80:y=60:w=160:h=120", sar=2)
|
||||
|
||||
final = _cut(
|
||||
db, make_user, tmp_path, src,
|
||||
{"x": 160, "y": 60, "w": 320, "h": 120, "frame_w": 640, "frame_h": 240},
|
||||
segments=[{"start_s": 0, "end_s": 0.5}, {"start_s": 1, "end_s": 1.5}], accurate=True,
|
||||
) # fmt: skip
|
||||
|
||||
assert _red_and_green(final) == (0, 160 * 120)
|
||||
|
||||
|
||||
def test_a_crop_of_a_rotated_video_is_cut_where_it_was_drawn(db, make_user, tmp_path, monkeypatch):
|
||||
"""The browser shows a rotated file upright, so the frame was drawn on the upright picture."""
|
||||
monkeypatch.setattr(settings, "files_root", str(tmp_path))
|
||||
src = tmp_path / "in.mp4"
|
||||
# Stored 320 x 240, green on its left half; shown turned a quarter, 240 x 320.
|
||||
_source(src, "320x240", "x=0:y=0:w=160:h=240", sar=1, rotate=90)
|
||||
|
||||
# Turned counter-clockwise, the stored left half is the shown bottom half.
|
||||
final = _cut(
|
||||
db, make_user, tmp_path, src,
|
||||
{"x": 0, "y": 160, "w": 240, "h": 160, "frame_w": 240, "frame_h": 320},
|
||||
) # fmt: skip
|
||||
|
||||
assert _red_and_green(final) == (0, 240 * 160)
|
||||
|
||||
|
||||
def test_a_clip_ffprobe_cannot_size_gets_the_size_its_crop_cut(
|
||||
db, make_user, tmp_path, monkeypatch
|
||||
):
|
||||
monkeypatch.setattr(settings, "files_root", str(tmp_path))
|
||||
src = tmp_path / "in.mp4"
|
||||
_source(src, "320x240", "x=80:y=60:w=160:h=120", sar=2)
|
||||
|
||||
def probe_fails():
|
||||
monkeypatch.setattr(pipeline.probe, "probe_media", lambda _p: (None,) * 5)
|
||||
|
||||
asset = _cut_asset(
|
||||
db, make_user, src,
|
||||
{"crop": {"x": 160, "y": 60, "w": 320, "h": 120, "frame_w": 640, "frame_h": 240}},
|
||||
then=probe_fails,
|
||||
) # fmt: skip
|
||||
|
||||
# Half the shown width of a 320-wide file: 160 stored, not the 320 the frame counted.
|
||||
assert (asset.width, asset.height) == (160, 120)
|
||||
|
||||
|
||||
@pytest.mark.parametrize("key", ["frame_w", "x"])
|
||||
def test_an_infinite_number_in_a_crop_is_dropped_not_a_crash(key):
|
||||
crop = {"x": 4, "y": 6, "w": 100, "h": 50, "frame_w": 640, "frame_h": 240}
|
||||
spec = edit.normalize_edit_spec({"crop": {**crop, key: float("inf")}})
|
||||
if key == "frame_w":
|
||||
assert spec["crop"] == {"x": 4, "y": 6, "w": 100, "h": 50}
|
||||
else:
|
||||
assert "crop" not in spec
|
||||
|
||||
@@ -150,6 +150,7 @@ def test_a_password_folder_link_shows_nothing_until_unlocked(shelf, client):
|
||||
assert client.get(f"/api/public/watch/{token}").json() == {
|
||||
"needs_password": True,
|
||||
"kind": "folder",
|
||||
"video": False,
|
||||
}
|
||||
bare = f"/api/public/watch/{token}/files/{shelf['inside'][0]}/file"
|
||||
assert client.get(bare).status_code == 403
|
||||
@@ -312,7 +313,7 @@ def test_a_password_file_link_says_it_opens_a_file(shelf, client):
|
||||
.json()
|
||||
)
|
||||
meta = client.get(f"/api/public/watch/{made['token']}").json()
|
||||
assert meta == {"needs_password": True, "kind": "file"}
|
||||
assert meta == {"needs_password": True, "kind": "file", "video": True}
|
||||
|
||||
|
||||
def test_a_file_whose_bytes_are_gone_is_not_offered(shelf, client, db):
|
||||
|
||||
@@ -14,7 +14,7 @@ from sqlalchemy import select
|
||||
from sqlalchemy.exc import IntegrityError
|
||||
|
||||
from app.discover import pins
|
||||
from app.catalog.models import Channel, Video
|
||||
from app.catalog.models import Channel, Subscription, Video
|
||||
from app.reader import queries, service
|
||||
from app.reader.models import ChannelTag, ComfortPin, Playlist, PlaylistItem, Tag, VideoState
|
||||
|
||||
@@ -60,12 +60,30 @@ def test_a_system_tag_is_nobodys_to_edit(db, make_user):
|
||||
service.create_tag(db, other, "Theirs", "other", None) # another user's tag stays theirs
|
||||
db.commit()
|
||||
assert queries.own_tag(db, user, system.id) is None
|
||||
assert [t.id for t, _ in queries.tags_with_counts(db, user)] == [system.id]
|
||||
assert [t.id for t, *_ in queries.tags_with_counts(db, user)] == [system.id]
|
||||
|
||||
|
||||
# --- tags ------------------------------------------------------------------------------------
|
||||
|
||||
|
||||
def test_a_tag_counts_apart_the_links_on_channels_the_user_still_follows(db, make_user):
|
||||
"""An unsubscribe keeps the channel's tag links; the Channels page's filter lists only followed
|
||||
channels, so its count must not hold the others."""
|
||||
user, other = make_user(), make_user()
|
||||
tag = service.create_tag(db, user, "Music", "other", None)
|
||||
for cid in ("UCa", "UCb", "UCgone"):
|
||||
db.add(Channel(id=cid, title=cid))
|
||||
db.flush()
|
||||
for cid in ("UCa", "UCb"):
|
||||
db.add(Subscription(user_id=user.id, channel_id=cid))
|
||||
db.add(Subscription(user_id=other.id, channel_id="UCgone"))
|
||||
for cid in ("UCa", "UCb", "UCgone"):
|
||||
db.add(ChannelTag(channel_id=cid, tag_id=tag.id, user_id=user.id, source="user"))
|
||||
db.commit()
|
||||
[(_, count, subscribed)] = [r for r in queries.tags_with_counts(db, user) if r[0].id == tag.id]
|
||||
assert (count, subscribed) == (3, 2)
|
||||
|
||||
|
||||
def test_a_tag_is_made_once_per_name_and_an_unknown_category_is_other(db, make_user):
|
||||
user = make_user()
|
||||
tag = service.create_tag(db, user, "Later", "nonsense", "#fff")
|
||||
|
||||
@@ -135,3 +135,14 @@ def test_a_card_of_a_video_stays_a_video(shared, db):
|
||||
assert f"/watch/{made['token']}" in tags and "video.other" in tags
|
||||
locked = shared("other.mp4", _VIDEO, password="pw")
|
||||
assert "Password-protected video" in og._watch_tags(locked["token"], db)
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
("name", "data", "video"),
|
||||
[("clip.mp4", _VIDEO, True), ("song.m4a", _AUDIO, False), ("backup.tar.gz", None, False)],
|
||||
)
|
||||
def test_a_password_link_says_whether_it_opens_a_video(shared, client, name, data, video):
|
||||
"""The lock screen asks to watch a video and to open anything else, whichever path it is on."""
|
||||
token = shared(name, data, password="pw")["token"]
|
||||
meta = client.get(f"/api/public/watch/{token}").json()
|
||||
assert meta == {"needs_password": True, "kind": "file", "video": video}
|
||||
|
||||
Reference in New Issue
Block a user