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

Force key frame on PLI #338

Merged
merged 1 commit into from
Jul 25, 2022
Merged

Conversation

EmrysMyrddin
Copy link
Contributor

@EmrysMyrddin EmrysMyrddin commented May 19, 2021

Description

This is a first POC around handling PLI for codecs on which we have control over key frame generation.

I have added the idea of an EncoderController instead of adding ForceKeyFrame and SetBitRate directly on the ReadCloser interface.

This allows more flexibility on encoder implementation. The main benefit is that you can optionally implement bit rate and key frame control. In our case, only 2 codecs have this implementations.

Using interfaces for each controls allows to check if an encoder have this controls and enable features only on compatible encoders.

The next step probably to add handling of bit rate negotiation packets, but this PR is already pretty long so I don't want to put too much stuff in it for now.

Reference issue

This PR needs some modification in the pion/webrtc package for being able to read RTCP feedbacks. Here is the related PR that needs to be merged before this one: pion/webrtc#1812

@EmrysMyrddin EmrysMyrddin changed the title Implements RequireKeyFrame for VPX codec Force key frame on PLI May 19, 2021
@EmrysMyrddin EmrysMyrddin force-pushed the force-key-frame-on-pli branch 4 times, most recently from 456950d to 668f0fb Compare May 19, 2021 14:58
Copy link
Member

@at-wat at-wat left a comment

Choose a reason for hiding this comment

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

API design looks nice to me. Left some minor comments.

pkg/codec/codec.go Outdated Show resolved Hide resolved
pkg/codec/mmal/mmal.go Outdated Show resolved Hide resolved
pkg/codec/vpx/vpx.go Outdated Show resolved Hide resolved
@EmrysMyrddin EmrysMyrddin force-pushed the force-key-frame-on-pli branch 3 times, most recently from 97035d2 to e2b71a1 Compare November 4, 2021 11:24
@EmrysMyrddin
Copy link
Contributor Author

Stil waiting for pion/webrtc#1812 to be merged for being able to make this PR ready to merge.

Do you have any insigh on what I can do to make it happen ?

.idea/misc.xml Outdated
@@ -0,0 +1,6 @@
<?xml version="1.0" encoding="UTF-8"?>
Copy link
Member

Choose a reason for hiding this comment

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

@EmrysMyrddin Remove this file please

@Sean-Der Sean-Der force-pushed the force-key-frame-on-pli branch 4 times, most recently from 7ad4cfc to f687012 Compare July 25, 2022 14:42
Also make the ReadCloser an Controllable allows to uncouple
the controller implementation from the encoder.
This is not needed for the 2 codec controller already implemented (openh264 and vpx)
but is more future proof in case it required for other codecs.
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