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

Add control over cacheSize, customizable hasCongestion #1302

Closed
mkrn opened this issue Oct 12, 2023 · 27 comments
Closed

Add control over cacheSize, customizable hasCongestion #1302

mkrn opened this issue Oct 12, 2023 · 27 comments

Comments

@mkrn
Copy link
Contributor

mkrn commented Oct 12, 2023

Is your feature request related to a problem? Please describe.
While implementing a custom bitrateAdapter it became clear that hasCongestion method cannot be relied upon, because it always return true on all devices.
The method and RTMPSender class have hardcoded values for cache size and fullness criteria

fun hasCongestion(): Boolean {
    val size = cacheSize
    val remaining = cacheSize - itemsInQueue
    val capacity = size + remaining
    return size >= capacity * 0.2f //more than 20% queue used. You could have congestion
  }

The itemsInQueue is private, so it cannot be read from the outside

Describe the solution you'd like
Make 0.2f hardcoded magic value customizable, and/or make itemsInQueue public so the queue could be better inspected in a custom bitrateAdapter.

Describe alternatives you've considered
tried not using hasCongestion, but it's prone to creating a stream drop

Additional context

@pedroSG94
Copy link
Owner

Hello,

You are right. I added an optional value for it:
#1303

Anyway, I also detected a bug in that method and it was updated in the last release and it should work as expected with the current default value:
https://github.com/pedroSG94/RootEncoder/blob/master/rtmp/src/main/java/com/pedro/rtmp/rtmp/RtmpSender.kt#L188

Let me know if it is working as you expected to fix/update the PR and merge it

@mkrn
Copy link
Contributor Author

mkrn commented Oct 12, 2023

Thanks a lot! That bug in kotlin version of hasCongestion was the issue.
As to wether keep those percentage methods, I'm in favor, just in case some phones handle queue differently.
Thank you for creating it so swiftly

@pedroSG94
Copy link
Owner

I will test the PR a bit and merge for next version

@mkrn
Copy link
Contributor Author

mkrn commented Oct 12, 2023

@pedroSG94 there's another major problem:
when there are a lot of droppedVideoFrames the queue stays small and this doesn't affect the bitrate adapter in any way.

Here's how I test:

  • Using iPhone, setup Developer mode in Settings
  • Under "Network Conditioner" setup profile with max output bandwidth 3500 , 10% packet loss on exit, 100ms delay on packets sent
  • Switch iPhone to 5G, Use iPhone as Personal Hotspot, connect Android to it's WiFi hotspot

Result I see a lot of dropped frames, not affecting the queue size or bandwidth sent, so quality is not decreasing with Adapter

@mkrn
Copy link
Contributor Author

mkrn commented Oct 12, 2023

at first about 200 frames get sent, 0 dropped, then all frames get dropped until there's timeout, it disconnects.

@pedroSG94
Copy link
Owner

I will try to check it but I'm not sure about the logic because dropped frames only increase if the queue is full and you try to enqueue other frame.

I can't do the suggested way to test because I only have an iPad without mobile data (only WiFi). I will try to find other way to reproduce it

@pedroSG94
Copy link
Owner

at first about 200 frames get sent, 0 dropped, then all frames get dropped until there's timeout, it disconnects.

Maybe rtmp sender stuck sending a frame so the queue is always full. I will try to find a way to check it

@mkrn
Copy link
Contributor Author

mkrn commented Oct 12, 2023

It's weird.. Im trying to check droppedVideoFrames onNewBitrateRtmp like this:

