Add codec options to VideoEncoder API #1050

Dan-Flores · 2025-11-13T22:16:17Z

This PR adds the codec_options dictionary arg to enable all remaining codec arguments.
The Python API accepts a Dict[str, str], converts it to a flattened List[str] to pass to custom_ops.py, which rebuilds the dict in unflattenCodecOptions().

Validation

To validate numeric options passed into codec_options, validateDoubleOption is refactored into tryToValidateCodecOption:

Check the type for each option
If numeric, attempt to convert provided value to a double
Validate value is in valid range
Do no validation for non-numeric options

Testing

The error handling done in tryToValidateCodecOption is documented in test_codec_options_errors.

Via local testing, I also checked that:

sortCodecOptions properly finds and sets format options correctly
Complex args such as {"-x264-params": "keyint=120:bframes=3"} are utilized

NicolasHug

Thanks @Dan-Flores , looks great! Made a few comments below.

NicolasHug · 2025-11-14T11:10:12Z

src/torchcodec/encoders/_video_encoder.py

        pixel_format: Optional[str] = None,
        crf: Optional[Union[int, float]] = None,
        preset: Optional[Union[str, int]] = None,
+        codec_options: Optional[Dict[str, str]] = None,


Could we make it Dict[str, Any]? I think there's value in allowing "some_param":16 instead of the un-pythonic "some_param: "16". We just need to call str() on all the values, which is OK.

NicolasHug · 2025-11-14T11:11:00Z

src/torchcodec/encoders/_video_encoder.py

                a string: "fast", "medium", "slow"). Defaults to None
                (which will use encoder's default).
+            codec_options (dict[str, str], optional): A dictionary of codec-specific
+                options to pass to the encoder, e.g. ``{"preset": "slow", "tune": "film"}``.


We already have preset as a built-in parameter so it might be confusing to document it here.

NicolasHug · 2025-11-14T11:15:38Z

src/torchcodec/_core/Encoder.h

+  void sortCodecOptions(
+      const std::map<std::string, std::string>& codecOptions,
+      AVDictionary** codecDict,
+      AVDictionary** formatDict);


This seems like it could be a pure function in an anonymous namespace rather than a method?

NicolasHug · 2025-11-14T11:17:57Z

src/torchcodec/encoders/_video_encoder.py

 from torchcodec import _core


+def _flatten_codec_options(codec_options: Optional[Dict[str, str]]) -> Optional[list]:


Nit: write that at the bottom like the rest of the helpers?

I probably don't need a function to do this in Python, I'll remove the function altogether.

NicolasHug · 2025-11-14T11:19:35Z

test/test_encoders.py

+                "avcodec_open2 failed: Invalid argument",
+            ),
+        ],
+    )


Nice job on the parametrization above

NicolasHug · 2025-11-14T11:31:04Z

src/torchcodec/_core/ops.py

    crf: Optional[Union[int, float]] = None,
    pixel_format: Optional[str] = None,
    preset: Optional[str] = None,
+    codec_options: Optional[list[str]] = None,


I'm starting to wonder if that's the right name for this parameter. IIUC, this can also be used to set the AVFormatContext parameters, right? So it's not just related to the codec itself?

Maybe this could be extra_options or something like that? CC @scotts @mollyxu

Yeah, extra_options is probably better.

test/test_encoders.py

NicolasHug · 2025-11-14T11:33:44Z

src/torchcodec/_core/Encoder.cpp

