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

Inconsistency of Sending Bitate with Available Outgoing Bitrate #989

Open
sarumjanuch opened this issue Jan 25, 2023 · 22 comments · Fixed by #992
Open

Inconsistency of Sending Bitate with Available Outgoing Bitrate #989

sarumjanuch opened this issue Jan 25, 2023 · 22 comments · Fixed by #992
Labels
Milestone

Comments

@sarumjanuch
Copy link
Contributor

sarumjanuch commented Jan 25, 2023

During my work on BWE backport branch, I have noticed that mediasoup sometimes sends at higher bitrate than that which is reported by BWE algorithm. I tried to make this reproducible as much as possible so the setup is next: mediasoup-demo consumerReplicas branch running on the localhost, with two browser tabs opened on the same machine. Tab1: produce=false&consumerReplicas=19. Tab2: consumer=false&consumerReplicas=19. So we have single producer that produces simulcast stream, and 20 consumers replicas on the other side. Inside Trasnport.cpp I have hardcoded available bitrate to be 60 Mbps, it is never changing. Despite that this is what can be seen on graphs:
image

It seems that it correlates with packet loss:
image

In the same time RTX bitrate is relatively small to explain such a big gap:
image

webrtc-internals send side:
image

webrtc-internals receive side:
image

Distribution of available bitrate seems to be fine per logs: https://pastebin.com/inTDYp2Q, but actual sending bitrate is not.

chrome://webrtc-intenals for one of this cases: https://drive.google.com/file/d/1nvciwm9SW3KAUjAPHCQtMbQN9fs2YKar/view?usp=sharing

@ibc
Copy link
Member

ibc commented Jan 25, 2023

Even if mic is muted it's still generating bitrate. Can this be repeated after closing the micProducer? CC.disableMic() in console in sender.

@ibc
Copy link
Member

ibc commented Jan 25, 2023

More things to be asked by us:

  1. Are we considering the RTX traffic sent from sender to mediasoup when we count the Producer bitrate?
  2. Does the rtpSendBitrate in transport stats from mediasoup to receiver include RTX traffic generated from mediasoup to receiver?
  3. When we calculate the bitrate of each layer in the Producer streams that mediasoup receive, we consider the original size of packets being received my mediasoup. However, when mediasoup relays those packets to receivers it adds many RTP header extensions that increase their size. Could this be the main reason (not excluding 1 and 2)?

@jmillan
Copy link
Member

jmillan commented Jan 26, 2023

I've tried several times and didn't reproduce it.

@sarumjanuch, could you please create a webrtc-internals dump so we can have eyes on the same data, by for example, importing it here https://fippo.github.io/webrtc-dump-importer/ ?

@sarumjanuch
Copy link
Contributor Author

It takes some time for this to happen. https://drive.google.com/file/d/1nvciwm9SW3KAUjAPHCQtMbQN9fs2YKar/view?usp=sharing

@jmillan
Copy link
Member

jmillan commented Jan 26, 2023

Are we considering the RTX traffic sent from sender to mediasoup when we count the Producer bitrate?

Yes, those RTX packets that where nacked are considered.

https://github.com/versatica/mediasoup/blob/v3/worker/src/RTC/RtpStreamRecv.cpp#L392

Does the rtpSendBitrate in transport stats from mediasoup to receiver include RTX traffic generated from mediasoup to receiver?

No, it's added to rtxSendBitrate.

https://github.com/versatica/mediasoup/blob/v3/worker/src/RTC/Transport.cpp#L2680

@sarumjanuch
Copy link
Contributor Author

sarumjanuch commented Jan 26, 2023

Even if mic is muted it's still generating bitrate. Can this be repeated after closing the micProducer? CC.disableMic() in console in sender.

Per stats dump we don't see any audio bitrate at the moment when this issue happens, that could explain this gap. All is from video bitrate.

@jmillan
Copy link
Member

jmillan commented Jan 26, 2023

Looking at the provided webrtc-internals dump it can be seen that in the time the bitrate increases that much (around 20Mbps), there are 10 consumers which received bitrate increases (around 2Mbps). The others 10 don't. It looks like we are increasing the layer for those 10 consumers.

@sarumjanuch
Copy link
Contributor Author

@jmillan The one that i provided must include data for 3 peer connections 2 from sender 1 from receiver. BTW I thought that it may have something to do with my branch, so just in case, i just compiled latest v3 branch and was able to reproduce that, for me it usually takes 10-12 minutes for issue to appear.

@sarumjanuch
Copy link
Contributor Author

Attaching V3 dump just in case.
webrtc_internals_dump_v3.txt