@Override
  public void onNewBitrateRtmp(final long bitrate) {
    LogManager.customLog(
      TAG,
      rtmpCamera2.getDroppedVideoFrames() +
      " / " +
      rtmpCamera2.getSentVideoFrames() +
      " / " +
      rtmpCamera2.getDroppedAudioFrames()
    );
    rtmpCamera2.resetDroppedVideoFrames();
    rtmpCamera2.resetDroppedAudioFrames();
    rtmpCamera2.resetSentVideoFrames();
    rtmpCamera2.resetSentAudioFrames();

to use the result in adaptBitrate but here's what I get:

first few seconds 0 dropped, 31 sent video, but then I get
850 dropped, 24 sent, 1334 dropped audio frames (onNewBitrateRtmp is not called for almost 30 seconds!)

Any ideas?

@pedroSG94
Copy link
Owner

If the send thread is stucked it is normal because that callback is called in that thread.

The question is why the send thread stuck for 30s

I'm looking a way to limit bandwidth and test it

@mkrn
Copy link
Contributor Author

mkrn commented Oct 12, 2023

I think at some point all frames get dropped and while they're all dropped, onNewBitrateRtmp is not called.
fpsListener seems to be called every second.
Can I call bitrateAdapter.adaptBitrate inside fpsListener ?

@pedroSG94
Copy link
Owner

Can you tell me bitrate used on the library?

Also, I assume that 3500 is kbps of upload bandwidth limit, right?

@mkrn
Copy link
Contributor Author

mkrn commented Oct 12, 2023

IMG_36C963A9FE43-1
My settings on iPhone serving as a hotspot

My settings in app:
minBitrate = 100 * 1024
maxBitrate = 3000 * 1024
startBitrate = 1500 * 1024

@pedroSG94
Copy link
Owner

fpsListener seems to be called every second.\nCan I call bitrateAdapter.adaptBitrate inside fpsListener ?

Because this listener use a different thread. It is using the GL render thread.

You can use it but you will have the same problem and the sender thread will keep stucked.

@mkrn
Copy link
Contributor Author

mkrn commented Oct 12, 2023

Ok, yes hopefully you can find how to get it unstuck, or run calculation on another thread so we can properly react (Reduce bitrate when frames are being dropped)

@pedroSG94
Copy link
Owner

I can limit bandwidth but I can't reproduce the case for stuck the sending thread.
Can you test this to know if this could be the reason?
Comment this line:
https://github.com/pedroSG94/RootEncoder/blob/master/rtmp/src/main/java/com/pedro/rtmp/rtmp/RtmpClient.kt#L231

I expect that after this, your sender thread never should stuck but you still lose frames as expected. Let me know if it is working and I will try find a fix

@mkrn
Copy link
Contributor Author

mkrn commented Oct 12, 2023

what's the best way to import the library as source to test it? Currently using the library like this:
implementation 'com.github.pedroSG94.RootEncoder:library:2.3.0'

@pedroSG94
Copy link
Owner

Try this way:
https://github.com/pedroSG94/RootEncoder/wiki/Add-library-to-your-project#manual
Maybe you also have to remove dokka plugin in each module gradle

@mkrn
Copy link
Contributor Author

mkrn commented Oct 13, 2023

Ok I've successfully added the library, and tried commending the line solution, but unfortunately the situation is the same, dropped frames pile up and no new onNewBitrate calls until after several seconds the stream is disconnected for not sending any data..

@mkrn
Copy link
Contributor Author

mkrn commented Oct 13, 2023

rtmpCamera2.resizeCache(30);
tried with the standard bitrate adapter, still same is happening

@pedroSG94
Copy link
Owner

pedroSG94 commented Oct 13, 2023

Hello,

At least I can discard that read while writing is the reason. About resize cache this has no effect because the problem is not related with cache size. it is related with the sender thread stopping to send.

Now we can check which method stuck the thread. Print few logs here:
https://github.com/pedroSG94/RootEncoder/blob/master/rtmp/src/main/java/com/pedro/rtmp/rtmp/CommandsManager.kt#L207
https://github.com/pedroSG94/RootEncoder/blob/master/rtmp/src/main/java/com/pedro/rtmp/rtmp/CommandsManager.kt#L208
https://github.com/pedroSG94/RootEncoder/blob/master/rtmp/src/main/java/com/pedro/rtmp/rtmp/CommandsManager.kt#L209

Also, in iphone configuration try to remove "perda di pacote di saida" and "atraso di saida" to discard that it is the problem.
To emulate your case limiting bandwidth I used this app:
https://play.google.com/store/apps/details?id=com.network.speedbooster&hl=en_419&gl=US
You can try use it to limit bandwidth and we can use the same environment to test. This limit wifi and mobile data connection so you can use it without a sim card.

In my case I only was able to produce discard frames that is the expected way that the library should works but it is never stucked.

Other test could be use a different device to know if you only can reproduce it with one device

@mkrn
Copy link
Contributor Author

mkrn commented Oct 14, 2023

ok I tested on 2 devices:

  • Google Pixel 4
  • Samsung Galaxy S21

This happens when there's any "packet loss"

Frames just get discarded continiously:

10-14 10:39:48.527  1894  4349 I RtmpSender: Audio frame discarded
10-14 10:39:48.528  1894  4349 I RtmpSender: Audio frame discarded
10-14 10:39:48.529  1894  4349 I RtmpSender: Audio frame discarded
10-14 10:39:48.531  1894  4349 I RtmpSender: Audio frame discarded
10-14 10:39:48.533  1894  4349 I RtmpSender: Audio frame discarded
10-14 10:39:48.534  1894  4349 I RtmpSender: Audio frame discarded
10-14 10:39:48.542  1894  4338 I RtmpSender: Video frame discarded
10-14 10:39:48.575  1894  4338 I RtmpSender: Video frame discarded
10-14 10:39:48.609  1894  4338 I RtmpSender: Video frame discarded
10-14 10:39:48.643  1894  4338 I RtmpSender: Video frame discarded
10-14 10:39:48.647  1894  4349 I RtmpSender: Audio frame discarded
10-14 10:39:48.648  1894  4349 I RtmpSender: Audio frame discarded
10-14 10:39:48.678  1894  4338 I RtmpSender: Video frame discarded
10-14 10:39:48.687  1894  4349 I RtmpSender: Audio frame discarded
10-14 10:39:48.688  1894  4349 I RtmpSender: Audio frame discarded
10-14 10:39:48.690  1894  4349 I RtmpSender: Audio frame discarded
10-14 10:39:48.710  1894  4338 I RtmpSender: Video frame discarded
10-14 10:39:48.743  1894  4338 I RtmpSender: Video frame discarded
10-14 10:39:48.767  1894  4349 I RtmpSender: Audio frame discarded
10-14 10:39:48.768  1894  4349 I RtmpSender: Audio frame discarded
10-14 10:39:48.774  1894  4338 I RtmpSender: Video frame discarded
10-14 10:39:48.807  1894  4349 I RtmpSender: Audio frame discarded
10-14 10:39:48.808  1894  4349 I RtmpSender: Audio frame discarded
10-14 10:39:48.811  1894  4349 I RtmpSender: Audio frame discarded
10-14 10:39:48.811  1894  4338 I RtmpSender: Video frame discarded
10-14 10:39:48.812  1894  4349 I RtmpSender: Audio frame discarded

Any way to detect it to lower bitrate?

My idea is something like this:

  @Override
  public void onNewBitrateRtmp(final long bitrate) {
 
    bitrateAdapter.adaptBitrate(
      bitrate, rtmpCamera2.getDroppedVideoFrames()
    ) 

   rtmpCamera2.resetDroppedVideoFrames();
    rtmpCamera2.resetDroppedAudioFrames();
    rtmpCamera2.resetSentVideoFrames();
    rtmpCamera2.resetSentAudioFrames();

but onNewBitrateRtmp is not called while many frames are discarded (I can see new getDroppedVideoFrames count in rtmpCamera2.setFpsListener)

@pedroSG94
Copy link
Owner

pedroSG94 commented Oct 14, 2023

Hello,

This will guarantee that bitratemanager is called each second no matter if sender thread is blocked or not:
fccdf20
If you already have the project manually compiled you can add this commit in the rtmpsender and test it.

Now, the only problem is the reason about the sender never finish to send a packet. If possible try to test using the app mentioned above to use the same environment than me and know if the problem is related with iPhone limiter. If it is the case I can considerate buy an old iPhone to reproduce the case and try to fix it if possible.

@pedroSG94
Copy link
Owner

pedroSG94 commented Oct 14, 2023

This happens when there's any "packet loss"

Do you means that remove packet loss in the iPhone limiter fix the problem? This could be related with TCP protocol. It is a protocol that need that server response to confirm that the packet is send and if it fail, it will do a retry because it is a protocol that guarantee that all is correctly received in the other side.
Maybe the iPhone limiter produce that this never happens with the packet loss and stuck the sender for that reason (basically it is waiting for a response that never happens).

@mkrn
Copy link
Contributor Author

mkrn commented Oct 16, 2023

Thank you! This was extremely helpful! Now I was able to add to my custom bitrate adapter, if there are any dropped frames I immediately drop bitrate to the minumum and this lets me continue the stream without drops.
Also, clearCache is useful! I think we're lacking to see exactly how full the cache is.

@pedroSG94
Copy link
Owner

Thank you! This was extremely helpful! Now I was able to add to my custom bitrate adapter, if there are any dropped frames I immediately drop bitrate to the minumum and this lets me continue the stream without drops. Also, clearCache is useful! I think we're lacking to see exactly how full the cache is.

Ok, I will add a method for it today

@pedroSG94
Copy link
Owner

Added here:
#1308

@pedroSG94
Copy link
Owner

All this changes was added to version 2.3.1

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

No branches or pull requests

3 participants
@mkrn @pedroSG94 and others