Adaptive bitrate doesn't work

marcin-adamczewski commented 4 years ago

Hi @pedroSG94 I'm testing BitrateAdapter and I've implemented it the same way as it is in the example. When I switch from high-speed WiFI to 3G the bitrate is not adapted properly and it is very high. The same situation when I start streaming from 3G. The tested upload speed of 3G is ~1.5Mb/s, however, adapted bitrate is ~5Mb/s (which is my max bitrate) resulting in lags on stream.

Maybe I'm doing something wrong, but I've checked the source code and the bitrate that you pass to onNewBitrateRtmp(bitrate) callback may not be correct. It's taken from RtmpConnection.publishVideoData(size) just after you write the data to the socket. I think that socket has an internal buffer and the data size you write to socket's outputstream doesn't have to be equal to the size of data that has been uploaded. I've compared this bitrate value to the value from TrafficStats.getTotalTxBytes(), which shows the amount of data that has been uploaded and there are discrepancies between them. Here is the code to quickly compare it

private long bitrateSum = 0;
  private long initialTotalUploadBytes = 0;
  private long previousCheckTimeMs = 0L;
  private long lastTotalUploadBytes = 0;

  private BitrateAdapter adapter = new BitrateAdapter(new BitrateAdapter.Listener() {
    @Override
    public void onBitrateAdapted(int bitrate) {
      Log.d("lol2", "bitrate adapted: " + bitrate);
      rtmpCamera2.setVideoBitrateOnFly(bitrate);
    }
  });

  @Override
  public void onConnectionSuccessRtmp() {
    adapter.setMaxBitrate(5 * 1024 * 1024);
    initialTotalUploadBytes = TrafficStats.getTotalTxBytes();
  }

  @Override
  public void onNewBitrateRtmp(long bitrate) {
    Log.d("lol", "onNewBitrate: " + bitrate);
    adapter.adaptBitrate((int) bitrate);

    bitrateSum += bitrate;
    Log.d("lol", "Bitrate sum so far: " + bitrateSum / 1024f / 1024f + " Mb");

    long uploadedBytesSoFar = TrafficStats.getTotalTxBytes() - initialTotalUploadBytes;
    Log.d("lol", "Real upload so far: " + uploadedBytesSoFar * 8f / 1024f / 1024f + " Mb");

    long bytesDiff = uploadedBytesSoFar - lastTotalUploadBytes;
    long nowMs = System.currentTimeMillis();
    int timeDiff = (int) ((nowMs - previousCheckTimeMs) / 1000f);
    float realUploadSpeed = bytesDiff * 8f / timeDiff / 1024f / 1024f;
    Log.d("lol", "Real upload speed: " + realUploadSpeed + " Mb/s");

    previousCheckTimeMs = nowMs;
    lastTotalUploadBytes = uploadedBytesSoFar;
  }

There is also one interesting thing that I don't fully understand, maybe you could explain. Being on high-speed WiFI, setting rtmpCamera2.setVideoBitrateOnFly(20 * 1024 * 1024); determines the video bitrate but also the upload speed. I mean, when I set it to 20Mb/s like above the upload speed will be 20Mb/s, when I set it to 2Mb/s the upload speed will be 2Mb/s. So is there any contract in the RTMP that video quality affects upload speed? Can't I upload 2Mb/s quality video with 5Mb/s speed? Or maybe it depends on the internal buffer size?

pedroSG94 commented 4 years ago

I need test this code. I'm not sure if socket has a buffer but this is the only way to do it(That I know). Keep in mind that your code also count traffic with others apps.

About second question. I don't fully understand but when you set video bitrate your stream upload traffic speed is basically video + audio bitrate. Of course, if you have 20Mb/s of internet connection you can do 20Mb/s stream or less. Resolution and upload speed is not related you can do stream to 4k but with 1Mb/s bitrate (obviously you will have a pixelated stream) (I'm working in stream rotation for portrait mode but it will take a bit. For now, my idea is use a parameter for xml to configure it.)

pedroSG94 commented 4 years ago

I was testing bitrate callback and work fine. I did it: With default parameters 1200 1024 video bitrate + 64 1024 audio bitrate = 1294336 bps You need keep moving device to get max bitrate because H264 algorithm reduce bitrate if you have a static image to reduce traffic.

If you set a logcat in callback you get this:

@Override
  public void onNewBitrateRtmp(long bitrate) {
    Log.e("Pedro", "Lib: " + bitrate);
  }

2020-04-20 15:46:30.128 26416-26573/com.pedro.rtpstreamer E/Pedro: Lib: 88
2020-04-20 15:46:31.129 26416-26573/com.pedro.rtpstreamer E/Pedro: Lib: 1379104
2020-04-20 15:46:32.133 26416-26573/com.pedro.rtpstreamer E/Pedro: Lib: 1469184
2020-04-20 15:46:33.155 26416-26573/com.pedro.rtpstreamer E/Pedro: Lib: 1175216
2020-04-20 15:46:34.174 26416-26573/com.pedro.rtpstreamer E/Pedro: Lib: 1332528
2020-04-20 15:46:35.174 26416-26573/com.pedro.rtpstreamer E/Pedro: Lib: 1335928
2020-04-20 15:46:36.174 26416-26573/com.pedro.rtpstreamer E/Pedro: Lib: 1422536
2020-04-20 15:46:37.176 26416-26573/com.pedro.rtpstreamer E/Pedro: Lib: 1443672
2020-04-20 15:46:38.203 26416-26573/com.pedro.rtpstreamer E/Pedro: Lib: 1208616
2020-04-20 15:46:39.229 26416-26573/com.pedro.rtpstreamer E/Pedro: Lib: 1199072
2020-04-20 15:46:40.232 26416-26573/com.pedro.rtpstreamer E/Pedro: Lib: 1125568
2020-04-20 15:46:41.232 26416-26573/com.pedro.rtpstreamer E/Pedro: Lib: 1328304
2020-04-20 15:46:42.232 26416-26573/com.pedro.rtpstreamer E/Pedro: Lib: 1368376
2020-04-20 15:46:43.233 26416-26573/com.pedro.rtpstreamer E/Pedro: Lib: 1208864
2020-04-20 15:46:44.235 26416-26573/com.pedro.rtpstreamer E/Pedro: Lib: 1245304
2020-04-20 15:46:45.243 26416-26573/com.pedro.rtpstreamer E/Pedro: Lib: 1383432