@sarumjanuch
Copy link
Contributor Author

Attaching another one with only 2 PC on v3 branch. This time it took way much longer for issue to appear. So be patient when importing webrtc-dump.
https://drive.google.com/file/d/1oYcncLf5yWTHl505XNu5RVIZVuW16k8n/view?usp=sharing

@ibc
Copy link
Member

ibc commented Jan 27, 2023

Not really related but anyway: #992

@sarumjanuch
Copy link
Contributor Author

Attaching webrtc-internals with mediasoup debug log.

webrtc_internals_dump_with_debug_log_1.txt
mediasoup_debug_1.log

@sarumjanuch
Copy link
Contributor Author

As per logs of mediasoup in 12:45:17 we should receive only one 2:1 layer:

2023-01-27T12:45:17.912Z mediasoup:Channel [pid:48227] RTC::SimulcastConsumer::IncreaseLayer() | setting provisional layers to 2:0 [virtual bitrate:20984935, required bitrate:2201235]
2023-01-27T12:45:17.912Z mediasoup:Channel [pid:48227] RTC::SimulcastConsumer::IncreaseLayer() | testing layers 2:0 [virtual bitrate:18783700, required bitrate:2201235]
2023-01-27T12:45:17.912Z mediasoup:Channel [pid:48227] RTC::SimulcastConsumer::IncreaseLayer() | setting provisional layers to 2:0 [virtual bitrate:18783700, required bitrate:2201235]
2023-01-27T12:45:17.912Z mediasoup:Channel [pid:48227] RTC::SimulcastConsumer::IncreaseLayer() | testing layers 2:0 [virtual bitrate:16582465, required bitrate:2201235]
2023-01-27T12:45:17.912Z mediasoup:Channel [pid:48227] RTC::SimulcastConsumer::IncreaseLayer() | setting provisional layers to 2:0 [virtual bitrate:16582465, required bitrate:2201235]
2023-01-27T12:45:17.912Z mediasoup:Channel [pid:48227] RTC::SimulcastConsumer::IncreaseLayer() | testing layers 2:0 [virtual bitrate:14381230, required bitrate:2201235]
2023-01-27T12:45:17.912Z mediasoup:Channel [pid:48227] RTC::SimulcastConsumer::IncreaseLayer() | setting provisional layers to 2:0 [virtual bitrate:14381230, required bitrate:2201235]
2023-01-27T12:45:17.912Z mediasoup:Channel [pid:48227] RTC::SimulcastConsumer::IncreaseLayer() | testing layers 2:0 [virtual bitrate:12179995, required bitrate:2201235]
2023-01-27T12:45:17.912Z mediasoup:Channel [pid:48227] RTC::SimulcastConsumer::IncreaseLayer() | setting provisional layers to 2:0 [virtual bitrate:12179995, required bitrate:2201235]
2023-01-27T12:45:17.912Z mediasoup:Channel [pid:48227] RTC::SimulcastConsumer::IncreaseLayer() | testing layers 2:0 [virtual bitrate:9978760, required bitrate:2201235]
2023-01-27T12:45:17.912Z mediasoup:Channel [pid:48227] RTC::SimulcastConsumer::IncreaseLayer() | setting provisional layers to 2:0 [virtual bitrate:9978760, required bitrate:2201235]
2023-01-27T12:45:17.912Z mediasoup:Channel [pid:48227] RTC::SimulcastConsumer::IncreaseLayer() | testing layers 2:0 [virtual bitrate:7777525, required bitrate:2201235]
2023-01-27T12:45:17.912Z mediasoup:Channel [pid:48227] RTC::SimulcastConsumer::IncreaseLayer() | setting provisional layers to 2:0 [virtual bitrate:7777525, required bitrate:2201235]
2023-01-27T12:45:17.912Z mediasoup:Channel [pid:48227] RTC::SimulcastConsumer::IncreaseLayer() | testing layers 2:0 [virtual bitrate:5576290, required bitrate:2201235]
2023-01-27T12:45:17.912Z mediasoup:Channel [pid:48227] RTC::SimulcastConsumer::IncreaseLayer() | setting provisional layers to 2:0 [virtual bitrate:5576290, required bitrate:2201235]
2023-01-27T12:45:17.912Z mediasoup:Channel [pid:48227] RTC::SimulcastConsumer::IncreaseLayer() | avoid upgrading to spatial layer 2 due to recent BWE downgrade
2023-01-27T12:45:17.912Z mediasoup:Channel [pid:48227] RTC::SimulcastConsumer::IncreaseLayer() | testing layers 2:1 [virtual bitrate:3375055, required bitrate:2020326]
2023-01-27T12:45:17.912Z mediasoup:Channel [pid:48227] RTC::SimulcastConsumer::IncreaseLayer() | setting provisional layers to 2:1 [virtual bitrate:3375055, required bitrate:2020326]
2023-01-27T12:45:17.912Z mediasoup:Channel [pid:48227] RTC::SimulcastConsumer::IncreaseLayer() | testing layers 2:1 [virtual bitrate:1354729, required bitrate:2020326]
2023-01-27T12:45:17.912Z mediasoup:Channel [pid:48227] RTC::SimulcastConsumer::IncreaseLayer() | testing layers 2:1 [virtual bitrate:1354729, required bitrate:2020326]
2023-01-27T12:45:17.912Z mediasoup:Channel [pid:48227] RTC::SimulcastConsumer::IncreaseLayer() | testing layers 2:1 [virtual bitrate:1354729, required bitrate:2020326]
2023-01-27T12:45:17.912Z mediasoup:Channel [pid:48227] RTC::SimulcastConsumer::IncreaseLayer() | testing layers 2:1 [virtual bitrate:1354729, required bitrate:2020326]
2023-01-27T12:45:17.912Z mediasoup:Channel [pid:48227] RTC::SimulcastConsumer::IncreaseLayer() | testing layers 2:1 [virtual bitrate:1354729, required bitrate:2020326]
2023-01-27T12:45:17.912Z mediasoup:Channel [pid:48227] RTC::SimulcastConsumer::IncreaseLayer() | testing layers 2:1 [virtual bitrate:1354729, required bitrate:2020326]
2023-01-27T12:45:17.912Z mediasoup:Channel [pid:48227] RTC::SimulcastConsumer::IncreaseLayer() | testing layers 2:1 [virtual bitrate:1354729, required bitrate:2020326]
2023-01-27T12:45:17.912Z mediasoup:Channel [pid:48227] RTC::SimulcastConsumer::IncreaseLayer() | testing layers 2:1 [virtual bitrate:1354729, required bitrate:2020326]
2023-01-27T12:45:17.912Z mediasoup:Channel [pid:48227] RTC::SimulcastConsumer::IncreaseLayer() | testing layers 2:1 [virtual bitrate:1354729, required bitrate:2020326]
2023-01-27T12:45:17.912Z mediasoup:Channel [pid:48227] RTC::SimulcastConsumer::IncreaseLayer() | testing layers 2:1 [virtual bitrate:1354729, required bitrate:2020326]
2023-01-27T12:45:17.912Z mediasoup:Channel [pid:48227] RTC::SimulcastConsumer::IncreaseLayer() | testing layers 2:1 [virtual bitrate:1354729, required bitrate:2020326]
2023-01-27T12:45:17.912Z mediasoup:Channel [pid:48227] RTC::SimulcastConsumer::IncreaseLayer() | testing layers 2:1 [virtual bitrate:1354729, required bitrate:2020326]
2023-01-27T12:45:17.912Z mediasoup:Channel [pid:48227] RTC::SimulcastConsumer::IncreaseLayer() | testing layers 2:1 [virtual bitrate:1354729, required bitrate:2020326]
2023-01-27T12:45:17.912Z mediasoup:Channel [pid:48227] RTC::SimulcastConsumer::IncreaseLayer() | testing layers 2:1 [virtual bitrate:1354729, required bitrate:2020326]
2023-01-27T12:45:17.912Z mediasoup:Channel [pid:48227] RTC::SimulcastConsumer::IncreaseLayer() | testing layers 2:1 [virtual bitrate:1354729, required bitrate:2020326]
2023-01-27T12:45:17.912Z mediasoup:Channel [pid:48227] RTC::SimulcastConsumer::IncreaseLayer() | testing layers 2:1 [virtual bitrate:1354729, required bitrate:2020326]
2023-01-27T12:45:17.912Z mediasoup:Channel [pid:48227] RTC::SimulcastConsumer::IncreaseLayer() | testing layers 2:1 [virtual bitrate:1354729, required bitrate:2020326]
2023-01-27T12:45:17.912Z mediasoup:Channel [pid:48227] RTC::SimulcastConsumer::IncreaseLayer() | testing layers 2:1 [virtual bitrate:1354729, required bitrate:2020326]
2023-01-27T12:45:17.912Z mediasoup:Channel [pid:48227] RTC::SimulcastConsumer::IncreaseLayer() | avoid upgrading to spatial layer 2 due to recent BWE downgrade
2023-01-27T12:45:17.912Z mediasoup:Channel [pid:48227] RTC::SimulcastConsumer::IncreaseLayer() | testing layers 2:1 [virtual bitrate:1354729, required bitrate:2020326]
2023-01-27T12:45:17.912Z mediasoup:Channel [pid:48227] RTC::SimulcastConsumer::IncreaseLayer() | testing layers 2:1 [virtual bitrate:1354729, required bitrate:2020326]
2023-01-27T12:45:17.912Z mediasoup:Channel [pid:48227] RTC::SimulcastConsumer::IncreaseLayer() | testing layers 2:1 [virtual bitrate:1354729, required bitrate:2020326]
2023-01-27T12:45:17.912Z mediasoup:Channel [pid:48227] RTC::SimulcastConsumer::IncreaseLayer() | testing layers 2:1 [virtual bitrate:1354729, required bitrate:2020326]
2023-01-27T12:45:17.912Z mediasoup:Channel [pid:48227] RTC::SimulcastConsumer::IncreaseLayer() | testing layers 2:1 [virtual bitrate:1354729, required bitrate:2020326]
2023-01-27T12:45:17.912Z mediasoup:Channel [pid:48227] RTC::SimulcastConsumer::IncreaseLayer() | testing layers 2:1 [virtual bitrate:1354729, required bitrate:2020326]
2023-01-27T12:45:17.912Z mediasoup:Channel [pid:48227] RTC::SimulcastConsumer::IncreaseLayer() | testing layers 2:1 [virtual bitrate:1354729, required bitrate:2020326]
2023-01-27T12:45:17.912Z mediasoup:Channel [pid:48227] RTC::SimulcastConsumer::IncreaseLayer() | testing layers 2:1 [virtual bitrate:1354729, required bitrate:2020326]
2023-01-27T12:45:17.912Z mediasoup:Channel [pid:48227] RTC::SimulcastConsumer::IncreaseLayer() | testing layers 2:1 [virtual bitrate:1354729, required bitrate:2020326]
2023-01-27T12:45:17.912Z mediasoup:Channel [pid:48227] RTC::SimulcastConsumer::IncreaseLayer() | testing layers 2:1 [virtual bitrate:1354729, required bitrate:2020326]
2023-01-27T12:45:17.912Z mediasoup:Channel [pid:48227] RTC::SimulcastConsumer::IncreaseLayer() | testing layers 2:1 [virtual bitrate:1354729, required bitrate:2020326]
2023-01-27T12:45:17.912Z mediasoup:Channel [pid:48227] RTC::SimulcastConsumer::IncreaseLayer() | testing layers 2:1 [virtual bitrate:1354729, required bitrate:2020326]
2023-01-27T12:45:17.912Z mediasoup:Channel [pid:48227] RTC::SimulcastConsumer::IncreaseLayer() | testing layers 2:1 [virtual bitrate:1354729, required bitrate:2020326]
2023-01-27T12:45:17.912Z mediasoup:Channel [pid:48227] RTC::SimulcastConsumer::IncreaseLayer() | testing layers 2:1 [virtual bitrate:1354729, required bitrate:2020326]
2023-01-27T12:45:17.912Z mediasoup:Channel [pid:48227] RTC::SimulcastConsumer::IncreaseLayer() | testing layers 2:1 [virtual bitrate:1354729, required bitrate:2020326]
2023-01-27T12:45:17.912Z mediasoup:Channel [pid:48227] RTC::SimulcastConsumer::IncreaseLayer() | testing layers 2:1 [virtual bitrate:1354729, required bitrate:2020326]
2023-01-27T12:45:17.912Z mediasoup:Channel [pid:48227] RTC::SimulcastConsumer::IncreaseLayer() | testing layers 2:1 [virtual bitrate:1354729, required bitrate:2020326]
2023-01-27T12:45:17.912Z mediasoup:Channel [pid:48227] RTC::SimulcastConsumer::IncreaseLayer() | testing layers 2:1 [virtual bitrate:1354729, required bitrate:2020326]
2023-01-27T12:45:17.912Z mediasoup:Channel [pid:48227] RTC::SimulcastConsumer::IncreaseLayer() | avoid upgrading to spatial layer 2 due to recent BWE downgrade
2023-01-27T12:45:17.912Z mediasoup:Channel [pid:48227] RTC::Transport::DistributeAvailableOutgoingBitrate() | after layer-by-layer iterations [availableBitrate:1354729]

