diff --git a/modules/videos/common.py b/modules/videos/common.py
index 4008b77d..1c450f5d 100644
--- a/modules/videos/common.py
+++ b/modules/videos/common.py
@@ -3,6 +3,7 @@ import os
import pathlib
import subprocess
import tempfile
+from decimal import Decimal
from functools import partial
from pathlib import Path
from typing import Union, Tuple, List, Set, Iterable, Optional
@@ -254,10 +255,38 @@ def get_video_duration(video_path: Path) -> Optional[int]:
logger.warning(f'FFPROBE failed to get duration with stderr: {e.stderr.decode()}')
raise
stdout = proc.stdout.decode()
- duration = int(float(stdout.strip()))
+ duration = int(Decimal(stdout.strip()))
return duration
+def check_for_video_corruption(video_path: Path) -> bool:
+ """Uses ffprobe to check for specific ways a video file can be corrupt."""
+ if not isinstance(video_path, Path):
+ video_path = Path(video_path)
+ if not video_path.is_file():
+ raise FileNotFoundError(f'{video_path} does not exist!')
+
+ cmd = [FFPROBE_BIN, str(video_path)]
+ try:
+ proc = subprocess.run(cmd, check=True, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
+ except subprocess.CalledProcessError as e:
+ logger.warning(f'FFPROBE failed to check for corruption with stdout: {e.stdout.decode()}')
+ logger.warning(f'FFPROBE failed to check for corruption with stderr: {e.stderr.decode()}')
+ return True # video is corrupt.
+
+ messages = (
+ 'Invalid NAL unit size',
+ 'Error splitting the input into NAL units',
+ )
+ stderr = proc.stderr.decode()
+ corrupt = False
+ for error in messages:
+ if error in stderr:
+ logger.warning(f'Possible video corruption ({error}): {video_path}')
+ corrupt = True
+ return corrupt
+
+
def apply_info_json(channel_id: int):
"""Update view_count for all Videos in a channel using its info_json file.
diff --git a/modules/videos/downloader.py b/modules/videos/downloader.py
index 9728ed9b..2f1b9cf7 100755
--- a/modules/videos/downloader.py
+++ b/modules/videos/downloader.py
@@ -45,7 +45,7 @@ PREFERRED_VIDEO_FORMAT = 'best[height=720],22,720p,mp4-480p,mp4-360p,mp4-240p,18
def extract_info(url: str, ydl: YoutubeDL = YDL) -> dict:
"""Get info about a video. Separated for testing."""
- return ydl.extract_info(url, download=False, process=False)
+ return ydl.extract_info(url, download=False, process=True)
def prepare_filename(entry: dict, ydl: YoutubeDL = YDL) -> str:
@@ -254,6 +254,7 @@ class VideoDownloader(Downloader, ABC):
options = get_downloader_config().dict()
options['outtmpl'] = f'{out_dir}/{options["file_name_format"]}'
options['merge_output_format'] = PREFERRED_VIDEO_EXTENSION
+ logger.debug(f'Download video with options: {options}')
logger.debug(f'Downloading {url} to {out_dir}')
@@ -265,10 +266,8 @@ class VideoDownloader(Downloader, ABC):
# Get the path where the video will be saved.
entry = extract_info(url, ydl=ydl)
final_filename = pathlib.Path(prepare_filename(entry, ydl=ydl)).absolute()
- if final_filename.suffix == '.NA':
- # yt-dlp has odd behavior sometimes and ignores our preferred video extension.
- final_filename = final_filename.with_suffix('.mp4')
- logger.warning(f'Had to replace video extension: {final_filename}')
+ if final_filename.suffix.lower() != f'.{PREFERRED_VIDEO_EXTENSION}':
+ raise DownloadError(f'Cannot download video {url} because yt-dlp filename is invalid.')
return final_filename, entry
diff --git a/modules/videos/lib.py b/modules/videos/lib.py
index 885e265f..651b9b86 100644
--- a/modules/videos/lib.py
+++ b/modules/videos/lib.py
@@ -18,7 +18,7 @@ from wrolpi.vars import PYTEST
from .captions import get_captions
from .common import generate_video_paths, remove_duplicate_video_paths, apply_info_json, get_video_duration, \
is_valid_poster, convert_image, generate_video_poster, logger, REQUIRED_OPTIONS, ConfigError, \
- get_no_channel_directory
+ get_no_channel_directory, check_for_video_corruption
from .models import Channel, Video
logger = logger.getChild(__name__)
@@ -181,6 +181,9 @@ def validate_video(video: Video, channel_generate_poster: bool):
video_path = video.video_path.path if isinstance(video.video_path, MediaPath) else video.video_path
+ # Check for corruption, but don't throw away the video even if it is corrupt. Errors will be logged in this call.
+ check_for_video_corruption(video_path)
+
if not video.title or not video.upload_date or not video.source_id:
# Video is missing things that can be extracted from the video file name.
# These are the least trusted, so anything already on the video should be trusted.
diff --git a/modules/videos/test/test_common.py b/modules/videos/test/test_common.py
index 059c5246..25ee41eb 100644
--- a/modules/videos/test/test_common.py
+++ b/modules/videos/test/test_common.py
@@ -14,6 +14,7 @@ from wrolpi.dates import local_timezone, now
from wrolpi.downloader import Download, DownloadFrequency
from wrolpi.test.common import build_test_directories, TestAPI
from wrolpi.vars import PROJECT_DIR
+from .. import common
from ..common import get_matching_directories, convert_image, remove_duplicate_video_paths, \
apply_info_json, get_video_duration, generate_video_poster, is_valid_poster
from ..lib import save_channels_config, get_channels_config, import_channels_config
@@ -414,3 +415,29 @@ def test_import_channels_config_outdated(test_session, test_directory, channel_f
# Videos were favorited by their channel "link".
assert vid1.favorite and vid2.favorite and vid3.favorite
assert not vid4.favorite
+
+
+def test_check_for_video_corruption(video_file, test_directory):
+ # The test video is not corrupt.
+ assert common.check_for_video_corruption(video_file) is False
+
+ # An empty file is corrupt.
+ empty_file = test_directory / 'empty file.mp4'
+ empty_file.touch()
+ assert common.check_for_video_corruption(empty_file) is True
+
+ # A video file must be complete.
+ truncated_video = test_directory / 'truncated_video.mp4'
+ with truncated_video.open('wb') as fh:
+ fh.write(video_file.read_bytes()[:10000])
+ assert common.check_for_video_corruption(truncated_video) is True
+
+ # Check for specific ffprobe errors.
+ with mock.patch('modules.videos.common.subprocess') as mock_subprocess:
+ # `video_file` is ignored for these calls.
+ mock_subprocess.run().stderr.decode.return_value = 'Something\nInvalid NAL unit size'
+ assert common.check_for_video_corruption(video_file) is True
+ mock_subprocess.run().stderr.decode.return_value = 'Something\nError splitting the input into NAL units'
+ assert common.check_for_video_corruption(video_file) is True
+ mock_subprocess.run().stderr.decode.return_value = 'Some stderr is fine'
+ assert common.check_for_video_corruption(video_file) is False
diff --git a/wrolpi/admin.py b/wrolpi/admin.py
index c1ea968a..37b5c258 100644
--- a/wrolpi/admin.py
+++ b/wrolpi/admin.py
@@ -54,6 +54,7 @@ def enable_hotspot():
logger.warning(f'Hotspot status: {status}')
if status == HotspotStatus.connected:
+ logger.info('Hotspot already connected. Restarting hotspot.')
disable_hotspot()
return enable_hotspot()
elif status == HotspotStatus.disconnected:
@@ -61,11 +62,13 @@ def enable_hotspot():
cmd = (SUDO, NMCLI, 'device', 'wifi', 'hotspot', 'ifname', 'wlan0',
'ssid', WROLPI_CONFIG.hotspot_ssid, 'password', WROLPI_CONFIG.hotspot_password)
subprocess.check_call(cmd)
+ logger.info('Enabling hotspot successful')
return True
elif status == HotspotStatus.unavailable:
# Radio is not on, turn it on.
cmd = (SUDO, NMCLI, 'radio', 'wifi', 'on')
subprocess.check_call(cmd)
+ logger.info('Turned on hotspot radio.')
return enable_hotspot()
elif status == HotspotStatus.unknown:
logger.error('Cannot enable hotspot with unknown status!')
@@ -76,6 +79,7 @@ def disable_hotspot():
cmd = (SUDO, NMCLI, 'radio', 'wifi', 'off')
try:
subprocess.check_call(cmd)
+ logger.info('Disabling hotspot successful')
return True
except Exception as e:
logger.error('Failed to disable hotspot', exc_info=e)
diff --git a/wrolpi/downloader.py b/wrolpi/downloader.py
index 3da16b10..55ab3551 100644
--- a/wrolpi/downloader.py
+++ b/wrolpi/downloader.py
@@ -182,6 +182,7 @@ class Downloader:
"""
Run a subprocess using the provided arguments. This process can be killed by the Download Manager.
"""
+ logger.debug(f'{self} launching download process with args: {" ".join(cmd)}')
proc = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE, cwd=cwd, **kwargs)
pid = proc.pid
logger.debug(f'{self} launched download process {pid=} for {url}')
@@ -370,6 +371,9 @@ class DownloadManager:
logger.warning(f'Failed to download {url}. Will be tried again later.', exc_info=e)
result = DownloadResult(success=False, error=str(traceback.format_exc()))
+ error_len = len(result.error) if result.error else 0
+ logger.debug(f'Got {result.success=} from {downloader} with {error_len=}')
+
# Long-running, get the download again.
with get_db_session(commit=True) as session_:
download = self.get_download(session_, id_=download_id)
@@ -377,7 +381,7 @@ class DownloadManager:
# clear out an outdated location.
download.location = result.location or download.location or None
# Clear any old errors if the download succeeded.
- download.error = result.error if download.error else None
+ download.error = result.error if result.error else None
download.next_download = self.get_next_download(download, session)
if try_again is False:
diff --git a/wrolpi/root_api.py b/wrolpi/root_api.py
index f9172104..cebfde69 100644
--- a/wrolpi/root_api.py
+++ b/wrolpi/root_api.py
@@ -17,6 +17,7 @@ from sanic_ext import validate
from sanic_ext.extensions.openapi import openapi
from wrolpi import admin
+from wrolpi.admin import HotspotStatus
from wrolpi.common import set_sanic_url_parts, logger, get_config, wrol_mode_enabled, Base, get_media_directory, \
wrol_mode_check, native_only, set_wrol_mode
from wrolpi.dates import set_timezone
@@ -158,12 +159,16 @@ def update_settings(_: Request, body: SettingsRequest):
# Remove any keys with None values, then save the config.
config = {k: v for k, v in body.__dict__.items() if v is not None}
wrolpi_config = get_config()
+ old_password = wrolpi_config.hotspot_password
wrolpi_config.update(config)
if body.wrol_mode:
download_manager.kill()
- if body.hotspot_status is True:
+ # If the password was changed, we need to restart the hotspot.
+ password_changed = (new_password := config.get('hotspot_password')) and old_password != new_password
+
+ if body.hotspot_status is True or (password_changed and admin.hotspot_status() == HotspotStatus.connected):
# Turn on Hotspot
if admin.enable_hotspot() is False:
raise HotspotError('Could not turn on hotspot')
diff --git a/wrolpi/test/test_root_api.py b/wrolpi/test/test_root_api.py
index a50dce75..5d973035 100644
--- a/wrolpi/test/test_root_api.py
+++ b/wrolpi/test/test_root_api.py
@@ -4,6 +4,7 @@ from itertools import zip_longest
from mock import mock
+from wrolpi.admin import HotspotStatus
from wrolpi.common import get_config
from wrolpi.dates import strptime
from wrolpi.db import get_db_session
@@ -197,8 +198,9 @@ def test_hotspot_settings(test_session, test_client, test_config):
assert response.json['code'] == 35
mock_admin.disable_hotspot.assert_called_once()
- config = get_config()
- assert config.hotspot_on_startup is True
+ mock_admin.enable_hotspot.reset_mock()
+ mock_admin.enable_hotspot.return_value = True
+ mock_admin.hotspot_status.return_value = HotspotStatus.connected
# Hotspot password can be changed.
mock_admin.disable_hotspot.return_value = True
@@ -207,6 +209,8 @@ def test_hotspot_settings(test_session, test_client, test_config):
assert response.status_code == HTTPStatus.NO_CONTENT
assert config.hotspot_password == 'new password'
assert config.hotspot_ssid == 'new ssid'
+ # Changing the password restarts the hotspot.
+ mock_admin.enable_hotspot.assert_called_once()
@skip_circleci