jbuf frame completeness

[cspiel1]

jbuf: replace adaptive mode by frame completeness check

• min/max specifies number of frames to keep in buffer
• jbuf_get() does not deliver un-complete frames or out-of-order packets as long as max is not reached
• jbuf_put() keeps track of the complete sequence of packets (see field end)

end always points to the end of the complete packet sequence. Thus between the head and end no packet is missing.

The implementation works for audio and video.

Keeping track of end, the number of frames nf and the number of complete frames ncf does not increase the O(.)-complexity of jbuf_put() or jbuf_get().

TODOs:

• handle marker bit
• plots to visualize algorithm with/without jitter simulation
• unit tests

[cspiel1]

The plots will heavily depend on this PR. So currently I am not sure if we can/should separate it.

[sreimers]

The plots will heavily depend on this PR. So currently I am not sure if we can/should separate it.

We should have at least a before and after comparison. We can simply use this one for main: f63865f

[cspiel1]

We should have at least a before and after comparison. We can simply use this one for main: f63865f

okay. Do you want to create the PR for your branch?
Then we need also a tools PR for baresip. baresip/baresip#2733

Edit: I could create both PR re/baresip for the plots.

• jbuf: trace data for plot #964

[cspiel1]

Should we enable -DUSE_TRACE=ON -DCMAKE_C_FLAGS="-DRE_JBUF_TRACE" in github actions?

[cspiel1]

Rebased, but it won't build so far. The plot data needs to be updated. I guess that github actions does not build the plot code.

[sreimers]

the builds are failing because of this test line:

test jbuf: TEST_ERR: /home/runner/work/re/re/test/jbuf.c:56: (No such file or directory [2])

Should we enable -DUSE_TRACE=ON -DCMAKE_C_FLAGS="-DRE_JBUF_TRACE" in github actions?

I think we should leave these are only for manual debug test builds and should not be needed (like modules not build with higher debug info).

[cspiel1]

This commit af375ad is to discuss trace module. It supports only one trace. If jbuf is enabled for audio + video, then the trace fails and we get mem leaks. This commit is minimal change to fix this.

Better would be to extend trace module to support multiple traces. This could be used to generate

• jbuf-audio.json
• jbuf-video.json

[sreimers]

I think trace should handled different.

There should only one single re_trace.json handled by libre_init and libre_close and controlled by USE_TRACE. Different modules or applications can use different category.

#965

[cspiel1]

A first plot for video with

audio_jitter_buffer_type off 
video_jitter_buffer_type fixed
video_jitter_buffer_delay 2-10

We can observe that there is one lost packet. The jbuf is filled until 10 frames are reached, then the un-complete frame passes.

[sreimers]

We can observe that there is one lost packet. The jbuf is filled until 10 frames are reached, then the un-complete frame passes.

Looks like max frames is very quickly decreased/consumed by decoder after this. This should be smoothed I think, since it causes:

• a high cpu spike by decoder
• visible video speedup

[cspiel1]

Currently we have

• audio

rtp -> jbuf -> decoder -> aubuf -> auplay
| main thread               | play thread  |

• video

rtp -> jbuf -> decoder -> display
| main thread                        |

A vrx_thread could be an improvement. jbuf should be thread safe already. I'll try a timer instead.

We should also think about the audio pipeline. There are jbuf and aubuf: The latter does jitter smoothing currently.

[cspiel1]

• one packet loss
  

• with jitter and packet losses
  

[sreimers]

Does this fixed 250ms value works well with different fps and min/max jbuf combinations?

[cspiel1]

Not sure. Anyway, while analyzing audio streams with jitter simulation I saw that we need the computation of moving average rdiff to reliable decide if the buffer may be reduced again and how much it can be reduced.

I'll replace the last two commits.

[cspiel1]

rdiff didn't solve problem for audio. It was only 2 packets while number of complete frames need to be higher to avoid underruns in following aubuf.

[cspiel1]

Shrink buffer after last out-of-order packet. Here plots for video

[cspiel1]

1. rise buffer if packets missing
   @sreimers as you can see if a packet is lost the buffer immediately jumps to the configured max (here 10 frames). This leads to a small break with frozen frame. E.g. for 20fps and max=10 results in 500ms break with frozen frame. If I understood you correctly this is wanted behavior.

Alternative: Beside the overall frame max set another boundary for frame freezing time. E.g. 2 frames. Then pass the next frame which is still incomplete in the hope that the decoder is able to decode this with some artifacts in the display.

2. shrinking buffer
   The current solution with tmr, the wait and again flags work and reduce the buffer slowly starting 1 second after the last out-of-order event occurred. After the first lost packet the buffer will stay at max frames as long as out-of-order packets are detected which is not ideal. Better would be to reduce the buffer also after a lost frame slowly but while out-of-order still occurs.
   This leads to the question: Why only after a lost frame? What if max rises only to max-1?

Next try: Periodically, e.g. every second the number of complete frames ncf is reduced by passing two frames to the decoder. Until ncf==0 is reached. So no other condition like "lost frame" or "out-of-order packet" has to met.

[cspiel1]

Last mentioned method works better. Have to add one commit.

x means reset_wait() was called.
y means jb->again was true in line 617.

[cspiel1]

Of course yesterday tried already without the line here removed 9a6c8c5 . But I had bad results because had both jbuf active (for audio and video) and thus the data was mixed which confused me.

[cspiel1]

Analysing results for audio stream

audio_jitter_buffer_type fixed
audio_jitter_buffer_delay 1-10
video_jitter_buffer_type off

audio_buffer        	20-300     	# ms
audio_buffer_mode	adaptive

• jbuf plot: "waiting" events may lead to real aubuf underruns
• aubuf ajb plot: There are too many underruns. We can see that the jitter is still detected but not as fast and high we would expect it. After the jitter simulation is active for ~4 seconds the underruns in aubuf seems to correlate with the "waiting" events.

[sreimers]

Why is this dropped? It's useful for unit testing and this ensures frame counting works correct.

[cspiel1]

okay, I'll add it.

[cspiel1]

The underruns correlate also with the "lost" packets.
Here the plots without packet loss (except one at the end):

Edit:
Above was no delay and 100ms jitter:

tc qdisc add dev ifb1 root netem delay 0ms 100ms

Now with 100ms delay and 50ms jitter:

tc qdisc add dev ifb1 root netem delay 100ms 50ms

Edit:
The lots of underruns are not acceptable. Have to be studied further.

[cspiel1]

Here #963 (comment) is an open question (1.) about "lost packets".

Edit:
Except of the "lost packets" question mentioned, I think the jbuf algorithm works now like expected. The underruns in aubuf can't be solved here. Maybe in another PR we could move the ajb back into jbuf and avoid a third "decoding thread" (we also already tried a decoding thread) by means of a timer running in the main thread (later in RTP rx thread).

I set this PR to "ready for review" and fix the unit tests.

[cspiel1]

Should we move jbuf to baresip like @alfredh suggests?
#962 (comment)

Again was thinking about the current solution in this PR. The result of jbuf_get() depends on timing. The shrink timing maybe should be moved to baresip/application. Also the unit tests now look strange now because of the "waiting" state.
--> Put back to draft now to prevent merge.

Also I'll try with our fixed rdiff implementation if the audio underruns are similar.

[cspiel1]

Compared with this results: #963 (comment)

tc qdisc add dev ifb1 root netem delay 0ms 100ms

audio_jitter_buffer_type adaptive
audio_jitter_buffer_delay 1-10
video_jitter_buffer_type off

audio_buffer        	20-300     	# ms
audio_buffer_mode	adaptive

This are less underruns.

[sreimers]

I will prepare some pcap based tests with SIPp (maybe within a extra tools repository), so it's easier to reproduce, see improvement and we can add real jitter examples (like WiFi/LTE). With editcap we can manipulate order/timer/lost and simulate edge cases. Maybe I add another experiment which is clockrate/ptime based instead of frame counting and we can compare the solutions we currently have.

1. rise buffer if packets missing
   @sreimers as you can see if a packet is lost the buffer immediately jumps to the configured max (here 10 frames). This leads to a small break with frozen frame. E.g. for 20fps and max=10 results in 500ms break with frozen frame. If I understood you correctly this is wanted behavior.

Unsure, will think about this, is this real packet lost or because of late lost? We should keep delay as small as possible (based on network condition).

Should we move jbuf to baresip like @alfredh suggests?
#962 (comment)

Yes it would make testing easier for now.

[cspiel1]

Great ideas for testing!

is this real packet lost or because of late lost?

This makes no difference. The question is if it makes sense not to jump to max in one step. Instead:
Beside the overall frame max set another boundary for frame freezing time. E.g. 2 frames. Then pass the next frame which is still incomplete in the hope that the decoder is able to decode this with some artifacts in the display.
This would mean that the latency only increases two frames for a lost or late packet. If there follow more lost or late packets the latency again increases.

Okay, then I prepare a PR for moving jbuf from re to baresip. This could be merged with low risk to current release.

[cspiel1]

move jbuf to baresip:

• jbuf: move jbuf to baresip #971
• jbuf: move from re to baresip baresip#2743

[cspiel1]

Applied the changes to a baresip branch based on baresip#2743
https://github.com/cspiel1/baresip/tree/jbuf_frame_completeness2