@Felix92: Bezüglich des `ffmpeg`-Aufrufs: Das ``shell=True`` sollte man nicht verwenden, sondern den Befehl und die Argumente als Liste übergeben:
Code: Alles auswählen
command = [
'ffmpeg',
'-y',
'-i', video_input_path,
'-r', '30',
'-s', '112x112',
'-c:v', 'libx264',
'-b:v', '3M',
'-strict', '-2',
'-movflags', 'faststart',
video_output_path,
]
subprocess.run(command)
`video_data` ist kein guter Name für einen Dateinamen/Pfad – da erwartet man die Videodaten selbst oder Daten über ein Video. Und das Attribut dann lokal an einen anderen Namen zu binden ist auch nicht schön. Zumal `video_file` auch etwas verwirrend ist, denn um ein Dateiobjekt handelt es sich ja auch nicht.
Der Zieldateiname in `large_video_cut()` – was eher `cut_large_video()` heissen sollte – wird über die Methode verteilt und zwei mal auf unterschiedliche Arten erstellt. Das sollte nur *einmal* passieren und entweder mit `Path` oder mit `os.path.join()`.
Weder ``if`` noch ``while`` sind Funktionen, also sollte man die auch nicht so schreiben als wären es welche. Die Klammern gehören da nicht hin.
Vergleiche mit literalen Wahrheitswerten macht man nicht. Da kommt nur wieder ein Wahrheitswert heraus, also kann man auch gleich den Wert nehmen mit dem man vergleicht oder mit dessen Negation mit ``not``. Bei `isOpened()` machst Du das ja auch schon einmal richtig. Warum diese Inkonsistenz?
Der Test ob `cap` geöffnet werden konnte, sollte eigentlich stattfinden bevor man mit allem weiteren weiter macht und vor allem auch verhindern das weiter gemacht wird, wenn das Öffnen fehlgeschlagen ist. Die `release()`-Aufrufe sollten durch ``try``:``finally`` sichergestellt werden. Und bei `VideoWriter` prüfst Du gar nicht, ob der funktioniert oder nicht.
Der ``return``-Wert sieht unsinnig aus: Ein `BoardVideo`-Objekt das einen geschlossenes `VideoWriter`-Objekt übergeben bekommt? Was soll denn damit noch angefangen werden?
Zwischenstand (ungetestet):
Code: Alles auswählen
def cut_large_video(self, fps):
"""Get the part of the speaker from the "main video" and save it in the
project folder and create an object of this.
"""
capture = cv2.VideoCapture(str(self.video_data))
try:
if not capture.isOpened():
raise RuntimeError('Error opening video stream or file')
new_filename = Path(
self.folder_path, self.folder_name, 'board_video.mp4'
)
fourcc = cv2.VideoWriter_fourcc(*'mp4v')
writer = cv2.VideoWriter(str(new_filename), fourcc, fps, (938, 530))
try:
if not writer.isOpened():
raise RuntimeError('Error opening video stream or file')
while True:
is_ok, frame = capture.read()
if not is_ok:
break
writer.write(frame[275:805, 17:955])
finally:
writer.release()
self.files.append(new_filename)
#
# TODO Passing a closed `VideoWriter` object looks wrong‽
#
return BoardVideo(writer)
finally:
capture.release()
cv2.destroyAllWindows()
`board_area()` ist kein guter Name für eine Funktion oder Methode. Solche Namen sollten die Tätigkeit beschreiben und `board_area` beschreibt keine Tätigkeit.
Du machst IMHO zu viel kopieren und einfügen. Hier ist jetzt ein `frame_number` was nicht benötigt wird, was nur da ist, weil diese Methode eine Kopie eines bereits vorhandenen Versuchs ist.
`sum` ist der Name einer eingebauten Funktion, den sollte man nicht an etwas anderes binden. Die Funktion kann man hier sogar verwenden!
Auch in dieser Methode wird nicht sinnvoll darauf reagiert wenn die Videodatei nicht geöffnet werden konnte.
Zwischenstand:
Code: Alles auswählen
#
# TODO Come up with a better method name.
#
def board_area(self, clip_prefix):
"""Analyse the video frame by frame and save the Clips (Board) in a
list.
"""
video = cv2.VideoCapture(str(self.file_path))
try:
if not video.isOpened():
raise RuntimeError('Error opening video stream or file')
times = list()
clip_numbers = count()
while True:
is_ok, frame = video.read()
if not is_ok:
break
average = cv2.mean(frame)
percentage_green = 100 * average[1] / sum(average)
if percentage_green > 40:
times.append(video.get(cv2.CAP_PROP_POS_MSEC) / 1000)
else:
if times:
clip = openshot.Clip(
'{}{}'.format(clip_prefix, next(clip_numbers))
)
clip.Start(times[0])
clip.End(times[-1])
self.subvideos.append(clip)
times.clear()
finally:
video.release()
cv2.destroyAllWindows()
Ich sehe nicht wo diese Methode Zeit vertrödelt. Wenn die zu langsam ist, dann ist das halt so langsam. Sollte es tatsächlich an Python liegen, müsstest Du es in C++ umschreiben. Sowohl OpenCV als auch OpenShot haben da ja eine entsprechende API.