Skip to content

Conversation

@Dan-Flores
Copy link
Contributor

This PR adds output frame_rate to the API.

Previously, we always encoded videos using the same frame_rate as the input frames.

Now, we can set the output frame_rate when calling to_file/to_tensor/to_filelike (as in AudioEncoder), to adjusts the encoded video's frame durations. If no output frame_rate is specified, fallback to reusing the input frame_rate.

Testing:

To test, I added test_frame_rate_parameter which slows and speeds up a test video and ensures the video metadata is updated correctly. I also observed the video plays back slower or faster.

API after changes:

The complete API now looks as follows:

  encoder = VideoEncoder(frames=frames, frame_rate=25)
  encoder.to_file(
      dest="video.mp4",
      codec="libx264",
      pixel_format="yuv420p",
      crf=20,
      preset="medium",
      extra_options={
		"tune": "fastdecode",		
		"x264-params": "deblock=-1,-1:aq-mode=2
	  },
      frame_rate=25,
  )

@meta-cla meta-cla bot added the CLA Signed This label is managed by the Meta Open Source bot. label Nov 14, 2025
pixel_format: Optional[str] = None,
crf: Optional[Union[int, float]] = None,
preset: Optional[Union[str, int]] = None,
frame_rate: Optional[int] = None,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm happy to bikeshed on parameter order. sample_rate is the last param in AudioEncoder, but that constructor is much smaller. Here, having it last might make frame_rate less discoverable?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My views:

  1. Optional parameters should always be keyword-only.
  2. codec should be the first keyword-only parameter, as it's likely to be the most used.
  3. extra_options should be the last keyword-only parameter, as "catch-all" parameters tend to be last.

I don't have a preference for the order of the rest. 🚲 🛖 !

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't realize extra_options wasn't keyword-only, sorry for not catching that earlier. Let's make it keyword-only and keep it as the last parameter (it's probably going to be the least used param). frame_rate can go anywhere between codec and extra_options

Copy link
Contributor

@NicolasHug NicolasHug left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR @Dan-Flores !

I think there are different ways to adjust the frame rate: in this PR we're keeping the frames exactly as they are and we adjust their duration. Effectively if the input video is 1 min long and we ask for twice the frame rate, then we just halved the video duration to 30s, so it's running at twice the speed.

I'm not entirely sure this is what we want - I think generally users would expect the same output duration and some frames to be duplicated, dropped, or potentially interpolated in their temporal dimension.

Adding output sample_rate support to the audio encoder was pretty hard, it involved internal FIFOs etc. I suspect adding output frame_rate support will be similarly difficult, at the very least we'll have to figure out what the FFmpeg CLI does and reproduce that.

I'd suggest to hold off for now. I think the features we have in the encoder are already fairly complete, we can leave that as future improvement!

@NicolasHug NicolasHug mentioned this pull request Nov 17, 2025
@Dan-Flores Dan-Flores marked this pull request as draft November 17, 2025 14:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Meta Open Source bot.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants