From 21d6fa802bdc25c9d69af89d2e85c27d1dcf339e Mon Sep 17 00:00:00 2001 From: JamBalaya56562 Date: Wed, 1 Jul 2026 07:45:24 +0900 Subject: [PATCH 1/3] fix(ui): hide disabled media browser buttons --- motioneye/static/js/main.js | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/motioneye/static/js/main.js b/motioneye/static/js/main.js index a45cf24b9..d833d0e98 100644 --- a/motioneye/static/js/main.js +++ b/motioneye/static/js/main.js @@ -4835,6 +4835,12 @@ function addCameraFrameUi(cameraConfig) { picturesButton.hide(); moviesButton.hide(); } + else if (!cameraConfig['still_images']) { + picturesButton.hide(); + } + if (cameraConfig['proto'] != 'mjpeg' && !cameraConfig['movies']) { + moviesButton.hide(); + } cameraFrameDiv.attr('id', 'camera' + cameraId); cameraFrameDiv[0].refreshDivider = 0; From 930afd8a8b0a78bb3cfbfd83507931c47e28a6a6 Mon Sep 17 00:00:00 2001 From: JamBalaya56562 Date: Wed, 1 Jul 2026 07:45:24 +0900 Subject: [PATCH 2/3] address review: keep the media button when old files still exist Per @Marijn0's review: hiding a media-browser button purely on the capture-mode flag also cut off access to media recorded before the mode was disabled. Now, when a capture mode is disabled, the button is hidden only after an async check of the existing list endpoint (with_stat=false) finds no files of that type. Enabled modes issue no request, and simple MJPEG cameras still hide both buttons. Co-Authored-By: Claude Opus 4.8 --- motioneye/static/js/main.js | 26 +++++++++++++++++++++----- 1 file changed, 21 insertions(+), 5 deletions(-) diff --git a/motioneye/static/js/main.js b/motioneye/static/js/main.js index d833d0e98..b6f1affda 100644 --- a/motioneye/static/js/main.js +++ b/motioneye/static/js/main.js @@ -4719,6 +4719,17 @@ function runMediaDialog(cameraId, mediaType) { /* camera frames */ +function hideMediaButtonIfEmpty(cameraId, mediaType, button) { + /* the list endpoint returns {mediaList: [...]} (empty when there are no + * files); with_stat=false keeps the request cheap. On error the button is + * left visible (safe default - don't hide when we are unsure). */ + ajax('GET', basePath + mediaType + '/' + cameraId + '/list/?with_stat=false', null, function (data) { + if (data && data.mediaList && !data.mediaList.length) { + button.hide(); + } + }); +} + function addCameraFrameUi(cameraConfig) { var cameraId = cameraConfig.id; @@ -4835,11 +4846,16 @@ function addCameraFrameUi(cameraConfig) { picturesButton.hide(); moviesButton.hide(); } - else if (!cameraConfig['still_images']) { - picturesButton.hide(); - } - if (cameraConfig['proto'] != 'mjpeg' && !cameraConfig['movies']) { - moviesButton.hide(); + else { + /* when a capture mode is disabled, only hide its media-browser button + * if there are no existing files left to browse, so media recorded + * before disabling capture stays accessible (#2731) */ + if (!cameraConfig['still_images']) { + hideMediaButtonIfEmpty(cameraId, 'picture', picturesButton); + } + if (!cameraConfig['movies']) { + hideMediaButtonIfEmpty(cameraId, 'movie', moviesButton); + } } cameraFrameDiv.attr('id', 'camera' + cameraId); From c23c3d8ad9169ffd5c2d8f0ba2695da298bba864 Mon Sep 17 00:00:00 2001 From: JamBalaya56562 Date: Thu, 2 Jul 2026 08:47:34 +0900 Subject: [PATCH 3/3] address review: cap media list existence checks --- motioneye/handlers/movie.py | 21 ++- motioneye/handlers/picture.py | 21 ++- motioneye/mediafiles.py | 64 +++++--- motioneye/remote.py | 13 +- motioneye/static/js/main.js | 8 +- tests/test_handlers/test_media_list_limit.py | 67 ++++++++ tests/test_mediafiles.py | 161 +++++++++++++++++++ tests/test_remote.py | 40 +++++ 8 files changed, 359 insertions(+), 36 deletions(-) create mode 100644 tests/test_handlers/test_media_list_limit.py diff --git a/motioneye/handlers/movie.py b/motioneye/handlers/movie.py index 435cc4602..3a3031f10 100644 --- a/motioneye/handlers/movie.py +++ b/motioneye/handlers/movie.py @@ -116,16 +116,27 @@ async def list(self, camera_id): logging.debug(f'listing movies for camera {camera_id}') camera_config = config.get_camera(camera_id) - if utils.is_local_motion_camera(camera_config): - # Get with_stat parameter from query string, default to True - # Only 'false' is treated as false, everything else is true - with_stat = self.get_argument('with_stat', 'true').lower() != 'false' + # Get with_stat parameter from query string, default to True + # Only 'false' is treated as false, everything else is true + with_stat = self.get_argument('with_stat', 'true').lower() != 'false' + + # An optional positive 'limit' makes the listing stop early once that + # many files were found, turning it into a cheap existence check + try: + limit = int(self.get_argument('limit', '')) + except ValueError: + limit = None + if limit is not None and limit <= 0: + limit = None + + if utils.is_local_motion_camera(camera_config): media_list = await mediafiles.list_media( camera_config, media_type='movie', prefix=self.get_argument('prefix', None), with_stat=with_stat, + limit=limit, ) if media_list is None: return self.finish_json({'error': 'Failed to get movies list.'}) @@ -139,6 +150,8 @@ async def list(self, camera_id): camera_config, media_type='movie', prefix=self.get_argument('prefix', None), + with_stat=with_stat, + limit=limit, ) if resp.error: return self.finish_json( diff --git a/motioneye/handlers/picture.py b/motioneye/handlers/picture.py index 32e0b88b7..cef0784be 100644 --- a/motioneye/handlers/picture.py +++ b/motioneye/handlers/picture.py @@ -213,16 +213,27 @@ async def list(self, camera_id): logging.debug(f'listing pictures for camera {camera_id}') camera_config = config.get_camera(camera_id) - if utils.is_local_motion_camera(camera_config): - # Get with_stat parameter from query string, default to True - # Only 'false' is treated as false, everything else is true - with_stat = self.get_argument('with_stat', 'true').lower() != 'false' + # Get with_stat parameter from query string, default to True + # Only 'false' is treated as false, everything else is true + with_stat = self.get_argument('with_stat', 'true').lower() != 'false' + + # An optional positive 'limit' makes the listing stop early once that + # many files were found, turning it into a cheap existence check + try: + limit = int(self.get_argument('limit', '')) + except ValueError: + limit = None + if limit is not None and limit <= 0: + limit = None + + if utils.is_local_motion_camera(camera_config): media_list = await mediafiles.list_media( camera_config, media_type='picture', prefix=self.get_argument('prefix', None), with_stat=with_stat, + limit=limit, ) if media_list is None: return self.finish_json({'error': 'Failed to get movies list.'}) @@ -236,6 +247,8 @@ async def list(self, camera_id): camera_config, media_type='picture', prefix=self.get_argument('prefix', None), + with_stat=with_stat, + limit=limit, ) if resp.error: return self.finish_json( diff --git a/motioneye/mediafiles.py b/motioneye/mediafiles.py index 74e0f43d8..630199682 100644 --- a/motioneye/mediafiles.py +++ b/motioneye/mediafiles.py @@ -109,6 +109,7 @@ def _list_media_files( exts: List[str], sub_path: Optional[str] = None, with_stat: bool = True, + limit: Optional[int] = None, ) -> List[tuple]: # Determine scan path based on sub_path parameter if sub_path is not None: @@ -121,33 +122,46 @@ def _list_media_files( else: scan_path = base_path - media_files = [] - for entry in os.scandir(scan_path): - # ignore hidden files/dirs and other unwanted files - if entry.name.startswith('.') or entry.name == 'lastsnap.jpg': - continue + media_files: List[tuple] = [] + # the context manager closes the scandir iterator even when the limit + # below breaks out of the loop early + with os.scandir(scan_path) as entries: + for entry in entries: + # stop walking early once enough files have been collected; with + # limit=1 this turns the listing into a cheap existence check + if limit is not None and len(media_files) >= limit: + break - # check if it's a file first (most common case) - if entry.is_file(follow_symlinks=False): - # filter by extension before calling stat - if not any(entry.path.lower().endswith(e) for e in exts): + # ignore hidden files/dirs and other unwanted files + if entry.name.startswith('.') or entry.name == 'lastsnap.jpg': continue - # If stat is not needed, use None as placeholder - st = None - if with_stat: - # stat call may fail due to race conditions or permission issues - try: - st = entry.stat(follow_symlinks=False) - except Exception as e: - logging.error(f'stat failed: {e}') + # check if it's a file first (most common case) + if entry.is_file(follow_symlinks=False): + # filter by extension before calling stat + if not any(entry.path.lower().endswith(e) for e in exts): continue - media_files.append((entry.path, st)) + # If stat is not needed, use None as placeholder + st = None + if with_stat: + # stat call may fail due to race conditions or permission issues + try: + st = entry.stat(follow_symlinks=False) + except Exception as e: + logging.error(f'stat failed: {e}') + continue + + media_files.append((entry.path, st)) - # recurse into subdirectories only when no sub_path filter is set - elif sub_path is None and entry.is_dir(follow_symlinks=False): - media_files.extend(_list_media_files(entry.path, exts, with_stat=with_stat)) + # recurse into subdirectories only when no sub_path filter is set + elif sub_path is None and entry.is_dir(follow_symlinks=False): + remaining = limit - len(media_files) if limit is not None else None + media_files.extend( + _list_media_files( + entry.path, exts, with_stat=with_stat, limit=remaining + ) + ) return media_files @@ -206,10 +220,10 @@ def _remove_older_files( uploadservices.clean_cloud(directory, {}, clean_cloud_info) -def _do_list_media(pipe, target_dir, exts, sub_path, with_stat): +def _do_list_media(pipe, target_dir, exts, sub_path, with_stat, limit=None): from mimetypes import guess_type - mf = _list_media_files(target_dir, exts, sub_path, with_stat) + mf = _list_media_files(target_dir, exts, sub_path, with_stat, limit) for p, st in mf: path = p[len(target_dir) :] if not path.startswith('/'): @@ -534,6 +548,7 @@ def list_media( media_type: str, prefix: Optional[str] = None, with_stat: bool = True, + limit: Optional[int] = None, ) -> Awaitable: target_dir = camera_config.get('target_dir') utils.validate_paths(prefix, target_dir=target_dir) @@ -548,7 +563,8 @@ def list_media( parent_pipe, child_pipe = multiprocessing.Pipe(duplex=False) process = multiprocessing.Process( - target=_do_list_media, args=(child_pipe, target_dir, exts, prefix, with_stat) + target=_do_list_media, + args=(child_pipe, target_dir, exts, prefix, with_stat, limit), ) process.start() child_pipe.close() diff --git a/motioneye/remote.py b/motioneye/remote.py index f307a8c10..6071f6b15 100644 --- a/motioneye/remote.py +++ b/motioneye/remote.py @@ -387,7 +387,11 @@ async def get_current_picture( async def list_media( - local_config, media_type, prefix: Optional[str] = None + local_config, + media_type, + prefix: Optional[str] = None, + with_stat: bool = True, + limit: Optional[int] = None, ) -> utils.ListMediaResponse: utils.validate_paths(prefix) @@ -403,6 +407,13 @@ async def list_media( if prefix is not None: query['prefix'] = prefix + # older remote motionEye versions simply ignore these parameters + if not with_stat: + query['with_stat'] = 'false' + + if limit is not None: + query['limit'] = str(limit) + # timeout here is 10 times larger than usual - we expect a big delay when fetching the media list p = path + f'/{media_type}/{camera_id}/list/' request = _make_request( diff --git a/motioneye/static/js/main.js b/motioneye/static/js/main.js index b6f1affda..7bcd5c820 100644 --- a/motioneye/static/js/main.js +++ b/motioneye/static/js/main.js @@ -4721,9 +4721,11 @@ function runMediaDialog(cameraId, mediaType) { function hideMediaButtonIfEmpty(cameraId, mediaType, button) { /* the list endpoint returns {mediaList: [...]} (empty when there are no - * files); with_stat=false keeps the request cheap. On error the button is - * left visible (safe default - don't hide when we are unsure). */ - ajax('GET', basePath + mediaType + '/' + cameraId + '/list/?with_stat=false', null, function (data) { + * files); limit=1 makes the server stop walking the media directory at the + * first file found, so this stays cheap even with many stored files. On + * error the button is left visible (safe default - don't hide when we are + * unsure). */ + ajax('GET', basePath + mediaType + '/' + cameraId + '/list/?with_stat=false&limit=1', null, function (data) { if (data && data.mediaList && !data.mediaList.length) { button.hide(); } diff --git a/tests/test_handlers/test_media_list_limit.py b/tests/test_handlers/test_media_list_limit.py new file mode 100644 index 000000000..d794c057b --- /dev/null +++ b/tests/test_handlers/test_media_list_limit.py @@ -0,0 +1,67 @@ +# Copyright (c) 2013 Calin Crisan +# This file is part of motionEye. +# +# motionEye is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . + +"""Tests verifying the optional 'limit' query parameter parsing in the media list endpoints.""" + +import json +from unittest.mock import AsyncMock, patch + +from motioneye.handlers.movie import MovieHandler +from motioneye.handlers.picture import PictureHandler +from tests.test_handlers import HandlerTestCase + + +class _MediaListLimitTests: + media_type: str + + def _fetch_list(self, qs=''): + cookie = self.make_session_cookie('admin') + with patch( + 'motioneye.mediafiles.list_media', new=AsyncMock(return_value=[]) + ) as list_media: + response = self.fetch( + f'/{self.media_type}/1/list/{qs}', headers={'Cookie': cookie} + ) + + self.assertEqual(200, response.code) + self.assertEqual([], json.loads(response.body)['mediaList']) + return list_media.call_args.kwargs + + def test_no_limit_defaults_to_none(self): + kwargs = self._fetch_list() + self.assertIsNone(kwargs['limit']) + self.assertTrue(kwargs['with_stat']) + + def test_positive_limit_is_passed_through(self): + kwargs = self._fetch_list('?with_stat=false&limit=1') + self.assertEqual(kwargs['limit'], 1) + self.assertFalse(kwargs['with_stat']) + + def test_invalid_limit_values_are_ignored(self): + for qs in ('?limit=abc', '?limit=0', '?limit=-5', '?limit='): + with self.subTest(qs=qs): + kwargs = self._fetch_list(qs) + self.assertIsNone(kwargs['limit']) + + +class PictureListLimitTest(_MediaListLimitTests, HandlerTestCase): + handler_cls = PictureHandler + media_type = 'picture' + + +class MovieListLimitTest(_MediaListLimitTests, HandlerTestCase): + handler_cls = MovieHandler + media_type = 'movie' diff --git a/tests/test_mediafiles.py b/tests/test_mediafiles.py index be9790d5b..3c0bf8316 100644 --- a/tests/test_mediafiles.py +++ b/tests/test_mediafiles.py @@ -20,6 +20,7 @@ from shutil import rmtree from tempfile import mkdtemp from time import time +from unittest.mock import patch from motioneye import mediafiles from motioneye.mediafiles import _list_media_files @@ -241,6 +242,121 @@ def test_list_media_files_deep_recursion(self): for path, st in result_no_stat: self.assertIsNone(st) + def test_list_media_files_limit_one_stops_early(self): + """Test that limit=1 returns a single entry (cheap existence check).""" + movie_exts = ['.mp4', '.avi', '.mkv'] + result = _list_media_files(self.test_dir, movie_exts, with_stat=False, limit=1) + + self.assertEqual(len(result), 1) + # the returned entry is one of the actual movie files + self.assertIn(result[0][0], self.movie_files) + + def test_list_media_files_limit_spans_recursion(self): + """Test that limit is honored across subdirectory recursion.""" + movie_exts = ['.mp4', '.avi', '.mkv'] + # only 2 movies live in the root dir, so a limit of 5 must also count + # files collected while recursing into subdirectories + result = _list_media_files(self.test_dir, movie_exts, limit=5) + + self.assertEqual(len(result), 5) + for path, st in result: + self.assertIn(path, self.movie_files) + + def test_list_media_files_limit_above_total_returns_all(self): + """Test that a limit larger than the number of files returns everything.""" + movie_exts = ['.mp4', '.avi', '.mkv'] + result = _list_media_files(self.test_dir, movie_exts, limit=100) + + result_paths = sorted([path for path, st in result]) + self.assertEqual(result_paths, sorted(self.movie_files)) + + def test_list_media_files_limit_with_sub_path(self): + """Test that limit also applies when listing a sub_path.""" + movie_exts = ['.mp4', '.avi', '.mkv'] + # 2024-01-01 contains two movie files + result = _list_media_files( + self.test_dir, movie_exts, sub_path='2024-01-01', limit=1 + ) + + self.assertEqual(len(result), 1) + self.assertIn('2024-01-01', result[0][0]) + + def test_list_media_files_limit_stops_scanning_early(self): + """Test that limit=1 stops consuming directory entries at the first + match instead of walking everything and truncating afterwards.""" + flat_dir = mkdtemp() + try: + num_files = 1000 + for i in range(num_files): + Path(os.path.join(flat_dir, f'movie{i}.mp4')).touch() + + consumed = [] + real_scandir = os.scandir + + class CountingScandir: + def __init__(self, path): + self._it = real_scandir(path) + + def __enter__(self): + return self + + def __exit__(self, *exc_info): + return self._it.__exit__(*exc_info) + + def __iter__(self): + for entry in self._it: + consumed.append(entry.name) + yield entry + + with patch('motioneye.mediafiles.os.scandir', CountingScandir): + result = _list_media_files(flat_dir, ['.mp4'], with_stat=False, limit=1) + + self.assertEqual(len(result), 1) + # every entry matches, so the walk must stop right after the + # first one instead of scanning all 1000 files + self.assertLessEqual(len(consumed), 2) + finally: + rmtree(flat_dir) + + def test_list_media_files_limit_counts_matches_not_scanned_entries(self): + """Test that limit counts collected media files rather than examined + directory entries, whatever the scan order.""" + mixed_dir = mkdtemp() + try: + # many non-media files and exactly one movie: with limit=1 the + # movie must be found no matter in which order entries are scanned + for i in range(50): + Path(os.path.join(mixed_dir, f'note{i}.txt')).touch() + movie = os.path.join(mixed_dir, 'only_movie.mp4') + Path(movie).touch() + + result = _list_media_files(mixed_dir, ['.mp4'], with_stat=False, limit=1) + self.assertEqual([path for path, st in result], [movie]) + + # and nothing is returned when no entry matches at all + result = _list_media_files(mixed_dir, ['.avi'], with_stat=False, limit=1) + self.assertEqual(result, []) + finally: + rmtree(mixed_dir) + + def test_list_media_files_limit_remaining_budget_in_recursion(self): + """Test that recursion only receives the remaining budget: with more + files per subdirectory than budget left, the total must still not + exceed the limit, whatever the scan order.""" + tree_dir = mkdtemp() + try: + # two subdirectories with 3 movies each; whichever is scanned + # first leaves only one slot of the limit=4 budget for the other + for sub in ('cam1', 'cam2'): + os.makedirs(os.path.join(tree_dir, sub)) + for i in range(3): + Path(os.path.join(tree_dir, sub, f'movie{i}.mp4')).touch() + + result = _list_media_files(tree_dir, ['.mp4'], with_stat=False, limit=4) + self.assertEqual(len(result), 4) + finally: + rmtree(tree_dir) + def test_list_media_files_no_recursion_with_sub_path_filter(self): """Test that _list_media_files does not recurse when sub_path is provided.""" # List files in level1_dir with sub_path filter (should not recurse into level2_dir) @@ -282,6 +398,51 @@ def test_list_media_files_no_recursion_with_sub_path_filter(self): self.assertEqual(result_paths, expected_files) +class TestDoListMedia(unittest.TestCase): + """Tests for the _do_list_media subprocess entry point, using a fake pipe + so no multiprocessing is involved.""" + + class _FakePipe: + def __init__(self): + self.sent = [] + self.closed = False + + def send(self, obj): + self.sent.append(obj) + + def close(self): + self.closed = True + + def setUp(self): + self.target_dir = mkdtemp() + for i in range(3): + Path(os.path.join(self.target_dir, f'movie{i}.mp4')).touch() + + def tearDown(self): + rmtree(self.target_dir) + + def test_do_list_media_respects_limit(self): + pipe = self._FakePipe() + mediafiles._do_list_media(pipe, self.target_dir, ['.mp4'], None, False, 2) + + self.assertEqual(len(pipe.sent), 2) + self.assertTrue(pipe.closed) + for entry in pipe.sent: + # without stat, only the path is sent + self.assertEqual(list(entry.keys()), ['path']) + self.assertTrue(entry['path'].startswith('/')) + + def test_do_list_media_defaults_list_everything_with_stat(self): + pipe = self._FakePipe() + mediafiles._do_list_media(pipe, self.target_dir, ['.mp4'], None, True) + + self.assertEqual(len(pipe.sent), 3) + self.assertTrue(pipe.closed) + for entry in pipe.sent: + self.assertIn('mimeType', entry) + self.assertIn('timestamp', entry) + + class TestMediaFilesPathValidation(unittest.TestCase): """Tests verifying that path validation (traversal, absolute, dir escape) is enforced in mediafiles functions.""" diff --git a/tests/test_remote.py b/tests/test_remote.py index af68f9a66..8405faee1 100644 --- a/tests/test_remote.py +++ b/tests/test_remote.py @@ -17,6 +17,7 @@ """Tests verifying that path traversal elements are rejected in remote module functions.""" import unittest +from unittest.mock import AsyncMock, MagicMock, patch from motioneye import remote @@ -145,5 +146,44 @@ async def test_del_media_group_rejects_traversal(self): self._assert_raises_path_traversal(ctx.exception) +class TestRemoteListMediaQuery(unittest.IsolatedAsyncioTestCase): + """Tests that list_media only forwards with_stat/limit query parameters + when they are explicitly set, so older remote motionEye versions keep + receiving the requests they already understand.""" + + _LOCAL_CONFIG = { + '@proto': 'mjpeg', + '@host': '127.0.0.1', + '@port': 8765, + '@username': '', + '@password': '', + '@path': '', + '@remote_camera_id': 1, + } + + async def _get_query(self, **kwargs): + response = MagicMock() + response.error = None + response.body = '{"mediaList": []}' + + with patch.object(remote, '_make_request') as make_request, patch.object( + remote, '_send_request', new=AsyncMock(return_value=response) + ): + resp = await remote.list_media(self._LOCAL_CONFIG, 'picture', **kwargs) + + self.assertIsNone(resp.error) + return make_request.call_args.kwargs['query'] + + async def test_default_request_sends_no_extra_params(self): + query = await self._get_query() + self.assertNotIn('with_stat', query) + self.assertNotIn('limit', query) + + async def test_with_stat_false_and_limit_are_forwarded(self): + query = await self._get_query(with_stat=False, limit=1) + self.assertEqual(query.get('with_stat'), 'false') + self.assertEqual(query.get('limit'), '1') + + if __name__ == '__main__': unittest.main()