Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

VideoEncoderConfig.framerate ? #103

Closed
chcunningham opened this issue Nov 6, 2020 · 5 comments
Closed

VideoEncoderConfig.framerate ? #103

chcunningham opened this issue Nov 6, 2020 · 5 comments
Assignees
Labels
extension Interface changes that extend without breaking.
Milestone

Comments

@chcunningham
Copy link
Collaborator

Currently Chromium's VideoEncoderConfig has a framerate parameter that is not part of the spec. Lets figure out if it should be.

Frame duration is required by libvpx vpx_codec_encode.
https://source.chromium.org/chromium/chromium/src/+/master:third_party/libvpx/source/libvpx/vpx/src/vpx_encoder.c;drc=223645aa83091fd88473ad2ddf20f80682b60a47;l=194

vp8 and vp9 both use it to decide compression vs speed
https://source.chromium.org/chromium/chromium/src/+/master:third_party/libvpx/source/libvpx/vp9/vp9_cx_iface.c;drc=1066e389d45dd39f22f2daf582fc56948d5cc480;l=1141

and both use to compute dts end for "receive raw frame" -- not sure what that's about. @jzern?
https://source.chromium.org/chromium/chromium/src/+/master:third_party/libvpx/source/libvpx/vp9/vp9_cx_iface.c;drc=1066e389d45dd39f22f2daf582fc56948d5cc480;l=1190

Summarizing the current state in Chromium:

  1. first try to get duration from videoFrame
  2. then try to get from config framerate
  3. guess based on clamped frame pts delta

Open questions:

  1. (@jzern) how bad is a wrong guess? if we guess 24fps when its really 10fps or 60fps, or highly variable fps, how big are the consequences? We probably can't make duration mandatory for every VideoFrame, but we might at least warn users if there is significant danger leaving it unspecified

  2. do we really need it in the config?

  • for highly variable framerate (some webcams), its not helpful
  • for constant framerate, its nice to have vs specifying duration on every frame. but also, for constant framerate the guessed duration will always be correct after the first frame (caveat: assuming its within the clamp range).
@jzern
Copy link

jzern commented Nov 6, 2020

vp8 and vp9 both use it to decide compression vs speed

We're really only using these codecs in one of 2 ways, realtime or good quality mode. VPX_DL_GOOD_QUALITY is 1000000 so pick_quickcompress_mode() most likely won't have any effect (and if it did would be 1 pass encoding). In chrome it's possible we're forcing realtime, I don't remember, but Android does this.

and both use to compute dts end for "receive raw frame" -- not sure what that's about. @jzern?

Not dts, but dst. This is just a difference between the libvpx call and the older wrappers for vp8/vp9 which take start and end times rather than timestamp and duration.

  1. (@jzern) how bad is a wrong guess? if we guess 24fps when its really 10fps
    or 60fps, or highly variable fps, how big are the consequences? We probably
    can't make duration mandatory for every VideoFrame, but we might at least
    warn users if there is significant danger leaving it unspecified

The worst outcome would be toggling between good and realtime quality, but as I said above that would be hard. You'd need a duration in microseconds to be > 1000000 where we'd normally be in the tens of millisecond range.

@sandersdan
Copy link
Contributor

Note to self: if users don't want to use milliseconds in the timestamp field (see #122), we could support that use by relying only on the duration field when it is present.

@chcunningham chcunningham self-assigned this Feb 19, 2021
@chcunningham chcunningham added this to the Q1 2021 milestone Feb 19, 2021
@chcunningham
Copy link
Collaborator Author

Moving from V1: this is a maybe-nice-to-have config addition. Adding it later is not a breaking change.

@chcunningham
Copy link
Collaborator Author

Triage note: marking 'extension', as this just adds a new field to the config.

@chcunningham
Copy link
Collaborator Author

Closing obsolete bug. VideoEncoderConfig now has framerate.
https://w3c.github.io/webcodecs/#dom-videoencoderconfig-framerate

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
extension Interface changes that extend without breaking.
Projects
None yet
Development

No branches or pull requests

3 participants