With TrafficStats this is the result:

  private long trafficBits = 0;

  @Override
  public void onNewBitrateRtmp(long bitrate) {
    trafficBits = TrafficStats.getTotalTxBytes() - trafficBits;
    //convert to bits
    Log.e("Pedro", "Traffic: " + (trafficBits * 8));
  }

2020-04-20 15:50:44.455 26763-26903/com.pedro.rtpstreamer E/Pedro: Traffic: 1380680
2020-04-20 15:50:45.457 26763-26903/com.pedro.rtpstreamer E/Pedro: Traffic: 2298265360
2020-04-20 15:50:46.477 26763-26903/com.pedro.rtpstreamer E/Pedro: Traffic: 1988040
2020-04-20 15:50:47.479 26763-26903/com.pedro.rtpstreamer E/Pedro: Traffic: 2299791256
2020-04-20 15:50:48.495 26763-26903/com.pedro.rtpstreamer E/Pedro: Traffic: 3799392
2020-04-20 15:50:49.507 26763-26903/com.pedro.rtpstreamer E/Pedro: Traffic: 2301251952
2020-04-20 15:50:50.518 26763-26903/com.pedro.rtpstreamer E/Pedro: Traffic: 5320176
2020-04-20 15:50:51.534 26763-26903/com.pedro.rtpstreamer E/Pedro: Traffic: 2302591224
2020-04-20 15:50:52.556 26763-26903/com.pedro.rtpstreamer E/Pedro: Traffic: 6626216
2020-04-20 15:50:53.561 26763-26903/com.pedro.rtpstreamer E/Pedro: Traffic: 2303933416
2020-04-20 15:50:54.575 26763-26903/com.pedro.rtpstreamer E/Pedro: Traffic: 7976584
2020-04-20 15:50:55.592 26763-26903/com.pedro.rtpstreamer E/Pedro: Traffic: 2305371888
2020-04-20 15:50:56.594 26763-26903/com.pedro.rtpstreamer E/Pedro: Traffic: 9295552
2020-04-20 15:50:57.611 26763-26903/com.pedro.rtpstreamer E/Pedro: Traffic: 2306749000
2020-04-20 15:50:58.634 26763-26903/com.pedro.rtpstreamer E/Pedro: Traffic: 10569024
2020-04-20 15:50:59.664 26763-26903/com.pedro.rtpstreamer E/Pedro: Traffic: 2308116024
2020-04-20 15:51:00.673 26763-26903/com.pedro.rtpstreamer E/Pedro: Traffic: 11868016

marcin-adamczewski commented 4 years ago

@pedroSG94 Thanks for checking this. Did you try it on slow Internet with upload speed like 1Mb/s? You would also have to set initial bitrate in prepareVideo() to say 3-5Mb/s, otherwise, you may not see the issue.

How do you assume from these logs that it is working fine? I mean these values are very close to each other (~1Mb/s). I guess you'd need to switch from faster Internet to slower and back to the good one in order to test it well. So for WiFi the bitrate should raise, up to max bitrate (5Mb/s) and once you switch it should decrease to ~1Mb/s.

You can use my code to test as the logs there shows well the discrepancies in a nice way. If you'd like to use 3G instead of LTE, there is an option in Network settings like "Preferred network type" where you can set 3G as the default network.

Regarding the socket buffer I've mentioned, it is being set in RtmpConnection like socket.setSendBufferSize(acknowledgementWindowsize); with the value of 5 million in my case.

pedroSG94 commented 4 years ago

10Mbs using 3G (upload speed around 4Mbs).