But per webrt-internals dump 19 of 20 consumers are already receiving highest bitrate layer at that time.

@ibc ibc closed this as completed in #992 Jan 27, 2023
@jmillan jmillan reopened this Jan 27, 2023
@jmillan
Copy link
Member

jmillan commented Jan 27, 2023

@ibc, I guess you closed this one by accident.

@ibc
Copy link
Member

ibc commented Jan 27, 2023

Wow didn't notice

jmillan added a commit to jmillan/mediasoup that referenced this issue Jan 30, 2023
Fixes versatica#989

* An old payload cannot upgrade the current temporal layer.
* Update current temporal layer to the target one.
jmillan added a commit to jmillan/mediasoup that referenced this issue Jan 30, 2023
Fixes versatica#989

* An old payload cannot upgrade the current temporal layer.
* Update current temporal layer to the target one.
jmillan added a commit to jmillan/mediasoup that referenced this issue Jan 31, 2023
Fixes versatica#989

* An old payload cannot upgrade the current temporal layer.
* Update current temporal layer to the target one.
jmillan added a commit to jmillan/mediasoup that referenced this issue Jan 31, 2023
@jmillan
Copy link
Member

jmillan commented Jan 31, 2023

As far as we have seen, the spatial layer selection done in SimulcastConsumer is correct. This moves the target to the codec implementation, which in the case of this exact tests is VP8.

