Skip to content

[FIX] VideoManager read function failed on multiple videos - #107

Merged
Breakthrough merged 2 commits into
Breakthrough:masterfrom
ivan23kor:many_videos_video_manager_fix
Jun 13, 2019
Merged

[FIX] VideoManager read function failed on multiple videos#107
Breakthrough merged 2 commits into
Breakthrough:masterfrom
ivan23kor:many_videos_video_manager_fix

Conversation

@ivan23kor

Copy link
Copy Markdown
Contributor

Traceback:

"scenedetect/video_manager.py", line 757, in read
::self._downscale_factor, ::self._downscale_factor, :]
TypeError: 'NoneType' object is not subscriptable

Reason:

  1. Initializing a VideoManager object with multiple video paths
  2. Set the downscale factor, e.g. with set_downscale_factor()
  3. Invoke read() function of the created VideoManager object, e.g. via passing it to the SceneManager detect_scenes() as frame_source argument

This happens because of the bug in video_manager.py around lines 750. If one of the captured videos is over, an empty numpy.ndarray is returned on line 745 and its further use leads to the aforementioned error.
The commit contains the bug fix in video_manager.py as well as an integration test (VideoManager + SceneManager) in tests/test_video_manager.py

Comment thread scenedetect/video_manager.py Outdated
if not read_frame and not self._get_next_cap():
break
# Switch to the next capture when the current one is over
if not read_frame:

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should be changed to while not read_frame instead now, since if for some reason the new cap doesn't return a frame, it will still be used below. Changing this to a while() would advance to the next cap, if any.

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry I realized just now, a while loop is not the solution actually - have a better suggestion.

Comment thread tests/test_video_manager.py Outdated
@Breakthrough

Breakthrough commented Jun 11, 2019

Copy link
Copy Markdown
Owner

Hi @ivan23kor;

Great find!! I have one small favour to ask - as part of the review, I think the if statement should be changed to a while to handle the case where a video doesn't have any frames for some reason.

Do you agree/disagree? Just want to get your feedback on that before merging this.

Thank you so much for the fix, much appreciated!

Comment thread scenedetect/video_manager.py Outdated
@Breakthrough

Breakthrough commented Jun 11, 2019

Copy link
Copy Markdown
Owner

Hi @ivan23kor;

Sorry my mistake - no need for nested loops, I think it would be better to just change line 752 to continue instead. Thoughts?

Thanks!!

@ivan23kor

Copy link
Copy Markdown
Contributor Author

The second commit is better, check it out!

@Breakthrough

Copy link
Copy Markdown
Owner

Very nice @ivan23kor! Thank you so much!!

@Breakthrough
Breakthrough merged commit 266b33a into Breakthrough:master Jun 13, 2019
@Breakthrough Breakthrough added this to the v0.6 milestone Jun 13, 2019
@Breakthrough Breakthrough modified the milestones: v0.6, v0.5.1 Jul 13, 2019
@ivan23kor
ivan23kor deleted the many_videos_video_manager_fix branch December 28, 2019 16:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants