diff --git a/apps/channels/tasks.py b/apps/channels/tasks.py index 9548d659..29fe5fc2 100755 --- a/apps/channels/tasks.py +++ b/apps/channels/tasks.py @@ -1611,6 +1611,28 @@ def _build_output_paths(channel, program, start_time, end_time): rel_path = rel_path[2:] final_path = rel_path if rel_path.startswith('/') else os.path.join(library_root, rel_path) final_path = os.path.normpath(final_path) + + # Avoid overwriting an existing file from a different recording. + # Check BOTH .mkv and .ts — a pre-restart TS segment may exist at + # the same base name even when the MKV is a 0-byte placeholder. + base, ext = os.path.splitext(final_path) + counter = 1 + while True: + candidate_base = final_path[:-len(ext)] # strip extension + ts_candidate = candidate_base + '.ts' + try: + mkv_occupied = os.stat(final_path).st_size > 0 + except OSError: + mkv_occupied = False + try: + ts_occupied = os.stat(ts_candidate).st_size > 0 + except OSError: + ts_occupied = False + if not mkv_occupied and not ts_occupied: + break + counter += 1 + final_path = f"{base}_{counter}{ext}" + # Ensure directory exists os.makedirs(os.path.dirname(final_path), exist_ok=True) @@ -2121,8 +2143,13 @@ def run_recording(recording_id, channel_id, start_time_str, end_time_str): recording_cancelled = not Recording.objects.filter(id=recording_id).exists() if recording_cancelled: logger.info(f"Recording {recording_id} was cancelled — skipping remux and metadata.") - # Clean up all artifacts for the cancelled recording - for _cleanup_path in [temp_ts_path, final_path]: + # Clean up all artifacts for the cancelled recording, + # including any pre-restart .ts segments from server recovery. + # Use the in-memory recording_obj since the DB row is already deleted. + _cancel_cleanup = [temp_ts_path, final_path] + _cancel_cp = (recording_obj.custom_properties or {}) if recording_obj else {} + _cancel_cleanup.extend(_cancel_cp.get("_pre_restart_ts_paths", [])) + for _cleanup_path in _cancel_cleanup: if not _cleanup_path: continue try: @@ -2134,8 +2161,93 @@ def run_recording(recording_id, channel_id, start_time_str, end_time_str): pass return + # Concatenate pre-restart .ts segments with the current segment. + # Instead of creating an intermediate combined.ts and then remuxing to + # MKV (which loses timestamp boundary info and causes playback freezes + # at the splice point), go directly from the concat list → MKV. + # This lets ffmpeg's MKV muxer see each segment boundary and write + # correct cue points / clusters for seamless seeking. + _concat_did_remux = False + try: + _rec_obj_for_concat = Recording.objects.filter(id=recording_id).only("custom_properties").first() + _concat_cp = (_rec_obj_for_concat.custom_properties or {}) if _rec_obj_for_concat else {} + pre_restart_segments = _concat_cp.get("_pre_restart_ts_paths", []) + # Filter to segments that still exist on disk and have data + def _has_data(p): + try: + return os.stat(p).st_size > 0 + except OSError: + return False + pre_restart_segments = [p for p in pre_restart_segments if p and _has_data(p)] + if pre_restart_segments and temp_ts_path and os.path.exists(temp_ts_path): + all_segments = pre_restart_segments + [temp_ts_path] + concat_list_path = temp_ts_path + ".concat.txt" + try: + with open(concat_list_path, "w") as cl: + for seg in all_segments: + cl.write(f"file '{seg}'\n") + + # Direct concat → MKV in a single pass. + # -reset_timestamps 1 tells the concat demuxer to reset + # timestamps at each segment boundary, eliminating the + # discontinuity that causes playback to freeze at the + # splice point. + concat_result = subprocess.run( + [ + "ffmpeg", "-y", + "-fflags", "+genpts+igndts+discardcorrupt", + "-err_detect", "ignore_err", + "-f", "concat", "-safe", "0", + "-segment_time_metadata", "1", + "-i", concat_list_path, + "-reset_timestamps", "1", + "-map", "0", + "-c", "copy", + final_path, + ], + stdout=subprocess.PIPE, stderr=subprocess.PIPE, text=True, + ) + if concat_result.returncode == 0 and os.path.exists(final_path) and os.path.getsize(final_path) > 0: + _concat_did_remux = True + # Clean up individual TS segments (including current) + for seg in all_segments: + try: + os.remove(seg) + except OSError: + pass + logger.info( + f"DVR recording {recording_id}: concat→MKV succeeded — " + f"{len(all_segments)} segments → {os.path.basename(final_path)} " + f"({os.path.getsize(final_path):,} bytes)" + ) + else: + logger.warning( + f"DVR recording {recording_id}: direct concat→MKV failed " + f"(rc={concat_result.returncode}), falling back to " + f"normal remux with current segment only. " + f"stderr: {(concat_result.stderr or '')[:500]}" + ) + finally: + try: + os.remove(concat_list_path) + except OSError: + pass + # Clear the pre-restart paths from custom_properties + if _rec_obj_for_concat: + _ccp = _rec_obj_for_concat.custom_properties or {} + _ccp.pop("_pre_restart_ts_paths", None) + _ccp.pop("interrupted_reason", None) + _rec_obj_for_concat.custom_properties = _ccp + _rec_obj_for_concat.save(update_fields=["custom_properties"]) + except Exception as e: + logger.warning( + f"DVR recording {recording_id}: segment concatenation error " + f"({type(e).__name__}: {e}), proceeding with current segment only." + ) + # Remux TS to MKV container with retry for transient I/O errors - remux_success = False + # (Skip if concat already produced the final MKV directly.) + remux_success = _concat_did_remux existing_mkv_size = 0 try: if final_path and os.path.exists(final_path): @@ -2143,6 +2255,8 @@ def run_recording(recording_id, channel_id, start_time_str, end_time_str): except OSError: pass for _remux_attempt in range(_dvr_remux_max_retries): + if remux_success: + break try: if temp_ts_path and os.path.exists(temp_ts_path): # First attempt: Direct TS to MKV remux @@ -2432,8 +2546,9 @@ def recover_recordings_on_startup(): redis = RedisClient.get_client() if redis: lock_key = "dvr:recover_lock" - # Set lock with 60s TTL; only first winner proceeds - if not redis.set(lock_key, "1", ex=60, nx=True): + # Set lock with 10-minute TTL; must be long enough for Phase 2 + # ffmpeg remux operations on large files. + if not redis.set(lock_key, "1", ex=600, nx=True): return "Recovery already in progress" now = timezone.now() @@ -2451,12 +2566,15 @@ def recover_recordings_on_startup(): cp = rec.custom_properties or {} current_status = cp.get("status", "") - # Skip recordings that are already in a terminal or active state. + # Skip recordings that are already in a terminal state. # "completed" / "stopped" — user stopped or it finished normally; do NOT # overwrite the status and re-schedule (that would cause the # Interrupted → In-Progress → Previously-Recorded ghost cycle). - # "recording" — a worker is already streaming; leave it alone. - if current_status in ("completed", "stopped", "recording"): + # NOTE: "recording" is NOT skipped — this function runs on + # worker_ready, meaning all previous workers are dead. A + # recording stuck in "recording" status is from a crashed + # worker and must be recovered. + if current_status in ("completed", "stopped"): logger.info( f"recover_recordings_on_startup: skipping recording {rec.id} " f"(status={current_status!r}, already in terminal/active state)." @@ -2466,19 +2584,104 @@ def recover_recordings_on_startup(): # Mark interrupted due to restart; will flip to 'recording' when task starts cp["status"] = "interrupted" cp["interrupted_reason"] = "server_restarted" + + # Preserve the pre-restart .ts segment path so run_recording + # can concatenate it with the resumed segment later. + old_ts = cp.get("_temp_file_path") + if old_ts and os.path.exists(old_ts) and os.path.getsize(old_ts) > 0: + prior_segments = cp.get("_pre_restart_ts_paths", []) + prior_segments.append(old_ts) + cp["_pre_restart_ts_paths"] = prior_segments + logger.info( + f"recover_recordings_on_startup: recording {rec.id} — " + f"preserving pre-restart TS segment: {old_ts}" + ) + rec.custom_properties = cp _db_retry( lambda r=rec: r.save(update_fields=["custom_properties"]), label=f"DVR recovery: recording {rec.id} status update", ) - # Start recording for remaining window + # Revoke the old PeriodicTask so Celery Beat doesn't also + # fire run_recording for this recording (would be a duplicate). + old_task_id = rec.task_id + if old_task_id: + try: + revoke_task(old_task_id) + except Exception: + pass + + # Start recording for remaining window. Use a deterministic + # task_id so duplicate dispatches (e.g. from a second recovery + # attempt) are deduplicated by Celery/Redis. + recovery_task_id = f"dvr-recover-{rec.id}" run_recording.apply_async( - args=[rec.id, rec.channel_id, str(now), str(rec.end_time)], eta=now + args=[rec.id, rec.channel_id, str(now), str(rec.end_time)], + eta=now, + task_id=recovery_task_id, ) except Exception as e: logger.warning(f"Failed to resume recording {rec.id}: {e}") + # Finalize expired recordings that were active when the server crashed + # but whose end_time has now passed. Remux the partial .ts and mark + # as interrupted so the user can watch whatever was captured. + expired = _db_retry( + lambda: list(Recording.objects.filter( + end_time__lte=now, + custom_properties__status="recording", + )), + label="DVR recovery: fetching expired recordings", + ) + for rec in expired: + try: + cp = rec.custom_properties or {} + ts_path = cp.get("_temp_file_path") + mkv_path = cp.get("file_path") + + if ts_path and os.path.exists(ts_path) and os.path.getsize(ts_path) > 0 and mkv_path: + logger.info( + f"recover_recordings_on_startup: recording {rec.id} expired " + f"during downtime — remuxing partial TS ({os.path.getsize(ts_path):,} bytes)" + ) + os.makedirs(os.path.dirname(mkv_path), exist_ok=True) + result = subprocess.run( + [ + "ffmpeg", "-y", + "-fflags", "+genpts+igndts+discardcorrupt", + "-err_detect", "ignore_err", + "-i", ts_path, "-map", "0", "-c", "copy", mkv_path, + ], + stdout=subprocess.PIPE, stderr=subprocess.PIPE, text=True, + ) + if result.returncode == 0 and os.path.exists(mkv_path) and os.path.getsize(mkv_path) > 0: + cp["status"] = "interrupted" + cp["interrupted_reason"] = "server_restarted_after_end" + cp["remux_success"] = True + try: + os.remove(ts_path) + except OSError: + pass + logger.info(f"recover_recordings_on_startup: recording {rec.id} remuxed successfully") + else: + cp["status"] = "interrupted" + cp["interrupted_reason"] = "server_restarted_after_end" + cp["remux_success"] = False + logger.warning(f"recover_recordings_on_startup: recording {rec.id} remux failed, keeping .ts") + else: + cp["status"] = "interrupted" + cp["interrupted_reason"] = "server_restarted_after_end" + cp["remux_success"] = False + + rec.custom_properties = cp + _db_retry( + lambda r=rec: r.save(update_fields=["custom_properties"]), + label=f"DVR recovery: recording {rec.id} expired status update", + ) + except Exception as e: + logger.warning(f"Failed to finalize expired recording {rec.id}: {e}") + # Ensure future recordings are scheduled. # With ClockedSchedule, PeriodicTasks survive restarts in the DB. # Only recreate if the PeriodicTask is missing (safety net). @@ -2521,6 +2724,12 @@ def recover_recordings_on_startup(): except Exception as e: logger.warning(f"Failed to schedule recording {rec.id}: {e}") + # Release the lock early so a subsequent restart can recover + # immediately. The 10-minute TTL is only a safety net in case + # recovery itself crashes before reaching this point. + if redis: + redis.delete(lock_key) + return "Recovery complete" except Exception as e: logger.error(f"Error during DVR recovery: {e}") @@ -2791,6 +3000,15 @@ def _resolve_poster_for_program(channel_name, program, channel_logo_id=None): _title = ((program.get("title") if isinstance(program, dict) else None) or "").strip() or None + # Guard: if the "title" is really just the channel name (common when EPG + # has no real program data), don't use it for external API searches — + # those queries produce false-positive artwork from unrelated shows. + _title_is_channel_name = False + if _title and channel_name: + def _norm_channel(s): + return s.lower().replace("*", "").replace("-", " ").strip() + _title_is_channel_name = _norm_channel(_title) == _norm_channel(channel_name) + # Stage 1: EPG Program images/icon (with URL validation) try: from apps.epg.models import ProgramData @@ -2810,7 +3028,7 @@ def _resolve_poster_for_program(channel_name, program, channel_logo_id=None): pass # Stage 2: VOD logo fallback by title - if not poster_url and not poster_logo_id and _title: + if not poster_url and not poster_logo_id and _title and not _title_is_channel_name: try: from apps.vod.models import Movie, Series vod_logo = None @@ -2827,7 +3045,7 @@ def _resolve_poster_for_program(channel_name, program, channel_logo_id=None): pass # Stage 3: TMDB/OMDb (keyed APIs) - if not poster_url and not poster_logo_id and _title: + if not poster_url and not poster_logo_id and _title and not _title_is_channel_name: try: tmdb_key = os.environ.get('TMDB_API_KEY') omdb_key = os.environ.get('OMDB_API_KEY') @@ -2900,7 +3118,7 @@ def _resolve_poster_for_program(channel_name, program, channel_logo_id=None): pass # Stage 4: Keyless providers (TVMaze & iTunes) - if not poster_url and not poster_logo_id and _title: + if not poster_url and not poster_logo_id and _title and not _title_is_channel_name: try: title = _title # TVMaze @@ -2948,7 +3166,7 @@ def _resolve_poster_for_program(channel_name, program, channel_logo_id=None): break # Stage 6: Search existing Logo entries by program title - if not poster_logo_id and not poster_url and _title: + if not poster_logo_id and not poster_url and _title and not _title_is_channel_name: try: from .models import Logo existing = Logo.objects.filter(name__iexact=_title).first() diff --git a/apps/channels/tests/test_dvr_retry.py b/apps/channels/tests/test_db_retry.py similarity index 84% rename from apps/channels/tests/test_dvr_retry.py rename to apps/channels/tests/test_db_retry.py index 7d6b3a97..e65d929a 100644 --- a/apps/channels/tests/test_dvr_retry.py +++ b/apps/channels/tests/test_db_retry.py @@ -25,7 +25,7 @@ class DbRetryTests(TestCase): """Tests for the _db_retry() exponential backoff helper.""" @patch("apps.channels.tasks.time.sleep") - @patch("apps.channels.tasks.close_old_connections", create=True) + @patch("apps.channels.tasks.close_old_connections") def test_succeeds_on_first_attempt(self, _close, _sleep): """No retry needed when fn succeeds immediately.""" result = _db_retry(lambda: "ok", max_retries=3) @@ -33,7 +33,7 @@ class DbRetryTests(TestCase): _sleep.assert_not_called() @patch("apps.channels.tasks.time.sleep") - @patch("django.db.close_old_connections") + @patch("apps.channels.tasks.close_old_connections") def test_retries_on_operational_error_then_succeeds(self, mock_close, mock_sleep): """Retry succeeds on second attempt after OperationalError.""" call_count = {"n": 0} @@ -49,7 +49,7 @@ class DbRetryTests(TestCase): self.assertEqual(call_count["n"], 2) @patch("apps.channels.tasks.time.sleep") - @patch("django.db.close_old_connections") + @patch("apps.channels.tasks.close_old_connections") def test_raises_after_max_retries_exhausted(self, mock_close, mock_sleep): """Raises OperationalError after all retries fail.""" def always_fail(): @@ -59,7 +59,7 @@ class DbRetryTests(TestCase): _db_retry(always_fail, max_retries=3, base_interval=1) @patch("apps.channels.tasks.time.sleep") - @patch("django.db.close_old_connections") + @patch("apps.channels.tasks.close_old_connections") def test_exponential_backoff_timing(self, mock_close, mock_sleep): """Sleep durations follow exponential backoff: 1s, 2s, 4s.""" call_count = {"n": 0} @@ -74,7 +74,7 @@ class DbRetryTests(TestCase): mock_sleep.assert_has_calls([call(1), call(2)]) @patch("apps.channels.tasks.time.sleep") - @patch("django.db.close_old_connections") + @patch("apps.channels.tasks.close_old_connections") def test_close_old_connections_called_between_retries(self, mock_close, mock_sleep): """Stale DB connections are reset before each retry attempt.""" call_count = {"n": 0} @@ -89,7 +89,7 @@ class DbRetryTests(TestCase): mock_close.assert_called_once() @patch("apps.channels.tasks.time.sleep") - @patch("django.db.close_old_connections") + @patch("apps.channels.tasks.close_old_connections") def test_non_operational_error_not_retried(self, mock_close, mock_sleep): """Non-OperationalError exceptions propagate immediately.""" def raise_value_error(): @@ -100,14 +100,14 @@ class DbRetryTests(TestCase): mock_sleep.assert_not_called() @patch("apps.channels.tasks.time.sleep") - @patch("django.db.close_old_connections") + @patch("apps.channels.tasks.close_old_connections") def test_returns_fn_return_value(self, mock_close, mock_sleep): """Return value of fn() is passed through.""" result = _db_retry(lambda: {"key": "value"}, max_retries=3) self.assertEqual(result, {"key": "value"}) @patch("apps.channels.tasks.time.sleep") - @patch("django.db.close_old_connections") + @patch("apps.channels.tasks.close_old_connections") def test_single_retry_allowed(self, mock_close, mock_sleep): """max_retries=1 means no retry — fail immediately.""" with self.assertRaises(OperationalError): @@ -177,7 +177,7 @@ class FinalMetadataSaveRetryTests(TestCase): with patch.object(rec, "save", side_effect=patched_save): with patch("apps.channels.tasks.time.sleep"): - with patch("django.db.close_old_connections"): + with patch("apps.channels.tasks.close_old_connections"): def _save(): rec.custom_properties = cp rec.save(update_fields=["custom_properties"]) @@ -192,35 +192,19 @@ class FinalMetadataSaveRetryTests(TestCase): # --------------------------------------------------------------------------- class InitialConnectionRetryTests(TestCase): - """When a candidate base URL fails with a retriable error before data flows, - the DVR task must retry the same base before moving to the next candidate.""" + """Verify that the DVR task's reconnection logic retries the same + base URL before falling back to the next candidate.""" - def setUp(self): - self.channel = Channel.objects.create( - channel_number=96, name="Connection Retry Channel" - ) + def test_reconnect_max_constant_exists_in_run_recording(self): + """run_recording must define a max-reconnect limit to prevent + infinite retries on the same broken base URL.""" + import inspect + from apps.channels.tasks import run_recording + source = inspect.getsource(run_recording) - @patch("core.utils.send_websocket_update", side_effect=lambda *a, **kw: None) - def test_retriable_error_increments_reconnect_counter(self, _ws): - """Verify the reconnection counter logic: same base retried before fallback.""" - # This tests the logic structure directly rather than running the full task. - # The reconnection counter starts at 0, increments on each retriable error, - # and the loop 'continue's (retries same base) until max is exceeded. - max_reconnects = 5 - reconnects = 0 - attempts_on_base = 0 - - # Simulate the inner reconnection loop logic - while True: - attempts_on_base += 1 - # Simulate retriable error with no data - reconnects += 1 - if reconnects <= max_reconnects: - continue # Would retry same base - break # Would move to next base - - self.assertEqual(attempts_on_base, max_reconnects + 1) - self.assertEqual(reconnects, max_reconnects + 1) + # The reconnection counter pattern must be present + self.assertIn("reconnect", source.lower(), + "run_recording must contain reconnection logic") # --------------------------------------------------------------------------- @@ -263,7 +247,7 @@ class RecoveryRetryTests(TestCase): with patch.object(Recording, "save", patched_save): with patch("apps.channels.tasks.time.sleep"): - with patch("django.db.close_old_connections"): + with patch("apps.channels.tasks.close_old_connections"): _db_retry( lambda: rec.save(update_fields=["custom_properties"]), max_retries=3, diff --git a/apps/channels/tests/test_recording_pipeline.py b/apps/channels/tests/test_recording_pipeline.py new file mode 100644 index 00000000..b1362422 --- /dev/null +++ b/apps/channels/tests/test_recording_pipeline.py @@ -0,0 +1,524 @@ +"""Tests for recent DVR fixes. + +Covers: + 1. Collision avoidance: _build_output_paths checks both .mkv and .ts files + 2. Logo guard: _resolve_poster_for_program skips external APIs when title ≈ channel name + 3. Recording status lifecycle: status transitions visible via API + 4. Concat flags: error-tolerant ffmpeg flags used for segment concatenation + 5. Recovery skip-list: "recording" status NOT in terminal skip list +""" +import os +from datetime import timedelta +from unittest.mock import MagicMock, patch + +from django.test import TestCase +from django.utils import timezone +from rest_framework.test import APIRequestFactory, force_authenticate + +from apps.channels.models import Channel, Recording + + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + +def _make_admin(): + from django.contrib.auth import get_user_model + User = get_user_model() + u, _ = User.objects.get_or_create( + username="dvr_fixes_admin", + defaults={"user_level": User.UserLevel.ADMIN}, + ) + u.set_password("pass") + u.save() + return u + + +def _make_channel(name="Test Channel", number=100): + return Channel.objects.create(channel_number=number, name=name) + + +def _make_recording(channel, **overrides): + now = timezone.now() + defaults = { + "channel": channel, + "start_time": now - timedelta(hours=1), + "end_time": now + timedelta(hours=1), + "custom_properties": {}, + } + defaults.update(overrides) + return Recording.objects.create(**defaults) + + +# ========================================================================= +# 1. Collision avoidance — _build_output_paths +# ========================================================================= + +class CollisionAvoidanceTests(TestCase): + """_build_output_paths must increment the filename counter when + EITHER the .mkv OR the .ts file already exists with size > 0.""" + + def _call(self, channel, program, start, end): + from apps.channels.tasks import _build_output_paths + return _build_output_paths(channel, program, start, end) + + @patch("apps.channels.tasks.CoreSettings.get_dvr_tv_fallback_template", + return_value="TV/{show}/{start}.mkv") + @patch("apps.channels.tasks.CoreSettings.get_dvr_tv_template", + return_value="TV/{show}/S{season:02d}E{episode:02d}.mkv") + def test_no_collision_when_nothing_exists(self, _tv, _fb): + """Fresh path — no files exist, counter stays at 1.""" + ch = MagicMock(name="TestCh") + ch.name = "TestCh" + program = {"title": "My Show"} + now = timezone.now() + + def mock_stat(path): + raise OSError("No such file") + + with patch("os.stat", side_effect=mock_stat), \ + patch("os.makedirs"): + final, ts, fname = self._call(ch, program, now, now + timedelta(hours=1)) + + # Should NOT have a _2 suffix + self.assertNotIn("_2", final) + self.assertTrue(final.endswith(".mkv")) + + @patch("apps.channels.tasks.CoreSettings.get_dvr_tv_fallback_template", + return_value="TV/{show}/{start}.mkv") + @patch("apps.channels.tasks.CoreSettings.get_dvr_tv_template", + return_value="TV/{show}/S{season:02d}E{episode:02d}.mkv") + def test_collision_when_ts_exists_but_mkv_is_zero_bytes(self, _tv, _fb): + """Pre-restart scenario: MKV is 0-byte placeholder, TS has real data. + The old code only checked MKV size, so it would reuse the path. + The fix also checks TS, so it must increment.""" + ch = MagicMock(name="TestCh") + ch.name = "TestCh" + program = {"title": "My Show"} + now = timezone.now() + + def mock_stat(path): + if "_2" in path: + raise OSError("No such file") + result = MagicMock() + if path.endswith('.mkv'): + result.st_size = 0 # MKV is 0-byte placeholder + elif path.endswith('.ts'): + result.st_size = 5000000 # TS has real data from pre-restart + else: + result.st_size = 0 + return result + + with patch("os.stat", side_effect=mock_stat), \ + patch("os.makedirs"): + final, ts, fname = self._call(ch, program, now, now + timedelta(hours=1)) + + # Must have incremented to _2 + self.assertIn("_2", final, "Should increment counter when TS file has data") + + @patch("apps.channels.tasks.CoreSettings.get_dvr_tv_fallback_template", + return_value="TV/{show}/{start}.mkv") + @patch("apps.channels.tasks.CoreSettings.get_dvr_tv_template", + return_value="TV/{show}/S{season:02d}E{episode:02d}.mkv") + def test_collision_when_mkv_has_data(self, _tv, _fb): + """Standard collision: MKV file has data, should increment.""" + ch = MagicMock(name="TestCh") + ch.name = "TestCh" + program = {"title": "My Show"} + now = timezone.now() + + def mock_stat(path): + if "_2" in path: + raise OSError("No such file") + result = MagicMock() + if path.endswith('.mkv'): + result.st_size = 1000000 # MKV has data + else: + result.st_size = 0 + return result + + with patch("os.stat", side_effect=mock_stat), \ + patch("os.makedirs"): + final, ts, fname = self._call(ch, program, now, now + timedelta(hours=1)) + + self.assertIn("_2", final, "Should increment counter when MKV file has data") + + @patch("apps.channels.tasks.CoreSettings.get_dvr_tv_fallback_template", + return_value="TV/{show}/{start}.mkv") + @patch("apps.channels.tasks.CoreSettings.get_dvr_tv_template", + return_value="TV/{show}/S{season:02d}E{episode:02d}.mkv") + def test_no_collision_when_both_zero_bytes(self, _tv, _fb): + """Both MKV and TS exist but are 0 bytes — no collision.""" + ch = MagicMock(name="TestCh") + ch.name = "TestCh" + program = {"title": "My Show"} + now = timezone.now() + + def mock_stat(path): + result = MagicMock() + result.st_size = 0 # All files empty + return result + + with patch("os.stat", side_effect=mock_stat), \ + patch("os.makedirs"): + final, ts, fname = self._call(ch, program, now, now + timedelta(hours=1)) + + self.assertNotIn("_2", final, "Should NOT increment when all files are empty") + + @patch("apps.channels.tasks.CoreSettings.get_dvr_tv_fallback_template", + return_value="TV/{show}/{start}.mkv") + @patch("apps.channels.tasks.CoreSettings.get_dvr_tv_template", + return_value="TV/{show}/S{season:02d}E{episode:02d}.mkv") + def test_collision_increments_to_3_when_2_also_occupied(self, _tv, _fb): + """When both base and _2 are occupied, should go to _3.""" + ch = MagicMock(name="TestCh") + ch.name = "TestCh" + program = {"title": "My Show"} + now = timezone.now() + + def mock_stat(path): + if "_3" in path: + raise OSError("No such file") + result = MagicMock() + if path.endswith('.ts'): + result.st_size = 5000000 + else: + result.st_size = 0 + return result + + with patch("os.stat", side_effect=mock_stat), \ + patch("os.makedirs"): + final, ts, fname = self._call(ch, program, now, now + timedelta(hours=1)) + + self.assertIn("_3", final, "Should increment to _3 when base and _2 are occupied") + + +# ========================================================================= +# 2. Logo guard — _resolve_poster_for_program +# ========================================================================= + +class LogoGuardTests(TestCase): + """When the program title matches the channel name, external API + searches (VOD, TMDB, OMDb, TVMaze, iTunes) must be skipped.""" + + def _call(self, channel_name, program, channel_logo_id=None): + from apps.channels.tasks import _resolve_poster_for_program + return _resolve_poster_for_program(channel_name, program, channel_logo_id) + + @patch("apps.channels.tasks.requests.get") + def test_channel_name_as_title_skips_external_apis(self, mock_get): + """Title = 'USA A&E SD*', channel = 'USA A&E SD*' → no external calls.""" + program = {"title": "USA A&E SD*"} + logo_id, url = self._call("USA A&E SD*", program, channel_logo_id=42) + + # Should NOT have called any external APIs + mock_get.assert_not_called() + # Should fall back to channel logo + self.assertEqual(logo_id, 42) + self.assertIsNone(url) + + @patch("apps.channels.tasks.requests.get") + def test_channel_name_normalized_match(self, mock_get): + """Title = 'fox news', channel = 'FOX-News*' → normalized match, skip APIs.""" + program = {"title": "fox news"} + logo_id, url = self._call("FOX-News*", program, channel_logo_id=99) + + mock_get.assert_not_called() + self.assertEqual(logo_id, 99) + + @patch("apps.channels.tasks.requests.get") + def test_real_title_still_searched(self, mock_get): + """Title = 'Breaking Bad' on channel 'AMC' → should try external APIs.""" + # Mock TVMaze returning a result + mock_resp = MagicMock(ok=True, status_code=200) + mock_resp.json.return_value = { + "image": {"original": "https://tvmaze.com/breaking-bad.jpg"} + } + mock_get.return_value = mock_resp + + program = {"title": "Breaking Bad"} + logo_id, url = self._call("AMC", program) + + # Should have made at least one external API call + self.assertTrue(mock_get.called, "Should search external APIs for real titles") + self.assertIsNotNone(url) + + @patch("apps.channels.tasks.requests.get") + def test_no_title_skips_to_channel_logo(self, mock_get): + """No title at all → falls through to channel logo, no API calls.""" + program = {} + logo_id, url = self._call("SomeChannel", program, channel_logo_id=55) + + mock_get.assert_not_called() + self.assertEqual(logo_id, 55) + + @patch("apps.channels.tasks.requests.get") + def test_epg_image_still_used_even_when_title_is_channel_name(self, mock_get): + """Even when title = channel name, Stage 1 (EPG images) should still work.""" + from apps.epg.models import ProgramData, EPGSource, EPGData + + # Create an EPG source + EPGData entry + program with an icon URL + epg_source = EPGSource.objects.create(source_type="xmltv", name="Test EPG") + epg_data = EPGData.objects.create(tvg_id="test.ch", epg_source=epg_source) + prog = ProgramData.objects.create( + epg=epg_data, + title="Test Channel HD", + start_time=timezone.now() - timedelta(hours=1), + end_time=timezone.now() + timedelta(hours=1), + custom_properties={"icon": "https://epg-cdn.com/test-icon.png"}, + ) + + program = {"title": "Test Channel HD", "id": prog.id} + + # Mock _validate_url to return True for the icon URL + with patch("apps.channels.tasks._validate_url", return_value=True): + logo_id, url = self._call("Test Channel HD", program, channel_logo_id=10) + + # EPG icon should still be used (Stage 1 doesn't depend on title guard) + self.assertEqual(url, "https://epg-cdn.com/test-icon.png") + mock_get.assert_not_called() + + +# ========================================================================= +# 3. Recording status lifecycle via API +# ========================================================================= + +class RecordingStatusLifecycleTests(TestCase): + """Verify recording status transitions and that terminal recordings + are properly filterable (supports the red-dot fix in guideUtils).""" + + def setUp(self): + self.channel = _make_channel("Status Test Channel", 200) + self.user = _make_admin() + self.factory = APIRequestFactory() + + def _list_recordings(self): + from apps.channels.api_views import RecordingViewSet + request = self.factory.get("/api/channels/recordings/") + force_authenticate(request, user=self.user) + view = RecordingViewSet.as_view({"get": "list"}) + return view(request) + + @patch("core.utils.send_websocket_update", side_effect=lambda *a, **kw: None) + def test_stopped_recording_has_terminal_status(self, _ws): + """After stop, custom_properties.status = 'stopped'.""" + from apps.channels.api_views import RecordingViewSet + + rec = _make_recording(self.channel, custom_properties={ + "status": "recording", + "program": {"id": 1, "title": "Live Show"}, + }) + + request = self.factory.post(f"/api/channels/recordings/{rec.id}/stop/") + force_authenticate(request, user=self.user) + view = RecordingViewSet.as_view({"post": "stop"}) + + with patch("apps.channels.signals.revoke_task"): + response = view(request, pk=rec.id) + + self.assertIn(response.status_code, [200, 204]) + rec.refresh_from_db() + self.assertEqual(rec.custom_properties.get("status"), "stopped") + + def test_listing_includes_status_in_custom_properties(self): + """API listing returns custom_properties with status field.""" + _make_recording(self.channel, custom_properties={ + "status": "recording", + "program": {"id": 1, "title": "Recording Show"}, + }) + _make_recording(self.channel, custom_properties={ + "status": "stopped", + "program": {"id": 2, "title": "Stopped Show"}, + }) + + response = self._list_recordings() + self.assertEqual(response.status_code, 200) + + statuses = [r["custom_properties"].get("status") for r in response.data] + self.assertIn("recording", statuses) + self.assertIn("stopped", statuses) + + @patch("core.utils.send_websocket_update", side_effect=lambda *a, **kw: None) + def test_delete_recording_removes_from_listing(self, _ws): + """Deleting a recording removes it from the listing entirely.""" + from apps.channels.api_views import RecordingViewSet + + rec = _make_recording(self.channel, custom_properties={ + "status": "stopped", + "program": {"id": 3, "title": "To Delete"}, + }) + rec_id = rec.id + + request = self.factory.delete(f"/api/channels/recordings/{rec_id}/") + force_authenticate(request, user=self.user) + view = RecordingViewSet.as_view({"delete": "destroy"}) + + with patch("apps.channels.signals.revoke_task"): + response = view(request, pk=rec_id) + + self.assertIn(response.status_code, [200, 204]) + self.assertFalse(Recording.objects.filter(id=rec_id).exists()) + + +# ========================================================================= +# 4. Concat flags — error-tolerant ffmpeg +# ========================================================================= + +class ConcatFlagsTests(TestCase): + """Verify that the finalize phase uses error-tolerant ffmpeg flags + when concatenating pre-restart segments.""" + + def test_concat_command_includes_error_tolerant_flags(self): + """Inspect the source code to confirm error-tolerant flags are present. + This is a static analysis test — no ffmpeg execution needed.""" + import inspect + from apps.channels.tasks import run_recording + source = inspect.getsource(run_recording) + + # The concat subprocess.run call must include these flags + self.assertIn("+genpts+igndts+discardcorrupt", source, + "Concat must use +genpts+igndts+discardcorrupt fflags") + self.assertIn("ignore_err", source, + "Concat must use -err_detect ignore_err") + self.assertIn("-f", source) + self.assertIn("concat", source) + + def test_concat_goes_directly_to_mkv(self): + """Concat must produce MKV directly (not intermediate .ts) to + preserve timestamp boundaries and avoid playback freeze at splice.""" + import inspect + from apps.channels.tasks import run_recording + source = inspect.getsource(run_recording) + + # Must contain reset_timestamps for proper segment boundary handling + self.assertIn("reset_timestamps", source, + "Concat must use -reset_timestamps 1 for seamless seeking") + # Must write directly to final_path (MKV), not an intermediate .ts + self.assertIn("_concat_did_remux", source, + "Concat path must set flag to skip separate remux step") + + def test_segment_time_metadata_present(self): + """Verify concat uses -segment_time_metadata for boundary awareness.""" + import inspect + from apps.channels.tasks import run_recording + source = inspect.getsource(run_recording) + + self.assertIn("segment_time_metadata", source, + "Concat must use -segment_time_metadata 1 for segment boundary handling") + + +# ========================================================================= +# 5. Recovery skip-list +# ========================================================================= + +class RecoverySkipListTests(TestCase): + """Verify that the recovery function does NOT skip 'recording' status, + since that's the exact status recordings have when the server crashes.""" + + def test_recording_status_not_in_skip_list(self): + """Inspect recover_recordings_on_startup to ensure 'recording' is + NOT treated as a terminal/skip state.""" + import inspect + from apps.channels.tasks import recover_recordings_on_startup + source = inspect.getsource(recover_recordings_on_startup) + + # Find the skip condition line + # It should be: if current_status in ("completed", "stopped"): + # NOT: if current_status in ("completed", "stopped", "recording"): + lines = source.split('\n') + skip_line = None + for line in lines: + if 'current_status in' in line and ('completed' in line or 'stopped' in line): + skip_line = line.strip() + break + + self.assertIsNotNone(skip_line, "Should find the skip-list condition") + self.assertNotIn('"recording"', skip_line, + "Skip list must NOT contain 'recording' — " + "that's the status of crashed mid-stream recordings that need recovery") + + @patch("core.utils.RedisClient") + @patch("apps.channels.tasks.run_recording") + @patch("core.utils.send_websocket_update", side_effect=lambda *a, **kw: None) + def test_recovery_processes_recording_status(self, _ws, mock_run, mock_redis_cls): + """A recording with status='recording' should be recovered, not skipped.""" + mock_redis_conn = MagicMock() + mock_redis_conn.set.return_value = True # Acquire lock + mock_redis_cls.get_client.return_value = mock_redis_conn + + channel = _make_channel("Recovery Test", 300) + now = timezone.now() + rec = _make_recording(channel, custom_properties={ + "status": "recording", + "program": {"title": "Crashed Show"}, + }, end_time=now + timedelta(hours=2)) + + from apps.channels.tasks import recover_recordings_on_startup + + with patch("apps.channels.signals.revoke_task"): + result = recover_recordings_on_startup() + + # The recording should have been dispatched for recovery + self.assertTrue(mock_run.apply_async.called, + "Recording with status='recording' should be dispatched for recovery") + + @patch("core.utils.RedisClient") + @patch("apps.channels.tasks.run_recording") + @patch("core.utils.send_websocket_update", side_effect=lambda *a, **kw: None) + def test_recovery_skips_stopped_recordings(self, _ws, mock_run, mock_redis_cls): + """A recording with status='stopped' should be skipped by recovery.""" + mock_redis_conn = MagicMock() + mock_redis_conn.set.return_value = True + mock_redis_cls.get_client.return_value = mock_redis_conn + + channel = _make_channel("Recovery Skip Test", 301) + now = timezone.now() + rec = _make_recording(channel, custom_properties={ + "status": "stopped", + "program": {"title": "Finished Show"}, + }, end_time=now + timedelta(hours=2)) + + from apps.channels.tasks import recover_recordings_on_startup + with patch("apps.channels.signals.revoke_task"): + recover_recordings_on_startup() + + # Should NOT have dispatched a recovery task + mock_run.apply_async.assert_not_called() + + +# ========================================================================= +# 6. Frontend red-dot filter (guideUtils.mapRecordingsByProgramId) +# ========================================================================= + +class MapRecordingsByProgramIdTests(TestCase): + """These test the BACKEND side — confirming that recording status + is preserved in the API response so the frontend can filter on it. + + The actual frontend filtering is covered by frontend/src/pages/__tests__/DVR.test.jsx + and the guideUtils code, but we verify the data contract here.""" + + def test_recording_custom_properties_status_persisted(self): + """Recording status in custom_properties survives save/load cycle.""" + channel = _make_channel("Red Dot Test", 400) + rec = _make_recording(channel, custom_properties={ + "status": "stopped", + "program": {"id": 42, "title": "A Show"}, + }) + + rec.refresh_from_db() + self.assertEqual(rec.custom_properties["status"], "stopped") + + def test_terminal_statuses_are_well_defined(self): + """Verify the terminal status set matches what the frontend uses.""" + # These are the statuses that should NOT show a red dot in the Guide + terminal = {"stopped", "completed", "interrupted", "failed"} + channel = _make_channel("Terminal Status Test", 410) + + # Verify each status is a valid recording status + for status in terminal: + rec = _make_recording(channel, custom_properties={ + "status": status, + "program": {"id": 100, "title": "Test"}, + }) + rec.refresh_from_db() + self.assertEqual(rec.custom_properties["status"], status) diff --git a/apps/channels/tests/test_recording_stop_cancel.py b/apps/channels/tests/test_recording_stop_cancel.py index 962160ee..e4b22be4 100644 --- a/apps/channels/tests/test_recording_stop_cancel.py +++ b/apps/channels/tests/test_recording_stop_cancel.py @@ -7,7 +7,6 @@ Covers: - run_recording race guard before status write - _stop_dvr_clients() DVR client isolation """ -import time from datetime import timedelta from unittest.mock import MagicMock, AsyncMock, patch @@ -82,13 +81,29 @@ class StopEndpointTests(TestCase): self.assertIn("stopped_at", rec.custom_properties) def test_stop_calls_stop_dvr_clients_in_background(self): + """stop() spawns a background thread whose target calls _stop_dvr_clients.""" rec = self._make_rec() + with patch("apps.channels.api_views._stop_dvr_clients", return_value=1) as mock_stop, \ - patch("core.utils.send_websocket_update"): + patch("core.utils.send_websocket_update"), \ + patch("threading.Thread") as mock_thread: + mock_thread.return_value.start = MagicMock() self._stop(rec) - time.sleep(0.5) - self.assertTrue(mock_stop.called) - args, kwargs = mock_stop.call_args + + # Verify a daemon thread was spawned + mock_thread.assert_called_once() + thread_kwargs = mock_thread.call_args[1] + self.assertTrue(thread_kwargs.get("daemon"), "Thread must be daemon") + + # Execute the captured target with DB connection close patched out + target = thread_kwargs["target"] + with patch("apps.channels.api_views._stop_dvr_clients", return_value=1) as mock_stop2, \ + patch("apps.channels.signals.revoke_task", side_effect=Exception("skip")), \ + patch("django.db.connection") as mock_conn: + target() + + self.assertTrue(mock_stop2.called) + args, kwargs = mock_stop2.call_args actual_rec_id = kwargs.get("recording_id") or (args[1] if len(args) > 1 else None) self.assertEqual(actual_rec_id, rec.id) diff --git a/dispatcharr/celery.py b/dispatcharr/celery.py index c845dafe..436ffc35 100644 --- a/dispatcharr/celery.py +++ b/dispatcharr/celery.py @@ -2,7 +2,7 @@ import os from celery import Celery import logging -from celery.signals import task_postrun # Add import for signals +from celery.signals import task_postrun, worker_ready # Initialize with defaults before Django settings are loaded DEFAULT_LOG_LEVEL = 'DEBUG' @@ -149,3 +149,10 @@ def setup_celery_logging(**kwargs): except (AttributeError, TypeError): # If the log level string is invalid, default to DEBUG logger.setLevel(logging.DEBUG) + + +@worker_ready.connect +def on_worker_ready(**kwargs): + """Resume or finalize interrupted DVR recordings after a worker restart.""" + from apps.channels.tasks import recover_recordings_on_startup + recover_recordings_on_startup.delay() diff --git a/frontend/src/components/ErrorBoundary.jsx b/frontend/src/components/ErrorBoundary.jsx index 83fa4de7..b2de253d 100644 --- a/frontend/src/components/ErrorBoundary.jsx +++ b/frontend/src/components/ErrorBoundary.jsx @@ -4,12 +4,16 @@ class ErrorBoundary extends React.Component { state = { hasError: false }; static getDerivedStateFromError(error) { - return { hasError: true }; + return { hasError: true, error }; + } + + componentDidCatch(error, errorInfo) { + console.error('ErrorBoundary caught:', error, errorInfo); } render() { if (this.state.hasError) { - return