[cli] Add backwards compatibility for -roi/region-of-intertest option.

docs/changelog.md

@@ -5,36 +5,25 @@
 
 ## DVR-Scan 1.6
 
-### 1.6 (In Development)
+### 1.6-beta (In Development)
 
 #### Release Notes
 
-In development.
+DVR-Scan 1.6-beta showcases the new region editor and has improved seeking performance. There are also several other improvements and bugfixes since the last release.
 
 #### Changelog
 
-**WIP**:
-
- - TODO: Change -roi to have old behavior (x y w h), new one to use two points (x0 y0 x1 y1)
- - TODO: Add a new config file option to replace `region-of-interest`, call it `detection-regions`
-
- - [feature] ROI selection window has been improved, easier to create complex masks
- - [feature] ROI specification has been extended to allow more complex shapes
-    - Specifying two points (X0 Y0 X1 Y1) will result in a rectangular ROI
-    - Specifying three or more points will result in a polygonal ROI, e.g. for a triangle: `--roi 0 0 50 0 25 25`
- - [feature] Multiple regions of interest (rectangular or polygonal) can now be provided:
-    - Using the ROI selection window: `dvr-scan -i video.mp4 --roi`
-    - From command-line: `dvr-scan -i video.mp4 --roi 5 5 20 20 --roi 100 100 20 20`
-    - From config file: `region-of-interest = [5 5 20 20], [100 100 20 20]`
-
-**General**:
-
- - [feature] New `-fm` / `--frame-metrics` option draws motion score on each frame to help tune [detection parameters](https://dvr-scan.readthedocs.io/en/latest/guide/options/#
+ - [feature] [New region editor](https://dvr-scan.readthedocs.io/en/develop/guide/#region-editor) `-r`/`--region-editor` allows creation of multiple regions without shape restrictions, replaces `-roi`/`--region-of-interest`
+ - [feature] Multiple regions of interest (rectangular or polygonal) can now be created:
+    - Using the new region editor by adding the `-r`/`--region-editor` flag: `dvr-scan -i video.mp4 -r`
+    - New `-a`/`--add-region` replaces `-roi`/`--region-of-interest` option: `dvr-scan -i video.mp4 -a 5 5 20 5 20 20 5 20`
+    - Regions can now be saved to a file: press S in the region editor or use `-s`/`--save-region`
+    - Regions can now be loaded from a file: press O in the region editor or use `-R`/`--load-region`
+    - Config files can specify a region file to use by default with the `load-region` option, replaces the `region-of-interest` setting
+ - [feature] New `-fm` / `--frame-metrics` option draws motion score on each frame to help tune detection parameters
  - [cli] Short flag `-v` is now used for `--verbosity`, replaced by `-V` for `--version`
- - [cli] Short flag `-roi` is now deprecated, use `--roi` instead
- - [cli] Add `max-window-width` and `max-window-height` settings to config file, controls maximum size of ROI selection window
-    - By default, videos larger than the smallest display are downscaled
-    - Replaces deprecated `-roi WIDTH HEIGHT` overload
+ - [cli] `-roi`/`--region-of-interest` is now deprecated, replaced by region editor and add/save/load region flags
+    - Specifying this option will display the ROI in the new region format allowing you to update usages more easily
  - [general] Improved seeking performance, using `-st`/`--start-time` is now much faster ([#92](https://github.com/Breakthrough/DVR-Scan/issues/92))
  -detection-parameters)
  - [general] Noise reduction kernel can now be disabled by setting `-k`/`--kernel-size` to `0` ([#123](https://github.com/Breakthrough/DVR-Scan/issues/123))

dvr_scan/cli/__init__.py

@@ -272,48 +272,6 @@ def __call__(self, parser, namespace, values, option_string=None):
         parser.exit(message=version)
 
 
-class RegionActionDeprecated(argparse.Action):
-
-    def __init__(self,
-                 option_strings,
-                 dest,
-                 nargs=None,
-                 const=None,
-                 default=None,
-                 type=None,
-                 choices=None,
-                 required=False,
-                 help=None,
-                 metavar=None):
-        assert nargs == '*'
-        assert const is None
-        super(RegionActionDeprecated, self).__init__(
-            option_strings=option_strings,
-            dest=dest,
-            nargs=nargs,
-            const=const,
-            default=default,
-            type=type,
-            choices=choices,
-            required=required,
-            help=help,
-            metavar=metavar)
-
-    def __call__(self, parser, namespace, values: List[str], option_string=None):
-        # Used to show warning to user if they use -roi/--region-of-interest instead of -r/-a.
-        setattr(namespace, 'used_deprecated_roi_option', True)
-        # -r/--roi specified without coordinates
-        if not values:
-            setattr(namespace, 'region_editor', True)
-            return
-        # TODO(v1.6): Re-add backwards compatibility for X Y W H rectangles.
-        raise NotImplementedError()
-        # Append this ROI to any existing ones, if any.
-        items = getattr(namespace, 'roi_deprecated', [])
-        items += regions
-        setattr(namespace, 'roi_deprecated', items)
-
-
 class RegionAction(argparse.Action):
 
     DEFAULT_ERROR_MESSAGE = "Region must be 3 or more points of the form X0 Y0 X1 Y1 X2 Y2 ..."
@@ -586,16 +544,13 @@ def get_cli_parser(user_config: ConfigRegistry):
         help=('Timecode to stop processing the input (see -st for valid timecode formats).'),
     )
 
-    # TODO(v1.6): Restore this argument to the exact way it was. Error if specified with any of
-    # the replacements above.
-    # TODO(v1.7): Remove -roi (split up into other options).
+    # TODO(v2.0): Remove -roi (replaced by -r/--region-editor and -a/--add-region).
     parser.add_argument(
         '-roi',
-        '-region-of-interest',
-        metavar='x0 y0 w h',
+        '--region-of-interest',
         dest='region_of_interest',
+        metavar='x0 y0 w h',
         nargs='*',
-        action=RegionActionDeprecated,
         help=argparse.SUPPRESS,
     )
 

dvr_scan/cli/controller.py

@@ -25,7 +25,7 @@
 
 import dvr_scan
 from dvr_scan.cli import get_cli_parser
-from dvr_scan.cli.config import ConfigRegistry, ConfigLoadFailure
+from dvr_scan.cli.config import ConfigRegistry, ConfigLoadFailure, RegionValueDeprecated
 from dvr_scan.overlays import TextOverlay, BoundingBoxOverlay
 from dvr_scan.detector import MotionDetector, Rectangle
 from dvr_scan.scanner import DetectorType, OutputMode, MotionScanner
@@ -83,6 +83,19 @@ def _preprocess_args(args):
     # -roi has been replaced with --roi/--region of interest
     if hasattr(args, 'used_deprecated_roi_option'):
         logger.warning('WARNING: Short form -roi is deprecated, use --roi instead.')
+    # -roi/--region-of-interest
+    if hasattr(args, 'roi_deprecated') and args.roi_deprecated:
+        roi_deprecated = args.roi_deprecated
+        try:
+            roi_deprecated = RegionValueDeprecated(
+                value=' '.join(args.roi_deprecated), allow_size=True).value
+        except ValueError:
+            logger.error(
+                'Error: Invalid value for ROI: %s. ROI must be specified as a rectangle of'
+                ' the form `x,y,w,h` or the max window size `w,h` (commas/spaces are ignored).'
+                ' For example: -roi 200,250 50,100', ' '.join(args.roi_deprecated))
+            return False, None
+        args.roi_deprecated = roi_deprecated
     return True, args
 
 
@@ -286,6 +299,7 @@ def run_dvr_scan(settings: ProgramSettings) -> ty.List[ty.Tuple[FrameTimecode, F
         regions=settings.get_arg('regions'),
         load_region=load_region,
         save_region=settings.get_arg('save-region'),
+        roi_deprecated=settings.get('region-of-interest'),
     )
 
     # Scan video for motion with specified parameters.

dvr_scan/scanner.py

@@ -255,6 +255,9 @@ def __init__(self,
         self._regions: List[List[Point]] = []   # -a/--add-region, -w/--region-window
         self._load_region: Optional[str] = None # -R/--load-region
         self._save_region: Optional[str] = None # -s/--save-region
+        self._max_roi_size_deprecated = None
+        self._show_roi_window_deprecated = False
+        self._roi_deprecated = None
 
         # Input Video Parameters (set_video_time)
         self._input: VideoJoiner = VideoJoiner(input_videos) # -i/--input
@@ -382,13 +385,34 @@ def set_regions(self,
                     region_editor: bool = False,
                     regions: Optional[List[List[Point]]] = None,
                     load_region: Optional[str] = None,
-                    save_region: Optional[str] = None):
+                    save_region: Optional[str] = None,
+                    roi_deprecated: Optional[List[int]] = None):
         """Set options for limiting detection regions."""
         self._region_editor = region_editor
         self._regions = regions if regions else []
         self._load_region = load_region
         self._save_region = save_region
 
+        # Handle deprecated ROI option.
+        # TODO(v2.0): Remove.
+        if roi_deprecated is not None:
+            if roi_deprecated:
+                if not all(isinstance(i, int) for i in roi_deprecated):
+                    raise TypeError('Error: Non-integral type found in specified roi.')
+                if any(x < 0 for x in roi_deprecated):
+                    raise ValueError('Error: Negative value found in roi.')
+                if len(roi_deprecated) == 2:
+                    self._max_roi_size_deprecated = roi_deprecated
+                    self._show_roi_window_deprecated = True
+                elif len(roi_deprecated) == 4:
+                    self._roi_deprecated = roi_deprecated
+                    self._show_roi_window_deprecated = False
+                else:
+                    raise ValueError('Error: Expected either 2 or 4 elements in roi.')
+            # -roi with no arguments.
+            else:
+                self._show_roi_window_deprecated = True
+
     def set_event_params(self,
                          min_event_len: Union[int, float, str] = '0.1s',
                          time_pre_event: Union[int, float, str] = '1.5s',
@@ -461,6 +485,12 @@ def __exit__(self, type, value, traceback):
         return (NullProgressBar(), NullContextManager())
 
     def _handle_regions(self) -> bool:
+        # TODO(v2.0): Remove deprecated ROI selection handlers.
+        if (self._show_roi_window_deprecated) and (self._load_region or self._regions
+                                                   or self._region_editor):
+            raise ValueError("Use -r/--region-editor instead of -roi.")
+        self._select_roi_deprecated()
+
         if self._load_region:
             try:
                 regions = load_regions(self._load_region)
@@ -508,19 +538,22 @@ def _handle_regions(self) -> bool:
             logger.info(f"Limiting detection to {len(self._regions)} "
                         f"region{'s' if len(self._regions) > 1 else ''}.")
         else:
-            logger.info("No regions selected, using entire frame.")
+            logger.debug("No regions selected.")
         if self._save_region:
             regions = self._regions if self._regions else [[
                 Point(0, 0),
-                Point(frame_w - 1, 0),
-                Point(frame_w - 1, frame_h - 1),
-                Point(0, frame_h - 1)
+                Point(self._input.resolution[0] - 1, 0),
+                Point(self._input.resolution[0] - 1, self._input.resolution[1] - 1),
+                Point(0, self._input.resolution[1] - 1)
             ]]
-            with open(self._save_region, 'wt') as region_file:
+            path = self._save_region
+            if self._output_dir:
+                path = os.path.join(self._output_dir, path)
+            with open(path, 'wt') as region_file:
                 for shape in self._regions:
                     region_file.write(" ".join(f"{x} {y}" for x, y in shape))
                     region_file.write("\n")
-            logger.info(f"Saved region data to: {self._save_region}")
+            logger.info(f"Saved region data to: {path}")
         return True
 
     def stop(self):
@@ -936,3 +969,60 @@ def _encode_thread(self, encode_queue: queue.Queue):
             while not encode_queue.empty():
                 _ = encode_queue.get_nowait()
         # pylint: enable=bare-except
+
+    # TODO(v2.0): Remove deprecated function, replaced by Region Editor.
+    def _select_roi_deprecated(self) -> bool:
+        # area selection
+        if self._show_roi_window_deprecated:
+            self._logger.warning(
+                "WARNING: -roi/--region-of-interest is deprecated and will be removed. Use "
+                "-r/--region-editor instead.")
+            self._logger.info("Selecting area of interest:")
+            # TODO: We should process this frame.
+            frame_for_crop = self._input.read()
+            scale_factor = None
+            if self._max_roi_size is None:
+                self._max_roi_size = get_min_screen_bounds()
+            if self._max_roi_size is not None:
+                frame_h, frame_w = (frame_for_crop.shape[0], frame_for_crop.shape[1])
+                max_w, max_h = self._max_roi_size
+                # Downscale the image if it's too large for the screen.
+                if frame_h > max_h or frame_w > max_w:
+                    factor_h = frame_h / float(max_h)
+                    factor_w = frame_w / float(max_w)
+                    scale_factor = max(factor_h, factor_w)
+                    new_height = round(frame_h / scale_factor)
+                    new_width = round(frame_w / scale_factor)
+                    frame_for_crop = cv2.resize(
+                        frame_for_crop, (new_width, new_height), interpolation=cv2.INTER_CUBIC)
+            roi = cv2.selectROI("DVR-Scan ROI Selection", frame_for_crop)
+            cv2.destroyAllWindows()
+            if any([coord == 0 for coord in roi[2:]]):
+                self._logger.info("ROI selection cancelled. Aborting...")
+                return False
+            # Unscale coordinates if we downscaled the image.
+            if scale_factor:
+                roi = [round(x * scale_factor) for x in roi]
+            self._roi_deprecated = roi
+        if self._roi_deprecated:
+            self._logger.info(
+                'ROI set: (x,y)/(w,h) = (%d,%d)/(%d,%d)',
+                self._roi_deprecated[0],
+                self._roi_deprecated[1],
+                self._roi_deprecated[2],
+                self._roi_deprecated[3],
+            )
+
+            x, y, w, h = self._roi_deprecated
+            region = [Point(x, y), Point(x + w, y), Point(x + w, y + h), Point(x, y + h)]
+            region_arg = " ".join(f"{x} {y}" for x, y in region)
+            self._logger.warning(
+                "WARNING: region-of-interest (roi) is deprecated and will be removed. "
+                "Use the following equivalent option by command line:\n\n"
+                f"--add-region {region_arg}\n\n"
+                "For config files, save this ROI as a region file and specify the path as the "
+                "load-region option (e.g. load-region = /usr/share/regions.txt). You can also save "
+                "the ROI to a file and load it again by adding `-s region.txt` to this command, or "
+                "by launching the region editor (add -r to this command and hit S to save).")
+            self._regions += [region]
+        return True

tests/test_cli.py

@@ -254,3 +254,26 @@ def test_copy_mode(tmp_path):
         'copy',
     ]) == 0
     assert len(os.listdir(tmp_path)) == BASE_COMMAND_NUM_EVENTS, "Incorrect number of events found."
+
+
+def test_deprecated_roi(tmp_path):
+    """Test deprecated ROI translation."""
+    tmp_path = str(tmp_path)                     # Hack for Python 3.7 builder.
+    output = subprocess.check_output(
+        args=DVR_SCAN_COMMAND + BASE_COMMAND + [
+            '--output-dir',
+            tmp_path,
+            '--scan-only',
+            '-dt',
+            '2',
+            '-roi',
+            '10 20 10 15',
+            '-s',
+            'roi.txt',
+        ],
+        text=True)
+    roi_path = os.path.join(tmp_path, "roi.txt")
+    assert os.path.exists(roi_path)
+    with open(roi_path, "rt") as roi_file:
+        last_line_of_file = list(filter(None, roi_file.readlines()))[-1].strip()
+    assert last_line_of_file == "10 20 20 20 20 35 10 35"