Skip to content

Commit

Permalink
Bugfix when end_frame is zero (#1541)
Browse files Browse the repository at this point in the history
<!-- Contributing guide:
https://github.com/openvinotoolkit/datumaro/blob/develop/CONTRIBUTING.md
-->

### Summary
Ticket Number: 144505
When video end_frame is zero, it was saved as None, not 0.
Due to this, end_frame information was lost when exporting it.


### How to test
1. Make a video media with start_frame=0 and end_frame=0
    ```
item = dm.DatasetItem(..., media=Video('video.mp4', start_frame=0,
end_frame=0), ...)
    ```
2. export the dataset as datumaro format
3. check it the end_frame is described in the output json file
ex. 
    ```
    {
        "id": "video",
        ...,
        "video": {
            "path": "video.mp4",
            "step": 1,
            "start_frame": 0,
            "end_frame": 0
        }
    },
    ```
### Checklist
<!-- Put an 'x' in all the boxes that apply -->
- [x] I have added unit tests to cover my changes.​
- [ ] I have added integration tests to cover my changes.​
- [x] I have added the description of my changes into
[CHANGELOG](https://github.com/openvinotoolkit/datumaro/blob/develop/CHANGELOG.md).​
- [ ] I have updated the
[documentation](https://github.com/openvinotoolkit/datumaro/tree/develop/docs)
accordingly

### License

- [x] I submit _my code changes_ under the same [MIT
License](https://github.com/openvinotoolkit/datumaro/blob/develop/LICENSE)
that covers the project.
  Feel free to contact the maintainers if that's a concern.
- [ ] I have updated the license header for each file (see an example
below).

```python
# Copyright (C) 2024 Intel Corporation
#
# SPDX-License-Identifier: MIT
```
  • Loading branch information
jihyeonyi committed Jun 19, 2024
1 parent f62fe24 commit 58e1582
Show file tree
Hide file tree
Showing 5 changed files with 20 additions and 8 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Pass Keyword Argument to TabularDataBase
(<https://github.com/openvinotoolkit/datumaro/pull/1522>)

### Bug fixes
- Preserve end_frame information of a video when it is zero.
(<https://github.com/openvinotoolkit/datumaro/pull/1541>)

## Q2 2024 Release 1.7.0
### New features
- Support 'Video' media type in datumaro format
Expand Down
4 changes: 2 additions & 2 deletions src/datumaro/components/media.py
Original file line number Diff line number Diff line change
Expand Up @@ -612,14 +612,14 @@ def __init__(
self._path = path

assert 0 <= start_frame
if end_frame:
if end_frame is not None:
assert start_frame <= end_frame
# we can't know the video length here,
# so we cannot validate if the end_frame is valid.
assert 0 < step
self._step = step
self._start_frame = start_frame
self._end_frame = end_frame or None
self._end_frame = end_frame

self._reader = None
self._iterator: Optional[_VideoFrameIterator] = None
Expand Down
5 changes: 3 additions & 2 deletions tests/integration/cli/test_video.py
Original file line number Diff line number Diff line change
Expand Up @@ -163,14 +163,14 @@ def _make_sample_video_dataset(test_dir: str, media_type=None) -> Dataset:
),
DatasetItem(
"1",
media=Video(video_path, step=1, start_frame=0, end_frame=1),
media=Video(video_path, step=1, start_frame=0, end_frame=0),
annotations=[
Label(0),
],
),
DatasetItem(
"2",
media=Video(video_path, step=1, start_frame=2, end_frame=2),
media=Video(video_path, step=1, start_frame=1, end_frame=2),
annotations=[
Label(1),
],
Expand Down Expand Up @@ -206,6 +206,7 @@ def test_can_export_video_dataset(self):

dataset_dir = osp.join(test_dir, "test_video")
expected.save(dataset_dir, save_media=True)

run(self, "project", "import", "-p", project_dir, "-f", "datumaro", dataset_dir)

result_dir = osp.join(test_dir, "test_video_result")
Expand Down
7 changes: 7 additions & 0 deletions tests/unit/test_video.py
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,13 @@ def test_can_compare(self, test_dir):
assert video1 != Video(video_path3)
assert Video(video_path3, end_frame=3) == video1

@mark_requirement(Requirements.DATUM_GENERAL_REQ)
@scoped
def test_can_preserve_zero_end_frame(self):
video = Video("video.avi", start_frame=0, end_frame=0)

assert video._end_frame != None


class VideoExtractorTest:
@mark_requirement(Requirements.DATUM_GENERAL_REQ)
Expand Down
8 changes: 4 additions & 4 deletions tests/utils/test_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -199,16 +199,16 @@ def compare_datasets(
test.assertEqual(item_a.attributes, item_b.attributes, item_a.id)

if require_media and item_a.media and item_b.media:
if isinstance(item_a.media, Image):
if isinstance(item_a.media, VideoFrame):
test.assertEqual(item_a.media, item_b.media, item_a.id)
test.assertEqual(item_a.media.index, item_b.media.index, item_a.id)
elif isinstance(item_a.media, Image):
test.assertEqual(item_a.media, item_b.media, item_a.id)
elif isinstance(item_a.media, PointCloud):
test.assertEqual(item_a.media.data, item_b.media.data, item_a.id)
test.assertEqual(item_a.media.extra_images, item_b.media.extra_images, item_a.id)
elif isinstance(item_a.media, Video):
test.assertEqual(item_a.media, item_b.media, item_a.id)
elif isinstance(item_a.media, VideoFrame):
test.assertEqual(item_a.media, item_b.media, item_a.id)
test.assertEqual(item_a.index, item_b.index, item_a.id)
elif isinstance(item_a.media, MultiframeImage):
test.assertEqual(item_a.media.data, item_b.media.data, item_a.id)
test.assertEqual(len(item_a.annotations), len(item_b.annotations), item_a.id)
Expand Down

0 comments on commit 58e1582

Please sign in to comment.