Basically we are forwarding every old packet. Old being a lower pictureId than the most recent one. This means that no matter we've been in temporal layer 0 for one hour, every temporal layer 1 packets are being forwarded.

This is a brave attempt to fix it, by no forwarding any packet which temporal layer is greater than the current one.

I've tested locally and found no problems. But longer time tests will be conducted in order to see if this issue is fixed.

This is the only concern I had:

  • Current temporal layer: 1
  • RTP packet with sequence number 1000 and payload with temporal layer 1 is forwarded
  • Current temporal layer switches to: 0
  • A NACK for the RTP packet with sequence number 1000 arrives.
  • VP8::PayloadDescriptorHandler::Process would drop this packet with the new logic.
  • The above point will not happen because retransmissions do not pass through the codec Process() method again.

NOTE: Once confirmed other codecs will be changed.

@ibc
Copy link
Member

ibc commented Feb 1, 2023

  • Current temporal layer: 1
  • RTP packet with sequence number 1000 and payload with temporal layer 1 is forwarded
  • Current temporal layer switches to: 0
  • A NACK for the RTP packet with sequence number 1000 arrives.

I understand that the requested RTP packet MUST be forwarded right?

  • VP8::PayloadDescriptorHandler::Process would drop this packet with the new logic.

Here you say that the packet is drop.

  • The above point will not happen because retransmissions do not pass through the codec Process() method again.