2020-04-22 12:16:15.965 18740-18890/com.pedro.rtpstreamer E/lol: onNewBitrate: 8.392334E-5 Mb/s
2020-04-22 12:16:15.968 18740-18890/com.pedro.rtpstreamer E/lol: Real upload speed: 2.1545145E-6 Mb/s
2020-04-22 12:16:16.973 18740-18890/com.pedro.rtpstreamer E/lol: onNewBitrate: 17.849495 Mb/s
2020-04-22 12:16:16.974 18740-18890/com.pedro.rtpstreamer E/lol: Real upload speed: 0.37243652 Mb/s
2020-04-22 12:16:17.980 18740-18890/com.pedro.rtpstreamer E/lol: onNewBitrate: 8.128471 Mb/s
2020-04-22 12:16:17.981 18740-18890/com.pedro.rtpstreamer E/lol: Real upload speed: 0.51049805 Mb/s
2020-04-22 12:16:19.001 18740-18890/com.pedro.rtpstreamer E/lol: onNewBitrate: 11.777885 Mb/s
2020-04-22 12:16:19.003 18740-18890/com.pedro.rtpstreamer E/lol: Real upload speed: 1.1706848 Mb/s
2020-04-22 12:16:20.028 18740-18890/com.pedro.rtpstreamer E/lol: onNewBitrate: 1.742897 Mb/s
2020-04-22 12:16:20.030 18740-18890/com.pedro.rtpstreamer E/lol: Real upload speed: 1.8318481 Mb/s
2020-04-22 12:16:21.054 18740-18890/com.pedro.rtpstreamer E/lol: onNewBitrate: 15.497047 Mb/s
2020-04-22 12:16:21.055 18740-18890/com.pedro.rtpstreamer E/lol: Real upload speed: 3.0480347 Mb/s
2020-04-22 12:16:22.057 18740-18890/com.pedro.rtpstreamer E/lol: onNewBitrate: 15.295479 Mb/s
2020-04-22 12:16:22.058 18740-18890/com.pedro.rtpstreamer E/lol: Real upload speed: 4.0150604 Mb/s
2020-04-22 12:16:23.081 18740-18890/com.pedro.rtpstreamer E/lol: onNewBitrate: 9.659401 Mb/s
2020-04-22 12:16:23.083 18740-18890/com.pedro.rtpstreamer E/lol: Real upload speed: 3.8979187 Mb/s
2020-04-22 12:16:24.110 18740-18890/com.pedro.rtpstreamer E/lol: onNewBitrate: 1.9280396 Mb/s
2020-04-22 12:16:24.111 18740-18890/com.pedro.rtpstreamer E/lol: Real upload speed: 3.4475403 Mb/s

After delete line socket.setSendBufferSize(acknowledgementWindowsize);:

2020-04-22 12:17:51.831 19076-19214/com.pedro.rtpstreamer E/lol: onNewBitrate: 4.0180664 Mb/s
2020-04-22 12:17:51.832 19076-19214/com.pedro.rtpstreamer E/lol: Real upload speed: 4.3313904 Mb/s
2020-04-22 12:17:52.943 19076-19214/com.pedro.rtpstreamer E/lol: onNewBitrate: 3.9006958 Mb/s
2020-04-22 12:17:52.944 19076-19214/com.pedro.rtpstreamer E/lol: Real upload speed: 3.4375 Mb/s
2020-04-22 12:17:54.087 19076-19214/com.pedro.rtpstreamer E/lol: onNewBitrate: 3.7883148 Mb/s
2020-04-22 12:17:54.087 19076-19214/com.pedro.rtpstreamer E/lol: Real upload speed: 4.0672913 Mb/s
2020-04-22 12:17:55.262 19076-19214/com.pedro.rtpstreamer E/lol: onNewBitrate: 4.4927063 Mb/s
2020-04-22 12:17:55.263 19076-19214/com.pedro.rtpstreamer E/lol: Real upload speed: 4.1234436 Mb/s
2020-04-22 12:17:56.382 19076-19214/com.pedro.rtpstreamer E/lol: onNewBitrate: 3.8261108 Mb/s
2020-04-22 12:17:56.384 19076-19214/com.pedro.rtpstreamer E/lol: Real upload speed: 3.8874817 Mb/s
2020-04-22 12:17:57.465 19076-19214/com.pedro.rtpstreamer E/lol: onNewBitrate: 3.7574768 Mb/s
2020-04-22 12:17:57.466 19076-19214/com.pedro.rtpstreamer E/lol: Real upload speed: 4.792206 Mb/s
2020-04-22 12:17:58.507 19076-19214/com.pedro.rtpstreamer E/lol: onNewBitrate: 4.0800247 Mb/s
2020-04-22 12:17:58.508 19076-19214/com.pedro.rtpstreamer E/lol: Real upload speed: 3.2387543 Mb/s

You are right, socket buffer was the problem (fixed).

marcin-adamczewski commented 4 years ago

Hi @pedroSG94 Is it safe to remove this buffer? What it was used for before?

I have also some concerns regarding the algorithm itself. Imagine this case:

We're on fast WiFi (10Mb/s) and the max bitrate is set to 5Mb/s. Adapted bitrate will be ~5Mb/s then.
Then, we turn off WiFI and switch to 3G where upload speed is 3.5Mb/s.
Unfortunately, the algorithm won't decrease the bitrate to 3.5Mb/s. It is because of this line if (averageBitrate < oldBitrate * 0.65) { // decrease by 10% } a) so the averageBitrate is 5Mb/s, and the 0.65 of it = 3.25 (won't decrease) b) after 5 seconds the average bitrate will be ~3.5Mb/s, oldBitrate is still 5Mb/s, so 3.5 < 3.25 will still return false and the bitrate won't decrease.

You can try it yourself. Anyways it would be very useful to have such an algorithm unit tested, otherwise it will be hard to test all cases.

