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

fix increase layer when current layer has not receive SR #1098

Merged
merged 5 commits into from
May 19, 2024

Conversation

penguinol
Copy link
Contributor

@penguinol penguinol commented Jun 1, 2023

For transport without bandwidth estimation, such as PlainRtpTransport, we will only change layers when receiving first SR of each stream or there is a change in the score.
But in follow situation, we can not switch to the preferred layer.
We may receive SR of preferred layer before we receiving keyframe of current target layer, when currentSpitalLayer is still -1.
After we received keyframe of target layer, we received another SR of preferred layer, but it's not the first SR of this stream, so we will not call MayChangeLayers

sequenceDiagram
Src ->> mediasoup: CreateProducer S3T3
Dst->> mediasoup: CreateConsumer, perferred(2, 2)
Src ->> mediasoup: non-keyframe of S1
mediasoup->>mediasoup: targetSpatialLayer = 1, tsReferenceSpitalLayer = 1, s1 score = 10
Src ->> mediasoup: non-keyframe of S2
mediasoup->>mediasoup: s2 score = 10
Src ->> mediasoup: SenderReport of S1
mediasoup->>mediasoup: save ntp of S1
Src ->> mediasoup: SenderReport of S2
mediasoup->>mediasoup: fisrt SR of S2, but currentSpitalLayer is -1, does not call `MayChangeLayers` 
Src ->> mediasoup: key-frame of S1
mediasoup->>mediasoup: currentSpatialLayer = 1
Src ->> mediasoup: SenderReport of S2 
mediasoup->>mediasoup: not first SR of S2, does not call `MayChangeLayers`
Loading

@ibc
Copy link
Member

ibc commented May 19, 2024

I don't really understand this part:
CleanShot 2024-05-19 at 19 36 23@2x

If no keyframe of S1 is received, why does the graph says that tsReferenceSpatialLayer is 1? AFAIK it's -1 at this time.

@ibc
Copy link
Member

ibc commented May 19, 2024

@penguinol do you think this PR can also fix issue #492 and replace this old but never addressed PR #500?

@ibc
Copy link
Member

ibc commented May 19, 2024

Ok I think I get it. I'll test and merge. Sorry for the delay.

@ibc ibc merged commit 26ee0df into versatica:v3 May 19, 2024
34 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants