-
Notifications
You must be signed in to change notification settings - Fork 706
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
Add vp9 svc support by Dependency Descriptor #1586
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -273,8 +273,8 @@ func (f *Forwarder) DetermineCodec(codec webrtc.RTPCodecCapability) { | |
case "video/vp8": | ||
f.isTemporalSupported = true | ||
f.vp8Munger = NewVP8Munger(f.logger) | ||
case "video/av1": | ||
// TODO : we only enable dd layer selector for av1 now, at future we can | ||
case "video/av1", "video/vp9": | ||
// TODO : we only enable dd layer selector for av1 and vp9 now, at future we can | ||
// enable it for vp8 too | ||
f.ddLayerSelector = NewDDVideoLayerSelector(f.logger) | ||
} | ||
|
@@ -515,7 +515,7 @@ func (f *Forwarder) AllocateOptimal(availableLayers []int32, brs Bitrates, allow | |
} | ||
alloc.TargetLayers = buffer.VideoLayer{ | ||
Spatial: int32(math.Min(float64(f.maxPublishedLayer), float64(maxSpatial))), | ||
Temporal: buffer.DefaultMaxLayerTemporal, | ||
Temporal: f.maxLayers.Temporal, | ||
} | ||
} | ||
|
||
|
@@ -570,14 +570,14 @@ func (f *Forwarder) AllocateOptimal(availableLayers []int32, brs Bitrates, allow | |
alloc.TargetLayers.Spatial = l | ||
} | ||
} | ||
alloc.TargetLayers.Temporal = buffer.DefaultMaxLayerTemporal | ||
alloc.TargetLayers.Temporal = f.maxLayers.Temporal | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This change to fix client preferred fps (temporary layer), can you verify it is correct? @boks1971 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It should be okay @cnderrauber , but we probably need to allow that opportunistic higher layer. I will looking at adding it back for simulcast tracks and use proper layer for DD tracks. |
||
|
||
alloc.RequestLayerSpatial = alloc.TargetLayers.Spatial | ||
} else { | ||
requestLayerSpatial := int32(math.Min(float64(f.maxLayers.Spatial), float64(f.maxPublishedLayer))) | ||
if f.currentLayers.IsValid() && requestLayerSpatial == f.requestLayerSpatial && f.currentLayers.Spatial == f.requestLayerSpatial { | ||
// current is locked to desired, stay there | ||
alloc.TargetLayers = f.currentLayers | ||
alloc.TargetLayers = buffer.VideoLayer{Spatial: f.requestLayerSpatial, Temporal: f.maxLayers.Temporal} | ||
alloc.RequestLayerSpatial = f.requestLayerSpatial | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Might have to revert these changes back to how they were and deal with layer limiting based on DD or not. But, I will take care of it in the refactor branch. |
||
} else { | ||
// opportunistically latch on to anything | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't fully understand this. I cannot read the bit stream format document :-). But, this RFC seems to say that we do not need to dig beyond the VP9 Payload Descriptor to determine key frame. I may change this function based on that when I do my refactor. Guess, we really do not need to know if key frame or not with dependency descriptor. We will need that only when using simulcast.
https://datatracker.ietf.org/doc/html/draft-ietf-payload-vp9-16. The actual text
This is what I used in the refactor branch I am working on. I will compare with the above and check if they match.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's true, actually, it works well without this keyframe detection function, just add it to be consistent with other codecs.