+      av_dict_set(formatDict, key.c_str(), value.c_str(), 0);
+    } else {
+      // Default to codec option (includes AVCodecContext + encoder-private)
+      // validateCodecOption(*avCodecContext_->codec, key.c_str(), value);


Remove:

Suggested change

// validateCodecOption(*avCodecContext_->codec, key.c_str(), value);

NicolasHug · 2025-11-14T11:36:33Z

src/torchcodec/_core/Encoder.cpp

+    AVDictionary** codecDict,
+    AVDictionary** formatDict) {


This interface is OK for now but we should consider changing it once we use RAII types for the dics (see my other follow-up suggestion).

NicolasHug · 2025-11-14T11:38:30Z

src/torchcodec/_core/Encoder.cpp

+    const std::map<std::string, std::string>& codecOptions,
+    AVDictionary** codecDict,
+    AVDictionary** formatDict) {
+  // Search AVFormatContext's AVClass for options


Let's remove this comment, it doesn't add much value. Let's also document what this function is doing: it takes some options as input and sorts them into codec options and format options, which are returned into two separate dicts.

…o codec_options_encode_option

NicolasHug

Nice work @Dan-Flores !

NicolasHug · 2025-11-14T18:13:20Z

src/torchcodec/encoders/_video_encoder.py

+            extra_options=[
+                x for k, v in (extra_options or {}).items() for x in (k, str(v))
+            ],


I was going to suggest something like that instead of the previous _flatten_codec_options but refrained as I thought it might be too crazy lol.

I like it, I'd just suggest the following, which is a tiiiiiny bit safer, and makes it more obvious that everything in the list is a string.

Suggested change

extra_options=[

x for k, v in (extra_options or {}).items() for x in (k, str(v))

],

extra_options=[

str(x) for k, v in (extra_options or {}).items() for x in (k, v)

],

NicolasHug · 2025-11-14T18:14:50Z

src/torchcodec/encoders/_video_encoder.py

+            extra_options (dict[str, Any], optional): A dictionary of additional
+                encoder options to pass, e.g. ``{"preset": "slow", "tune": "film"}``.
+                Values will be converted to strings before passing to the encoder.


Make sure to align the docs (this one still have preset), same with the one below

NicolasHug · 2025-11-14T18:20:03Z

test/test_encoders.py

+        actual_codec_spec = self._get_video_metadata(dest, fields=["codec_name"]).get(
+            "codec_name"
+        )


here and everywhere, use the [] syntax instead of .get(). We usually get() only when we want to specify a fallback value, which I don't think is intended here.

NicolasHug · 2025-11-14T18:26:51Z

test/test_encoders.py

+        # Validate profile (case-insensitive, baseline is reported as "Constrained Baseline")
+        assert profile in metadata.get("profile", "").lower()


I'd suggest the following, because using in is less strict and can sometimes be surprisingly easy to pass, like assert "" in "abc"

Suggested change

# Validate profile (case-insensitive, baseline is reported as "Constrained Baseline")

assert profile in metadata.get("profile", "").lower()

expected_profile = "constrained baseline" if profile == "baseline" else profile

assert metadata["profile"].lower() == expected_profile

add codec options, apply numeric error handling

1d79594

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

Dan-Flores marked this pull request as ready for review November 14, 2025 05:22

NicolasHug reviewed Nov 14, 2025

View reviewed changes

Dan-Flores added 6 commits November 14, 2025 09:05

inline dict flattening, accept str, Any

e4d3ede

namespace, var names, other suggestions

e6fd72b

wip test

888c8d4

Merge branch 'main' of https://github.com/meta-pytorch/torchcodec int…

5629e6e

…o codec_options_encode_option

add codec_options test, generalize ffprobe function to reuse there

36a2c41

rename codec_options to extra_options

7548687

Dan-Flores mentioned this pull request Nov 14, 2025

Add UniqueAVDictionary class and utilize it in VideoEncoder #1053

Open

NicolasHug approved these changes Nov 14, 2025

View reviewed changes

str cast, docs update, [], remove 'in' from test

f933cef

Dan-Flores merged commit c69064f into meta-pytorch:main Nov 14, 2025
70 checks passed

Dan-Flores deleted the codec_options_encode_option branch November 14, 2025 21:51

		from torchcodec import _core


		def _flatten_codec_options(codec_options: Optional[Dict[str, str]]) -> Optional[list]:

		# Validate profile (case-insensitive, baseline is reported as "Constrained Baseline")
		assert profile in metadata.get("profile", "").lower()

Add codec options to VideoEncoder API #1050

Add codec options to VideoEncoder API #1050

Uh oh!

Conversation

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

Uh oh!

Validation

Testing

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!

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!

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Dan-Flores commented Nov 13, 2025 •

edited

Loading