Add crf to VideoEncoder API #1031

Dan-Flores · 2025-11-07T19:23:55Z

Since crf was already utlized in the C++ layer, this PR adds crf to the python API, and moves the tests from test_ops.py to test_encoders.py.

Validation

The function validateNumericOption lets us validate an argument if its AVOption has min and max fields. This error checking is applied to crf to improve our error message.
FFmpeg's output error message:

RuntimeError: avcodec_open2 failed: Result too large

To our own message:

RuntimeError: crf=-10 is out of valid range [-1, 3.40282e+38] for this codec. For more details, run 'ffmpeg -h encoder=libx264'

RuntimeError: crf=-10 is out of valid range [0, 63] for this codec. For more details, run 'ffmpeg -h encoder=libsvtav1'

RuntimeError: crf=-10 is out of valid range [-1, 63] for this codec. For more details, run 'ffmpeg -h encoder=libvpx-vp9'

Testing

The tests are updated to use the python API encoding pattern:

# Previous ops pattern:
encode_video_to_file(
            frames=source_frames,
            frame_rate=frame_rate,
            filename=encoder_output_path,
            pixel_format=pixel_format,
            crf=crf,
        )

# Updated python pattern:
encoder = VideoEncoder(frames=source_frames, frame_rate=frame_rate)
encoder.to_file(dest=encoder_output_path, pixel_format=pixel_format, crf=crf)

scotts · 2025-11-10T20:17:22Z

src/torchcodec/encoders/_video_encoder.py

            pixel_format (str, optional): The pixel format to encode frames into (e.g.,
                "yuv420p", "yuv444p"). If not specified, uses codec's default format.
+            crf (int, optional): Constant Rate Factor for encoding quality. Lower values
+                mean better quality. Valid range depends on the encoder (commonly 0-51).


Is it ever valid to be less than 0?

I believe -1 is valid and is equivalent to leaving crf unset. Otherwise, no negative values are valid.

scotts · 2025-11-10T20:18:54Z

I think this is great! We should add some tests for invalid crf values, both less than 0 (which I think is always invalid?), values we know are outside of the range for a given codec and the wrong type. It's fine if these result in exceptions on the C++ side, but we want to make sure users get a clean Python exception and not a segfault.

NicolasHug · 2025-11-11T10:56:58Z

src/torchcodec/encoders/_video_encoder.py

        Args:
            format (str): The container format of the encoded frames, e.g. "mp4", "mov",
-            "mkv", "avi", "webm", "flv", or "gif"
+                "mkv", "avi", "webm", "flv", etc.


Q - why remove "gif"? Do we not support it anymore?

We do not test explicitly for it anymore, but it still works. I mostly wanted to amend the docstring to make it seem less like a finalized, exhaustive list of supported formats.

NicolasHug · 2025-11-11T11:02:05Z

test/test_encoders.py

+
+        for s_frame, rt_frame in zip(source_frames, round_trip_frames):
+            assert psnr(s_frame, rt_frame) > 30
+            torch.testing.assert_close(s_frame, rt_frame, atol=2, rtol=0)


This seems to be failing for webm, you might need to use the previous logic

# If FFmpeg selects a codec or pixel format that does lossy encoding, assert 99% of pixels # are within a higher tolerance. if ffmpeg_version == 6: assert_close = partial(assert_tensor_close_on_at_least, percentage=99) atol = 15 else: assert_close = torch.testing.assert_close atol = 3 if format == "webm" else 2

Thanks for the reminder - it seems I applied the webm tolerance to the wrong test. We can simply use atol = 3 if format == "webm" else 2 on the round_trip_test, though I'm not sure why webm needs this special handling.

Dan-Flores · 2025-11-11T20:39:32Z

test/test_encoders.py

+                frame_rate=30,
+            )
+            getattr(encoder, method)(**valid_params, crf=-10)
+


This is the only new test case - all other tests are copied over from test_ops.py.

NicolasHug

Thanks @Dan-Flores , let's address the comments before merging but LGTM

NicolasHug · 2025-11-12T09:40:44Z

src/torchcodec/_core/Encoder.cpp

+  if (option->type == AV_OPT_TYPE_INT || option->type == AV_OPT_TYPE_INT64 ||
+      option->type == AV_OPT_TYPE_FLOAT || option->type == AV_OPT_TYPE_DOUBLE) {
+    TORCH_CHECK(
+        value >= option->min && value <= option->max,


This is comparing an int (value) to a double (min and max), we should cast.

This comment led me to realize that codecs can implement 'crf' as a double or an int.
I'll update the PR to accept either type, and treat it as a double in the C++, so this casting will not be necessary.

NicolasHug · 2025-11-12T09:42:04Z

src/torchcodec/_core/Encoder.cpp

+    const char* optionName,
+    int value) {
+  // First determine if codec's private class is defined
+  if (!avCodec.priv_class) {


are we OK to use priv_class? I.e. is it meant to be "private"?

In the avcodec.h, priv_class is defined in the section for public fields: https://www.ffmpeg.org/doxygen/2.0/libavcodec_2avcodec_8h_source.html

src/torchcodec/_core/Encoder.cpp

NicolasHug · 2025-11-12T09:50:43Z

src/torchcodec/_core/Encoder.cpp

  TORCH_CHECK(false, errorMsg.str());
 }
+
+void validateNumericOption(


For now this expects an int, not a float or double. Let's reflect that in the name. We may define this with a template later to be truely about generic numeric options.

Suggested change

void validateNumericOption(

void validateIntOption(

meta-cla bot added the CLA Signed This label is managed by the Meta Open Source bot. label Nov 7, 2025

Dan-Flores added 4 commits November 10, 2025 10:13

add crf to api, move and update tests

4b275f4

set crf as optional

740a675

move tests

54a8294

integrate pixel_format test changes

8fcc181

Dan-Flores force-pushed the crf_encode_option branch from 715db20 to 8fcc181 Compare November 10, 2025 19:09

Dan-Flores marked this pull request as ready for review November 10, 2025 19:21

add webm tolerance

d1e5bdf

scotts reviewed Nov 10, 2025

View reviewed changes

NicolasHug reviewed Nov 11, 2025

View reviewed changes

Dan-Flores added 2 commits November 11, 2025 08:21

add webm tolerance to the round trip test

222e74d

add numeric validation, apply to crf

14e797b

Dan-Flores commented Nov 11, 2025

View reviewed changes

NicolasHug approved these changes Nov 12, 2025

View reviewed changes

crf is somtimes a double actually

b7e52fb

Dan-Flores mentioned this pull request Nov 12, 2025

Add preset to VideoEncoder API #1042

Merged

Dan-Flores merged commit c69739f into meta-pytorch:main Nov 13, 2025
71 of 79 checks passed

Dan-Flores deleted the crf_encode_option branch November 13, 2025 16:06

Add crf to VideoEncoder API #1031

Add crf to VideoEncoder API #1031

Uh oh!

Conversation

Dan-Flores commented Nov 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Validation

Testing

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

scotts commented Nov 10, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

NicolasHug left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Dan-Flores commented Nov 7, 2025 •

edited

Loading