Here you say that "above point" won't happen. Do you mean that packet won't be drop but correctly forwarded?

In the other side, I don't understand the first change in the diff referenced in your message. Why checking == target temporal layer instead of original check?

@jmillan
Copy link
Member

jmillan commented Feb 1, 2023

The requested RTP packet will be anyways forwarded. We won't call VP8::PayloadDescriptorHandler::Process() for it, which is totally correct.

So the only concern I had (in past tense), about retransmissions being filtered, does not apply, so I have no negative concerns about the change.

I don't understand the first change in the diff referenced in your message. Why checking == target temporal layer instead of original check?

Why would we switch to whatever layer as long as it's greater than current? We should always change to the target layer specifically. Am I wrong?

@ibc
Copy link
Member

ibc commented Feb 1, 2023

I don't understand the first change in the diff referenced in your message. Why checking == target temporal layer instead of original check?

Why would we switch to whatever layer as long as it's greater than current? We should always change to the target layer specifically. Am I wrong?

I'd lie if I said that I really understand how the whole logic in Process() works.

@jmillan
Copy link
Member

jmillan commented Feb 1, 2023

I don't understand the first change in the diff referenced in your message. Why checking == target temporal layer instead of original check?

In any case, if we want to upgrade to a specific higher layer it will be to the target layer. The layer selection is done in the corresponding Consumer (Simulcast or SVC) implementation where we set the target layer based on available bandwidth.

@ibc
Copy link
Member

ibc commented Feb 1, 2023

In any case, if we want to upgrade to a specific higher layer it will be to the target layer. The layer selection is done in the corresponding Consumer (Simulcast or SVC) implementation where we set the target layer based on available bandwidth.

That's my understanding. We check, verify and so on layer changes in the Consumer and, once done, we tell it to the packet context via packet->ProcessPayload(), so the packet context should honor what we already decided and never choose different things, right?

@jmillan
Copy link
Member

jmillan commented Feb 1, 2023

so the packet context should honor what we already decided and never choose different things, right?

Correct.

Some tests have been added

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3 participants