mirror of
https://github.com/blakeblackshear/frigate.git
synced 2026-09-26 12:38:59 +03:00
Fix inconsistent export download filenames (#24111)
* fix inconsistent export download filenames Zip entries in a case download were named from `Export.name`, the friendly display name, while an individual download uses the file name on disk. The two have always been formatted differently, so one export came out as `front_door_20260823_020615-20260823_020734_abc123.mp4` on its own and `front door 2026-08-23 020615 2026-08-23 020734.mp4` inside a zip. Zip entries now use the on-disk file name, and renaming an export renames its file, so there's only one name to download under. The rename is blocked while ffmpeg still holds the file. * cap filename length and catch duplicate names * fix export rename and stop blocking the event loop * move the rename rollback off the event loop * no awaits
This commit is contained in:
committed by
Nicolas Mowen
parent
53fb6c8daa
commit
0aa086eefd
+64
-7
@@ -1,7 +1,9 @@
|
||||
"""Export apis."""
|
||||
|
||||
import contextlib
|
||||
import datetime
|
||||
import logging
|
||||
import os
|
||||
import random
|
||||
import string
|
||||
import time
|
||||
@@ -15,7 +17,7 @@ import psutil
|
||||
from fastapi import APIRouter, Depends, Query, Request
|
||||
from fastapi.responses import JSONResponse, StreamingResponse
|
||||
from pathvalidate import sanitize_filename
|
||||
from peewee import DoesNotExist
|
||||
from peewee import DatabaseError, DoesNotExist, IntegrityError
|
||||
from playhouse.shortcuts import model_to_dict
|
||||
|
||||
from frigate.api.auth import (
|
||||
@@ -72,6 +74,7 @@ from frigate.record.export import (
|
||||
DEFAULT_TIME_LAPSE_FFMPEG_INPUT_ARGS,
|
||||
ChaptersEnum,
|
||||
PlaybackSourceEnum,
|
||||
export_video_path,
|
||||
validate_ffmpeg_args,
|
||||
)
|
||||
from frigate.util.path import sanitize_contained_path
|
||||
@@ -405,14 +408,17 @@ class _StreamingZipBuffer:
|
||||
|
||||
|
||||
def _unique_archive_name(export: Export, used: set[str]) -> str:
|
||||
base = sanitize_filename(export.name) if export.name else None
|
||||
if not base:
|
||||
base = f"{export.camera}_{int(export.date)}"
|
||||
"""Zip entry name for an export, de-duplicated within the archive.
|
||||
|
||||
The on-disk name is the one the user sees either way: renaming an export
|
||||
renames its file, so a zip entry and an individual download can't drift.
|
||||
"""
|
||||
source = Path(export.video_path)
|
||||
candidate = source.name
|
||||
|
||||
candidate = f"{base}.mp4"
|
||||
counter = 1
|
||||
while candidate in used:
|
||||
candidate = f"{base}_{counter}.mp4"
|
||||
candidate = f"{source.stem}_{counter}{source.suffix}"
|
||||
counter += 1
|
||||
|
||||
used.add(candidate)
|
||||
@@ -928,8 +934,59 @@ async def export_rename(event_id: str, body: ExportRenameBody, request: Request)
|
||||
status_code=404,
|
||||
)
|
||||
|
||||
if export.in_progress:
|
||||
return JSONResponse(
|
||||
content={
|
||||
"success": False,
|
||||
"message": "Export is still being written and can't be renamed yet.",
|
||||
},
|
||||
status_code=400,
|
||||
)
|
||||
|
||||
new_path = export_video_path(body.name, export.id)
|
||||
old_path = export.video_path
|
||||
moved = new_path != old_path
|
||||
|
||||
# move the file first so a rename that can't happen leaves the row alone
|
||||
if moved:
|
||||
try:
|
||||
os.rename(old_path, new_path)
|
||||
except OSError:
|
||||
logger.exception("Failed to rename export file for %s", event_id)
|
||||
return JSONResponse(
|
||||
content={"success": False, "message": "Failed to rename export."},
|
||||
status_code=500,
|
||||
)
|
||||
|
||||
export.name = body.name
|
||||
export.save()
|
||||
export.video_path = new_path
|
||||
|
||||
try:
|
||||
export.save()
|
||||
except DatabaseError as err:
|
||||
# the queue database has no transactions, so undo the move by hand
|
||||
if moved:
|
||||
with contextlib.suppress(OSError):
|
||||
os.rename(new_path, old_path)
|
||||
|
||||
if isinstance(err, IntegrityError):
|
||||
logger.warning(
|
||||
"Export %s cannot be renamed, %s is taken", event_id, new_path
|
||||
)
|
||||
return JSONResponse(
|
||||
content={
|
||||
"success": False,
|
||||
"message": "Another export already uses that name.",
|
||||
},
|
||||
status_code=409,
|
||||
)
|
||||
|
||||
logger.exception("Failed to save renamed export %s", event_id)
|
||||
return JSONResponse(
|
||||
content={"success": False, "message": "Failed to rename export."},
|
||||
status_code=500,
|
||||
)
|
||||
|
||||
return JSONResponse(
|
||||
content=(
|
||||
{
|
||||
|
||||
Reference in New Issue
Block a user