Moreover, I can still notice discrepancies between real upload speed and bitrate from onNewBitrate method. I think the problem is the way the bitrate is being calculated. In the BitrateManager.calculateBitrate() there is

 if (timeDiff >= 1000) {
      connectCheckerRtmp.onNewBitrateRtmp(bitrate);

The timeDiff may be 3 seconds so the real bitrate is bitrate / 3s. IMO it should be like this

 if (timeDiff >= 1000) {
      connectCheckerRtmp.onNewBitrateRtmp((long) (bitrate / (timeDiff / 1000f)));

Now it's better but there is another issue. You removed setting the buffer, but the buffer is still there, so I guess there is some default value.

pedroSG94 commented 4 years ago

Yes, buffer size has a default value (1048576) but I'm not sure if change that value to 1 could produce a problem (0 is not allowed according to java documentation) and now, both bitrate are similar, so I'm doing a fix to decrease bitrate in all case

pedroSG94 commented 4 years ago

I think this could work:

  private int getBitrateAdapted2(int bitrate) {
    if (bitrate >= maxBitrate) { //You have high speed and max bitrate. Keep max speed
      oldBitrate = maxBitrate;
      return oldBitrate;
    } else if (bitrate <= oldBitrate) { //You have low speed and bitrate too high. Reduce bitrate by 10%.
      oldBitrate = (int) (bitrate * 0.9);
      return oldBitrate;
    } else { //You have high speed and bitrate too low. Increase bitrate by 10%.
      oldBitrate = (int) (bitrate * 1.1);
      if (oldBitrate > maxBitrate) oldBitrate = maxBitrate;
      return oldBitrate;
    }
  }

AlexUrrutia commented 4 years ago

Hello Pedro, are you planning a release soon fixing this kind of things? I'm launching an app next week and I'd like to compile the best version of your library, thanks!

pedroSG94 commented 4 years ago

Yes, I will do it soon. I think this monday

marcin-adamczewski commented 4 years ago

@pedroSG94 This algorithm will reduce the bitrate if needed, but I'm not sure if it will increase it. Let's bring back again this scenario 5Mb/s to 3Mb/s.

I switch off WiFi (5Mb/s) and switch on 3G (3Mb/s).
Bitrate decreases to 3Mb/s. So far so good.
Now I switch back again WiFI and bitrate won't increase because the amount of data being uploaded will be the same as bitrate, right? So we have a bitrate of 3Mb/s and it will be also the upload speed I guess? That way the condition else if (bitrate <= oldBitrate) may be very often true.

Tell me if I'm wrong, but I guess the bitrate doesn't show the real upload speed. Anyways the case is worth testing.

AlexUrrutia commented 4 years ago

Yes, I will do it soon. I think this monday

Excellent!

pedroSG94 commented 4 years ago

@marcin-adamczewski You are right, bitrate it not totally real speed but it is very close (at least in my case the different is around 0.2 or 0.3Mb/s less than 10% error) and sometimes (bitrate <= oldBitrate) could be true and decrease bitrate. For this reason, I wanted ask your about it. My other idea is use (bitrate <= oldBitrate * 0.9f) to decrease chance to be true but this could produce others problems like lose packets because you can't decrease if upload speed is 10% less than bitrate used in app.

The best solution is use TrafficStats methods but we can't discard upload speed used in background apps.

marcin-adamczewski commented 4 years ago

Yeap that's probably better. I'd need to test it though. Regarding some small packets lose it this case won't the buffer help here?

Regarding the buffer, maybe it's size should be configurable? Have a look at this article https://www.red5pro.com/blog/what-is-buffer-time-and-how-does-it-affect-your-streaming/ So as I understand the bigger buffer the fewer lags on the client side, but on the other hand the stream is less real-time. So depending on the case you may want to adjust it. Other cases where buffer may be useful are slow devices, where video encoding to h264 could be slow, which actually reduces real bandwidth if there is no buffer. Btw I'm completely not an expert here so take my opinion with a grain of salt.

pedroSG94 commented 4 years ago

This is the value that we set with setSendBufferSize: https://docs.oracle.com/javase/8/docs/api/java/net/SocketOptions.html#SO_SNDBUF

In my experience(maybe I'm wrong), this isn't a buffer that you can use to discard frames because if you have this buffer full and you continue send packets, that is stored in memory waiting to enter in buffer but you haven't a limit(if you have a low speed the result is OutOfMemory error). I think that buffer work like: You send packets to buffer, socket wait fill that buffer and send it to server (this could explain upload speed difference).

About red5pro. I think they are referring to other buffer not that method. Maybe something like I have here(I use this to avoid OutOfMemory error): https://github.com/pedroSG94/rtmp-rtsp-stream-client-java/blob/master/rtmp/src/main/java/net/ossrs/rtmp/SrsFlvMuxer.java#L124

mkrn commented 3 years ago

I used this adaptive bitrate algorithm in the past and it worked really well, and should avoid the oldBitrate newBitrate comparison and problems @marcin-adamczewski mentioned, based on comparing actual bytes sent vs bitrate.

During streaming on timer check delta of TrafficStats.getTotalTxBytes() - it's supposed to be per app, meaning it would be actual bytes sent by app per last second.
Compare the delta with current bitrate setting,
If it is below the bitrate for 2-3 consecutive seconds, then trigger insufficientBandwidth routine
If it more than or equals current bitrate for 2-3 seconds then trigger sufficientBandwidth
In insufficientBandwidth - reduce bitrate in half OR reduce by 512kbps up to minimum such as 100kbps
In sufficientBandwidth - (If fps is more than half of target) set bitrate = bitrate + 128kbps up to maximum of target bitrate

Please let me know if it would work

pedroSG94 commented 3 years ago

Hello @mkrn, Actually I'm doing a similar way that you suggest but increase and decrease bitrate is different and I change it every second instead of every 2 or 3 seconds. I don't think that the problem is the number of increase or decrease in bitrate I think that the problem is cache stored when you have internet problems. For example: 1 - Start stream with connection of 10Mbs and you set bitrate to 5Mbs. No problem here for now so all is working fine. 2 - After few minutes you have connection problems and your connection downgrade to 1Mbs that means that you can't send 5Mbs so you need to decrease bitrate. 3 - Before bitrate reduction is executed you are still generating frames with 5Mbs because it is not detected yet (you need wait 1s to detect it). For this reason, that frames are stored in the queue and this will need more time to send because you can't send it on real time.

This effect produce stream issues until that frames of 5Mbs are send and incomming frames of 1Mbs(bitrate reduced) is descarded because you can't send more frames to the queue (queue is full because you have 5Mbs frames stored on it). After all frames of 5Mbs are send, you can continue stream as expected with 1Mbs but you lost frames.

The question here is. What should we do with that 5Mbs frames stored in queue? Clear the queue and lose that frames? I tried this before but the result is really bad. This is symple to implement. Clear queue here: https://github.com/pedroSG94/rtmp-rtsp-stream-client-java/blob/master/rtmp/src/main/java/net/ossrs/rtmp/SrsFlvMuxer.java#L1045 Like this:

mFlvVideoTagCache.clear();

About [TrafficStats.getTotalTxBytes()](https://developer.android.com/reference/android/net/TrafficStats#getTotalTxBytes()) according with official documentation: Return number of bytes transmitted since device boot. Counts packets across all network interfaces. It seems to be of all apps, anyway, get internet connection using bytes send by socket is really close to the real measure so I don't think that I should change it. I think the problem is the reduction algorithm and how to handle with buffer queue.

mkrn commented 3 years ago

Thank you for explaining the queue I think I will experiment with reducing the cache size, to mitigate it.

Is https://developer.android.com/reference/android/net/TrafficStats#getUidTxBytes(int) getUidTxBytes the right function as it's per UID?

Also, have you considered using BITRATE_MODE_CBR (Constant bitrate mode) to make encoder bitrate more constant and not spike for movement causing the issues?

mkrn commented 3 years ago

I'm trying different algorithms but really can't think of a good one because: Actual Bitrate is all over the place, you set one bitrate, encoder can produce different bitrate, and since we're counting length of frames sent, the queue is also varying the bitrate.

Do you think it's better count lost frames and if there are > certain amount per period of time, reduce bitrate, and if the framerate is good, then increase until maximum?

pedroSG94 commented 3 years ago

Thank you for explaining the queue I think I will experiment with reducing the cache size, to mitigate it.

Is https://developer.android.com/reference/android/net/TrafficStats#getUidTxBytes(int) getUidTxBytes the right function as it's per UID?

Also, have you considered using BITRATE_MODE_CBR (Constant bitrate mode) to make encoder bitrate more constant and not spike for movement causing the issues?

I need investigate about that method to calculate traffic but actual method is working fine so I think that it has not sense to implement it. About CBR mode. You are right, I considered it before but not all device support it for this reason I let it by default by set it if supported is a good idea.

Do you think it's better count lost frames and if there are > certain amount per period of time, reduce bitrate, and if the framerate is good, then increase until maximum?

It is not a good idea. You need wait until queue is full to know if you have internet problem and normally this is slower than calculate it using bitrate. Also the problem of how to handle with buffers in the queue is not solved. I think that discard all frames except keyframes until the queue is half full could be a good solution.

marcin-adamczewski commented 3 years ago

Basically, I didn't have luck with the current implementation so I had to adjust it to my needs. I'll describe it below as it may be helpful for others. There were 2 biggest problems I faced:

Measuring the upload speed the way it is now, gives very often false results because of the socket buffer. So on my device, it was 10Mb buffer (I believe it is a device-specific value). So when your encoder produces 3Mb/s and sends it through the socket it actually saves all 3Mb of data to the buffer first. So it may calculate your upload speed as 3Mb/s but in fact, it may be 1Mb/s. To resolve the issue I had to set socket.sendSendBufferSize to some lower value like 1KB. Keep in mind you have to do this before connecting to the socket, otherwise, it won't have any effect.
Periodical increase of bitrate always caused some lags. There is some logic in the current implementation that increases the bitrate by 10% if everything is fine. It always caused that at some moment the bitrate was too high for the upload speed and there was a lag for 2 seconds until the bitrate has been reduced. Because of this, I had to change the algorithm, which looks more or less like this:
- define the minimum and maximum bitrate (in my case 1Mb/s-3Mb/s)
- start a global BitrateEstimator that constantly measures current upload speed
- if the video frames buffer size is greater than 20% (the one from SrsFlvMuxer) there is probably congestion. Run a new BitrateEstimator for 6 seconds to measure current upload speed, but immediately set a new bitrate using the currently estimated bitrate from the global estimator for a fast recover. After 6 seconds you'll have a new estimated upload speed.
- when either stream has connected or network changed (wifi <-> cellular), run new estimator for 6 seconds, starting with some initial bitrate (each network type has its own initial bitrate)

The drawback is that it can only increase the bitrate on network switch or reconnection. But in my case, it's good enough.

mkrn commented 3 years ago

Thanks @pedroSG94 @marcin-adamczewski

Do you think the adaptive bitrate algorithm could be done based solely on the queue size of the SrsFlvMuxer?

What's the variance of actual bitrate produced by encoder in your experience? When I set bitrate using setBitrateOnFly` to a value like 5.5 Mbps the calculateBitrate returns values that range from 2.2 to 8.7 Mbps.

pedroSG94 commented 3 years ago

Hello,

I did 2 commit for it.

One to reduce socket buffer and get better bitrate calculation: https://github.com/pedroSG94/rtmp-rtsp-stream-client-java/commit/bee8acb9f7af84eb54666596ac1af934f523dddb
Another one to calculate adaptBitrate using video queue state like suggested by @marcin-adamczewski (maybe I didn't understand your explanation): https://github.com/pedroSG94/rtmp-rtsp-stream-client-java/commit/938bcb1f9034c11dfb7d2456fd399219a44555c2

This last commit can be used like this:

//In documentation used in Wiki.
//Replace
if (bitrateAdapter != null) bitrateAdapter.adaptBitrate(bitrate);
//To
if (bitrateAdapter != null) bitrateAdapter.adaptBitrate(bitrate, rtmpCamera2.hasCongestion());

Also in my last commit you can modify increment and decrement as you want: https://github.com/pedroSG94/rtmp-rtsp-stream-client-java/commit/938bcb1f9034c11dfb7d2456fd399219a44555c2#diff-fc3d1a441cee26b274586a4c1002a8792a41edab85010b2d9f6cca3420a99ab4R96 https://github.com/pedroSG94/rtmp-rtsp-stream-client-java/commit/938bcb1f9034c11dfb7d2456fd399219a44555c2#diff-fc3d1a441cee26b274586a4c1002a8792a41edab85010b2d9f6cca3420a99ab4R111

mkrn commented 3 years ago

Thanks @pedroSG94

We've tested that and it seems like on some devices it works as intended, and the queue has 1-2 frames in it always while on some devices the queue is constantly full from the very beginning, as a result hasCongestion always true and the bitrate is extremely low (one is galaxy A20S)...

What could this be related to?

marcin-adamczewski commented 3 years ago

@mkrn

Do you think the adaptive bitrate algorithm could be done based solely on the queue size of the SrsFlvMuxer?

If you want to only reduce bitrate I believe it's enough to use this queue size. As far I remember the queue by default can store 30 video frames which is 1 second for 30fps video. If this queue starts filling up, say it's 20% filled then you most probably have congestion and can reduce the bitrate. The problem is how to increase the bitrate. You could do that for example when the buffer has 3 or less frames but the amount you increase the bitrate may be too high, resulting in a lag.

What's the variance of actual bitrate produced by encoder in your experience? When I set bitrate using setBitrateOnFly` to a value like 5.5 Mbps the calculateBitrate returns values that range from 2.2 to 8.7 Mbps.

I guess there was no big variance after reducing the buffer size of the socket, I'd need to test it though.

marcin-adamczewski commented 3 years ago

@pedroSG94 I believe that you'd have to call the setSendBufferSize method before calling socket.connect(). I had to do this way, otherwise, it had no effect.

marcin-adamczewski commented 3 years ago

@mkrn I always have congestion at the beginning. I'm not sure 100% of the cause but it may be the TCP slow-start mechanism. Basically at the beginning of TCP connection, the speed is very slow and increases with time. I guess it's a matter of 1-2 seconds to get full speed (not sure). That's why the buffer may be full at the beginning.

I don't want to confuse you too much but if the current solution won't be enough for you then you could check out my implementation https://github.com/marcin-adamczewski/rtmp-rtsp-stream-client-java/blob/master/app/src/main/java/com/pedro/rtpstreamer/openglexample/OpenGlRtmpActivity.java The class name is BitrateAdjuster And as I said it won't increase the bitrate dynamically but only on a new connection or when Internet type switches. It also hasn't been released to users yet so it's not tested by life :D

pedroSG94 commented 3 years ago

@pedroSG94 I believe that you'd have to call the setSendBufferSize method before calling socket.connect(). I had to do this way, otherwise, it had no effect.

I was doing tests and it was working not matter when it is called. Could this depend of device used? I was testing with this method: https://docs.oracle.com/javase/7/docs/api/java/net/Socket.html#getSendBufferSize() If I call setSendBufferSize = 1 that method return 4096 no matter when it is called and If I never call setSendBufferSize then return about 10000

For this reason, since I can't call it before connection using SSL socket (because that method return a connected socket) I'm doing it after connect in both cases.

marcin-adamczewski commented 3 years ago

@pedroSG94 I've checked it and you're right. It doesn't matter if you call it before or after connecting. I probably confused it with the setReadBufferSize() method which requires setting it before the connect() in some cases.

BTW in my case, the getSendBufferSize method always returns twice a bigger size buffer than set by setSendBufferSize.

Maybe it's not important but it may be risky to set the buffer to 1 as there is actually no buffer. I guess it's there for a reason to increase writing speed, that's why I've set it to 10 * 1024 (10kB). It didn't impact the bitrate calculations in my case compared to the default value of the buffer which in my case is 10MB.

mkrn commented 3 years ago

@pedroSG94 @marcin-adamczewski we found setBufferSize(1) didn't work on some devices.

socket.setSendBufferSize(2*1024*1024); works reliably on variety of devices.

marcin-adamczewski commented 3 years ago

@mkrn 2*1024*1024 is 2MB which is 16Mb. I guess it's gonna definitely impact the upload speed calculations. The bitrate is usually ~1-3Mb/s so this buffer can contain 5-16 seconds of a stream.

pedroSG94 commented 3 years ago

@pedroSG94 @marcin-adamczewski we found setBufferSize(1) didn't work on some devices.

It is doing a crash or what do you mean?

Tecnically, if you set it to 1, that method delegate to kernel and select the valid value closer to your value selected. For this reason I got 4096 instead of 1.

mkrn commented 3 years ago

@marcin-adamczewski Actually I end up not relying on upload speed calculation at all in adaptive bitrate. What we do now is - if there is congestion (i.e. queue is 15% full) then we immediately drop the target bitrate by set amount, then re-evaluate every 2s, if there's no congestion raise by lower amount. Basically adding and subtracting to the same target, instead of manipulating the value received from upload speed calculation.

marcin-adamczewski commented 3 years ago

@pedroSG94 I've noticed that reducing the send buffer size impacts the streaming performance of some devices. I set the buffer size to 10kB and it reduced bandwidth by ~60% on my Nexus 5x. Our QA reported such an issue on Samsung S8 I guess. Setting the buffer size to 50kB resolved the issue but who knows how it's gonna behave on other devices.

Here's a very interesting answer on SO that explains why there may be congestion when the buffer is too small compared to bandwidth and latency https://stackoverflow.com/a/41747545/8065223 Based on that I made some calculations assuming a fast network (100Mb/s) and quite a big round trip time of 50ms. bufferSize = 100Mb/s * 50ms = 100Mb/s * 0.05s = 5Mb/s = 5000kb/s = 625kB/s We probably don't need to utilize a full bandwidth of 100Mb/s for 3Mb/s bitrate video, but if we'd like to fully utilize 10Mb/s speed we'd still need 62KB in the buffer. Anyways as I don't want to risk congestion I decided to use TrafficStats class for now.

pedroSG94 commented 3 years ago

@mkrn and @marcin-adamczewski Thank you for all test and provide this info. If this could produce problems in somes devices. I think that I will delete setBufferSize assuming that default value will provide a fair performance in all devices.

Nodalailama commented 3 years ago

Hi,

I am facing the same objective that to offer my customers the best possible experience. Bref.

By reading this thread, thanks for all to share code, reflexions et opinions ... if the problem, in the end, is to detect when the buffer is to heavy because the bandwidth comes slow but haven't yet downgrade the bitrate, alors it will be good to watch the evolution of the buffer.

I am not a great mathematician ... but in my opinion there is a little window there: use a algorythm, maybe with a little IA, that monitor the evolution of the size of the buffer to "anticipate" a bandwidth problem and downgrade the bitrate before it really need it.

I think, user that broadcast has 75% always the same bandwidth environnement so ia is to get custom action for each user.

My idea is that the buffer has a normal "saturation" when everything go normally and when it start to not act normally it's a sign that it may gonna have problem.

Voilà tout.

mkrn commented 2 years ago

I took a deep dive into the adaptive bitrate in this library (latest version, 2.1.4), and I've been able to fix most of the adaptive bitrate problems. Here's what I found:

Problems with default BitrateAdapter

Average was not properly calculated
Bandwidth in the last second (onNewBitrateRtmp) is calculated as the total data size send to the socket, with packet headers and audio packets. Yet bitrate is set for video encoder only (audio bitrate set separately).
If there is congestion, need to immediately reduce bitrate, to minimize drop frames and even worse congestion
Keep the cache size (queue length) to the minimum, so congestion can be quickly discovered (it's 20% of the cache size), and so the backlog of old frames is easier to push out. I suggest 30.
It is necessary to account for what was in the queue for bandwidth calculation before raising bitrate, because of the bitrate adjustments. Wait for a few cycles after lowering to raise again.

Notice: All of this makes sense with Constant Bitrate only! Here's my version of BitrateAdapter that accounts for audio bitrate, packet overhead, and reacts faster to congestion.

package pro.eventlive;

import android.util.Log;

public class BitrateAdapter {

  public interface Listener {
    void onBitrateAdapted(int bitrate);
  }

  private int maxBitrate;
  private int minBitrate = 100 * 1024;
  private int audioBitrate = 96 * 1000; // TODO: This should be passed into the class
  private int oldBitrate;
  private int averageBitrate;
  private int cont;
  private int cyclesSinceReduced = 0;
  private Listener listener;
  private float decreaseRange = 0.8f; //20%
  private float increaseRange = 1.2f; //20%

  private int decreaseBy = 256 * 1024; 
  private int increaseBy = 128 * 1024; 
  private int packetOverhead = 15 * 1024; // Magic number

  public BitrateAdapter(Listener listener) {
    this.listener = listener;
    reset();
  }

  public void setMaxBitrate(int bitrate) {
    this.maxBitrate = bitrate;
    this.oldBitrate = bitrate;
    reset();
  }

  public void adaptBitrate(long actualBitrate) {
    averageBitrate += actualBitrate;
    if (cont > 0) {
        averageBitrate /= 2;
    }
    cont++;
    if (cont >= 3) { // lowered the measurement interval from 5s to 3s
      if (listener != null && maxBitrate != 0) {
        listener.onBitrateAdapted(getBitrateAdapted(averageBitrate));
        reset();
      }
    }
  }

  /**
   * Adapt bitrate on fly based on queue state.
   */
  public void adaptBitrate(long actualBitrate, boolean hasCongestion) {

    averageBitrate += actualBitrate;
    if (cont > 0) {
        averageBitrate /= 2;
    }
    cont++;

    if (hasCongestion) {
        // Immediately react with adjustments
        listener.onBitrateAdapted(getBitrateAdapted(averageBitrate, hasCongestion));
        reset();
    }

    if (cont >= 3) { // lowered the measurement interval from 5s to 3s
      if (listener != null && maxBitrate != 0) {
        cyclesSinceReduced++;
        listener.onBitrateAdapted(getBitrateAdapted(averageBitrate, hasCongestion));
        reset();
      }
    }
  }

  // Version that doesn't take the queue size into account
  private int getBitrateAdapted(int averageBw) { 
    if (averageBw >= maxBitrate) { //You have high speed and max bitrate. Keep max speed
      oldBitrate = maxBitrate;
    } else if (averageBw <= oldBitrate * 0.9f) { //You have low speed and bitrate too high. Reduce bitrate
      oldBitrate = Math.max(averageBw - audioBitrate - decreaseBy, minBitrate);
    } else if (averageBw >= oldBitrate) { //You have high speed and bitrate too low. Increase bitrate 
      oldBitrate = Math.min(oldBitrate + increaseBy, maxBitrate);
    }
    // keep it otherwise
    return oldBitrate;
  }

  private int getBitrateAdapted(int averageBw, boolean hasCongestion) { 
    // what we expect should have been sent over the network
    int expectedBandwidth = oldBitrate + audioBitrate + packetOverhead;
    // Explicitly has congestion in the queue or average bandwidth 90% less than expected
    if (hasCongestion || (averageBw <= expectedBandwidth * 0.9f)) {
        // Reduce! decreaseBy is added for when there's congestion but bitrate - audioBitrate is within current bitrate
        oldBitrate = Math.max(averageBw - audioBitrate - decreaseBy, minBitrate);
        cyclesSinceReduced = 0; // wait a few cycles to let the higher bitrate frames in the queue pass through
    } else if (averageBw >= expectedBandwidth&& cyclesSinceReduced >= 3) { 
        // When fully recovered, attempt to increase by a bit
        oldBitrate = Math.min(oldBitrate + increaseBy, maxBitrate);
    }

    // keep it otherwise
    return oldBitrate;
  }

  public void reset() {
    averageBitrate = 0;
    cont = 0;
  }

  public float getDecreaseRange() {
    return decreaseRange;
  }

  /**
   * @param decreaseRange in percent. How many bitrate will be reduced based on oldBitrate.
   * valid values:
   * 0 to 100 not included
   */
  public void setDecreaseRange(float decreaseRange) {
    if (decreaseRange > 0f && decreaseRange < 100f) {
      this.decreaseRange = 1f - (decreaseRange / 100f);
    }
  }

  public float getIncreaseRange() {
    return increaseRange;
  }

  /**
   * @param increaseRange in percent. How many bitrate will be increment based on oldBitrate.
   * valid values:
   * 0 to 100
   */
  public void setIncreaseRange(float increaseRange) {
    if (increaseRange > 0f && increaseRange < 100f) {
      this.increaseRange = 1f + (increaseRange / 100f);
    }
  }
}

Suggestions to improve further:

If the queue is congested and not sending the frames, then calculateBitrate will not be invoked and onNewBitrateRtmp will not be called and bitrate would not be adjusted. Instead, I suggest using a timer or callback for congestion.
When the queue is full we should not discard audio packets because they're small compared to video. Ideally, we should not discard Keyframes because without them the picture falls apart. How to do it? Two separate queues? Or queue clean up? It is better to miss some video frames than audio - because you will hear pauses in the audio but may not notice low frame rate.

mkrn commented 2 years ago

@pedroSG94 I can make a PR with the BitrateAdapter improvements, just need your input on some things:

Should we pass Audio bitrate into it?
Can we make separate audio and video queues?
Can we discard regular video frames before keyframes?
Can onNewBitrateRtmp be called exactly every second, instead of whenever the time elapsed is over 1 second?

pedroSG94 commented 2 years ago

Hello,

@pedroSG94 I can make a PR with the BitrateAdapter improvements, just need your input on some things:

Should we pass Audio bitrate into it?

Can we make separate audio and video queues?

Can we discard regular video frames before keyframes?

Can onNewBitrateRtmp be called exactly every second, instead of whenever the time elapsed is over 1 second?

1 - In you mean into onNewBitrateRtmp. I think it is necessary because that callback should indicate total bitrate of stream and if you do an only audio stream the callback will be useless. Maybe modify the callback using 2 variables into that (one for audio and one for video) is a good solution.

2 - Yes, it is possible and easy.

3 - I think it is possible but maybe a bit dfficult in RTSP module I will check it. I will think in a way to make it in RTSP module. The question is which frame should I discard, the last frame? If you are doing an stream with all keyframes (it is possible but normally not used for the bandwidth reason) the cost of this operation could be high because I need check all the queue.

4 - onNewBitrateRtmp is called each time a frame is send to socket (the time check). I will need create other thread for it and thread sync could produce lose frames (lose the size of a frame reducing the real value). I think that the time difference in that callback is not relevant because the time should be really close to 1s.

I will create a branch to handle this

mkrn commented 2 years ago

1) I meant passing audioBitrate to BitrateAdapter.
onNewBitrateRtmp reporting bandwidth for last second is good. That way you can display it in the app as your upload rate. In BitrateAdapter it's important to know the audio bitrate to calculate the video bitrate... i.e. videoBitrate = Bandwidth - audioBitrate - overhead

pedroSG94 commented 2 years ago

This is the branch for this: https://github.com/pedroSG94/rtmp-rtsp-stream-client-java/tree/adaptativebitrate

For now I only did the 1. I will develop 2 and 3 tomorrow

pedroSG94 commented 2 years ago

I updated branch with number 3. About number 2 this is producing a big performance problem and sync both queues to avoid unnecessary wait time it is not working fine. You only need this because you want check remaining capacity in video queue, right? For now, test with point 1 and 3 and I will think other way to do it

pedroSG94 / RootEncoder

Adaptive bitrate doesn't work #545