Video bug fix

CHANGELOG.md

@@ -21,6 +21,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Bug fixes
 - Preserve end_frame information of a video when it is zero.
   (<https://github.com/openvinotoolkit/datumaro/pull/1541>)
+- Changed the Datumaro format to ensure exported videos have relative paths and to prevent the same video from being overwritten.
+  (<https://github.com/openvinotoolkit/datumaro/pull/1547>)
 
 ## Q2 2024 Release 1.7.0
 ### New features

src/datumaro/components/exporter.py

@@ -418,12 +418,14 @@ def save_video(
         path = osp.join(basedir, fname)
         path = osp.abspath(path)
 
-        os.makedirs(osp.dirname(path), exist_ok=True)
-
-        if isinstance(item.media, VideoFrame):
-            item.media.video.save(path, crypter=NULL_CRYPTER)
-        else:  # Video
-            item.media.save(path, crypter=NULL_CRYPTER)
+        # To prevent the video from being overwritten
+        # (A video can have same path but different start/end frames)
+        if not osp.exists(path):
+            os.makedirs(osp.dirname(path), exist_ok=True)
+            if isinstance(item.media, VideoFrame):
+                item.media.video.save(path, crypter=NULL_CRYPTER)
+            else:  # Video
+                item.media.save(path, crypter=NULL_CRYPTER)
 
     @property
     def images_dir(self) -> str:

src/datumaro/plugins/data_formats/datumaro/exporter.py

@@ -184,12 +184,9 @@ def context_save_media(
 
             if context.save_media:
                 fname = context.make_video_filename(item)
-                # To prevent the video from being overwritten
-                # (A video can have same path but different start/end frames)
-                if not osp.exists(fname):
-                    context.save_video(item, fname=fname, subdir=item.subset)
+                context.save_video(item, fname=fname, subdir=item.subset)
                 item.media = Video(
-                    path=video.path,
+                    path=fname,
                     step=video._step,
                     start_frame=video._start_frame,
                     end_frame=video._end_frame,
@@ -202,8 +199,7 @@ def context_save_media(
 
             if context.save_media:
                 fname = context.make_video_filename(item)
-                if not osp.exists(fname):
-                    context.save_video(item, fname=fname, subdir=item.subset)
+                context.save_video(item, fname=fname, subdir=item.subset)
                 item.media = VideoFrame(Video(fname), video_frame.index)
 
             yield

tests/unit/data_formats/datumaro/test_datumaro_format.py

@@ -4,7 +4,10 @@
 
 import os
 import os.path as osp
+import shutil
+import sys
 from functools import partial
+from unittest.mock import patch
 
 import numpy as np
 import pytest
@@ -152,6 +155,18 @@ def test_can_save_and_load(
             stream=stream,
         )
 
+    @mark_requirement(Requirements.DATUM_GENERAL_REQ)
+    def test_export_video_only_once(
+        self,
+        fxt_test_datumaro_format_video_dataset: Dataset,
+        test_dir,
+    ):
+        with patch(
+            "datumaro.components.media.shutil.copyfile", wraps=shutil.copyfile
+        ) as mocked_save:
+            fxt_test_datumaro_format_video_dataset.export(test_dir, "datumaro", save_media=True)
+            assert mocked_save.call_count == 2  # train/video.avi, test/video.avi
+
     @mark_requirement(Requirements.DATUM_GENERAL_REQ)
     @pytest.mark.parametrize(
         "fxt_dataset_pair, compare, require_media, dimension",

tests/utils/test_utils.py

@@ -327,8 +327,13 @@ def _change_path_in_items(dataset, source_path, target_path):
         for item in dataset:
             if item.media:
                 if hasattr(item.media, "path") and item.media.path:
-                    path = item.media.path.replace(source_path, target_path)
-                    item.media = item.media.from_self(path=path)
+                    if isinstance(item.media, VideoFrame):
+                        path = (
+                            item.media.video._path
+                        )  # _path includes the OS-specific directory separator
+                    else:
+                        path = item.media._path
+                    item.media = item.media.from_self(path=path.replace(source_path, target_path))
                 if isinstance(item.media, PointCloud):
                     new_images = []
                     for image in item.media.extra_images:
@@ -346,7 +351,7 @@ def _change_path_in_items(dataset, source_path, target_path):
         if move_save_dir:
             save_dir = tmp_dir
             for file in os.listdir(test_dir):
-                os.symlink(osp.join(test_dir, file), osp.join(save_dir, file))
+                shutil.move(osp.join(test_dir, file), save_dir)
         else:
             save_dir = test_dir