Conversation
rizkiarm
left a comment
rizkiarm
left a comment
There was a problem hiding this comment.
Thank you for your contributions! It is a very good idea to add OpenCV support. But please do note that this addition would incur some cost to the user as it further complicates setup, given that it is widely known that OpenCV doesn't play well with some OS as well as very picky about dependencies.
That's why we believe that it would be better to keep skvideo as a default and give the choice to use OpenCV to the user. We would also like to keep the requirement as it is and put cv2 only as an optional requirement.
lipnet/lipreading/videos.py
Outdated
| def get_video_frames(self, path): | ||
| def get_video_frames_skvideo(self, path): | ||
| videogen = skvideo.io.vreader(path) | ||
| #print("aaaa") |
lipnet/lipreading/videos.py
Outdated
| return mouth_frames | ||
|
|
||
| def get_video_frames(self, path): | ||
| def get_video_frames_skvideo(self, path): |
There was a problem hiding this comment.
Don't change the current method of video loading as it has been proven to be reliable and doesn't require an additional installation of OpenCV, which is sometimes problematic (not to mention that the program itself sometimes doesn't work as expected).
Instead, provide an additional argument for the user in the from_video method. Something like this: from_video(self, path, use_open_cv=False). Please keep Sk-video to be the default method.
lipnet/lipreading/videos.py
Outdated
| from scipy import ndimage | ||
| from scipy.misc import imresize | ||
| import skvideo.io | ||
| import cv2 |
There was a problem hiding this comment.
Handle cv2 import failure (when users don't have OpenCV) as we want to give options to users to opt-out from using the feature
There was a problem hiding this comment.
I've decided to import cv2 later in the code
lipnet/lipreading/videos.py
Outdated
| return frames | ||
|
|
||
|
|
||
| def get_video_frames(self, path): |
There was a problem hiding this comment.
Rename the method to signify the use of opencv
2 participants
I've just rewrote function for frames reading from skvideo to OpenCV