Remove Unneccessary Re-Encode If Video Is Already H264 (#234)

* Do not re-encode h264 videos

- if video_filepath ext is mp4 and ffmpeg
  probe finds video stream with h264 codec
  add codec:copy to StreamCopy the video
  instead of re-encoding as h264.

* black format

* ruff

* Inline args

* Consider output_format in should_copy_video

* Add logging

cdp_backend/tests/utils/test_file_utils.py

@@ -386,3 +386,41 @@ def test_clip_and_reformat_video(
  assert outfile.exists()
  assert outfile == (expected_outfile or outfile)
  os.remove(outfile)
+
+
+@pytest.mark.parametrize(
+ "video_filepath, output_format, codec_type, codec_name, expected",
+ [
+ (Path("video.mp4"), "mp4", "video", "h264", True),
+ (Path("video.MP4"), "mp4", "video", "h264", True),
+ (Path("video.mp4"), "mp4", "video", "mpeg4", False),
+ (Path("video.mkv"), "mp4", "video", "h264", False),
+ (Path("video.MKV"), "mp4", "video", "h264", False),
+ (Path("video.avi"), "mp4", "video", "mpeg4", False),
+ (Path("video.avi"), "mp4", "audio", "mp3", False),
+ (Path("video.mp4"), "mp3", "video", "h264", False),
+ ],
+)
+def test_should_copy_video(
+ video_filepath: Path,
+ output_format: str,
+ codec_type: str,
+ codec_name: str,
+ expected: bool,
+) -> None:
+ with mock.patch("ffmpeg.probe") as ffmpeg_probe:
+ ffmpeg_probe.return_value = {
+ "streams": [{"codec_type": codec_type, "codec_name": codec_name}]
+ }
+ if expected:
+ assert file_utils.should_copy_video(video_filepath, output_format)
+ else:
+ assert not file_utils.should_copy_video(video_filepath, output_format)
+
+
+def test_should_copy_video_exception() -> None:
+ with mock.patch("ffmpeg.probe") as ffmpeg_probe:
+ import ffmpeg
+
+ ffmpeg_probe.side_effect = ffmpeg.Error("ffprobe", "nothing", "nothing good")
+ assert not file_utils.should_copy_video(Path("unknown_video.mp4"))

cdp_backend/utils/file_utils.py

@@ -719,14 +719,25 @@ def clip_and_reformat_video(
 
  output_path = output_path or append_to_stem(video_filepath, "_clipped")
 
+ output_kwargs = {"format": output_format}
+ if should_copy_video(video_filepath, output_format):
+ log.info(
+ f"Video {video_filepath} is already h264, "
+ "it will be clipped and copied instead of clipped and re-encoded."
+ )
+ output_kwargs["codec"] = "copy"
+
  try:
  ffmpeg_stdout, ffmpeg_stderr = (
  ffmpeg.input(
  video_filepath,
  ss=start_time or "0",
  to=end_time or "99:59:59",
  )
- .output(filename=str(output_path), format=output_format)
+ .output(
+ filename=str(output_path),
+ **output_kwargs,
+ )
  .run(capture_stdout=True, capture_stderr=True)
  )
  except ffmpeg._run.Error as e:
@@ -739,3 +750,52 @@ def clip_and_reformat_video(
  log.error(ffmpeg_stderr)
 
  return output_path
+
+
+def should_copy_video(video_filepath: Path, output_format: str = "mp4") -> bool:
+ """
+ Check if the video should be copied using ffmpeg StreamCopy codec or if it should
+ be re-encoded as h264.
+
+ A video will be copied iff the following conditions are met:
+ - The video at video_filepath has a .mp4 extension
+ - The desired output format is mp4
+ - The video at video_filepath has a video stream with a codec of h264
+
+ Parameters
+ ----------
+ video_filepath: Path
+ The filepath of the video under scrutiny.
+ output_format: str
+ The desired output format of the video at video_filepath.
+
+ Returns
+ -------
+ bool:
+ True if the video should be copied, False if it should be re-encoded.
+ """
+ if video_filepath.suffix.lower() != ".mp4":
+ return False
+
+ if output_format.lower() != "mp4":
+ return False
+
+ import ffmpeg
+
+ try:
+ streams = ffmpeg.probe(video_filepath)["streams"]
+ except ffmpeg.Error as e:
+ log.warning(
+ f"Failed to probe {video_filepath}. "
+ "Unable to determine if video should be copied or re-encoded."
+ f"Falling back to re-encoding. ffmpeg error: {e.stderr}"
+ )
+ return False
+
+ should_copy_video = False
+ for stream in streams:
+ if stream["codec_type"] == "video" and stream["codec_name"] == "h264":
+ should_copy_video = True
+ break
+
+ return should_copy_video