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

Enable playout-delay extension + NACKs + janus version bumps #41

Closed
wants to merge 1 commit into from
Closed

Enable playout-delay extension + NACKs + janus version bumps #41

wants to merge 1 commit into from

Conversation

danstiner
Copy link
Contributor

@danstiner danstiner commented Jan 7, 2022

This is not meant to merge as-is, but I wanted a place to coordinate rolling out the janus-ftl-plugin changes I've been making and we've been discussing over on Glimesh/janus-ftl-plugin#135. (edit: those changes have merged) This follows my original plan in Glimesh/janus-ftl-plugin#126 (comment)

This specifically targets what changes need to happen to turn on the playout-delay extension from Glimesh/janus-ftl-plugin#134, because it involves a few things:

  • Janus must be built with a patch applied to support the playout-delay extension. For convenience I have a branch ready based on Janus v0.11.6 in my fork of janus-gateway
  • Then the ftl plugin must be built with janus_playout_delay_support=true
  • This enables support, but the ftl plugin will not actually send playout-delay values by default. You still have to set env variables FTL_PLAYOUT_DELAY_MIN_MS and FTL_PLAYOUT_DELAY_MAX_MS to reasonable values.

I totally support your idea to deploy this to just one server to start with (note playout-delay is only meaningful on edge servers though). After deploying I'd want to verify:

  • In chrome://webrtc-internals/, the value of jitterBufferDelay/jitterBufferEmittedCount_in_ms should never really dip below the configured min playout delay
  • Check if Large keyframes can result in consistent freezes on every keyframe janus-ftl-plugin#101 is mitigated. Ideally verify for the same stream that an unpatched edge node repros the issue and a patched edge node does not repro. My usual test stream is a white noise background with a small moving clock etc
  • Run a stream over a lossy network to an ingest with NACKs enabled and an edge with playout-delay enabled. Compare the packet NACK and loss rates to the same content streamed to an ingest with NACKs enabled and a "legacy" edge node without playout delay enabled. Hopefully packet loss rate goes down while the NACK rate remains about the same. If the loss rate gets worse I'd be concerned.
  • Let the change sit for awhile and see if anyone reports issues.

While this does implicitly bump the janus version and libsrtp version, there was not impact I saw or expected from that, just keeping up with dependencies.

@clone1018
Copy link
Member

clone1018 commented Jan 17, 2022

and my patch of Janus should be pulled from https://github.com/danstiner/janus-gateway/tree/rtp-playout-delay-extension over to https://github.com/Glimesh/janus-gateway (I don't have permission to that repo)

I've added maintainer permissions for the "Video" team in all video related repositories, let me know if this works!

@danstiner
Copy link
Contributor Author

Thanks! I think it worked. I pushed the patched branch over and just merged Glimesh/janus-ftl-plugin#136 so I think everything is in place now.

I don't have a way to test these ansible changes so no guarantees they work, but they should be close.

Copy link
Contributor

@LukeHandle LukeHandle left a comment

Choose a reason for hiding this comment

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

Syntactically things look okay.

This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants