Configurable limits - Refactor current options

[nolan-veed]

Why

For #610

This is the first part to refactor existing options.

What

• Wrote parse_size().
• Renamed options and used parse_size() in roc_send, roc_recv.
• Deduced max options if not given, from other options.

Testing

• Added tests for parse_size().

[nolan-veed]

@gavv Would like some early feedback.

[gavv]

Thanks for PR and kudos for kibibyte :)

[gavv]

Here are a few comments.

[github-actions]

🤖 The latest upstream change made this pull request unmergeable. Please resolve the merge conflicts.

[github-actions]

🤖 The latest upstream change made this pull request unmergeable. Please resolve the merge conflicts.

[gavv]

Hi, I'm preparing 0.3.0 release and have rebased develop on master (this workflow is described here).

Please reset develop in your fork to up-to-date version and rebase your PR on that. (You can use git rebase --onto or just cherry-pick your commits.)

[nolan-veed]

Please reset develop in your fork to up-to-date version and rebase your PR on that. (You can use git rebase --onto or just cherry-pick your commits.)

All good. Thanks.

[gavv]

Great, please ping me when it's ready for review.

[nolan-veed]

See what you think if my latest changes.

[gavv]

In current implementation roc-recv is broken:

$ roc-recv -vv -s rtp+rs8m://localhost:10001 -r rs8m://localhost:10002 -c rtcp://localhost:10003  
14:37:38.764 [135553] [dbg] roc_sndio: [pulseaudio_backend.cpp:21] pulseaudio backend: initializing
14:37:38.764 [135553] [dbg] roc_sndio: [sox_backend.cpp:164] sox backend: initializing
14:37:38.766 [135553] [dbg] roc_sndio: [backend_map.cpp:20] backend map: initialized: n_backends=2 n_drivers=82
14:37:38.766 [135553] [dbg] roc_core: [slab_pool_impl.cpp:61] pool: initializing: name=packet_pool object_size=704 min_slab=8B(1S) max_slab=0B(0S)
14:37:38.766 [135553] [dbg] roc_core: [slab_pool_impl.cpp:61] pool: initializing: name=buffer_pool object_size=2096 min_slab=8B(1S) max_slab=0B(0S)
14:37:38.766 [135553] [dbg] roc_core: [slab_pool_impl.cpp:61] pool: initializing: name=buffer_pool object_size=144 min_slab=8B(1S) max_slab=0B(0S)
14:37:38.766 [135553] [dbg] roc_core: [slab_pool_impl.cpp:61] pool: initializing: name=format_pool object_size=280 min_slab=4480B(16S) max_slab=0B(0S)
14:37:38.766 [135553] [dbg] roc_node: [context.cpp:24] context: initializing
14:37:38.766 [135553] [dbg] roc_sndio: [pulseaudio_device.cpp:75] pulseaudio sink: opening device: device=default
14:37:38.766 [135555] [dbg] roc_ctl: [control_task_queue.cpp:95] control task queue: starting event loop
14:37:38.766 [135554] [dbg] roc_netio: [network_loop.cpp:278] network loop: starting event loop
14:37:38.768 [135556] [inf] roc_sndio: [pulseaudio_device.cpp:507] pulseaudio sink: opening stream: device=(null) n_channels=2 sample_rate=44100
14:37:38.776 [135553] [dbg] roc_core: [slab_pool_impl.cpp:61] pool: initializing: name=slot_pool object_size=632 min_slab=8B(1S) max_slab=0B(0S)
14:37:38.776 [135553] [dbg] roc_node: [receiver.cpp:34] receiver node: initializing
14:37:38.776 [135553] [dbg] roc_node: [receiver.cpp:75] receiver node: configuring audiosrc interface of slot 0
14:37:38.776 [135553] [inf] roc_pipeline: [receiver_source.cpp:64] receiver source: adding slot
14:37:38.776 [135553] [dbg] roc_pipeline: [receiver_slot.cpp:36] receiver slot: initializing
14:37:38.776 [135553] [inf] roc_node: [receiver.cpp:123] receiver node: binding audiosrc interface of slot 0 to rtp+rs8m://lo:10001
14:37:38.777 [135553] [dbg] roc_pipeline: [receiver_slot.cpp:42] receiver slot: adding audiosrc endpoint rtp+rs8m
14:37:38.777 [135554] [dbg] roc_netio: [udp_receiver_port.cpp:116] udp receiver: <udprecv 0x7fea50001168 bind=127.0.0.1:10001>: opened port
14:37:38.777 [135553] [dbg] roc_node: [receiver.cpp:75] receiver node: configuring audiorpr interface of slot 0
14:37:38.777 [135553] [inf] roc_node: [receiver.cpp:123] receiver node: binding audiorpr interface of slot 0 to rs8m://lo:10002
14:37:38.777 [135553] [dbg] roc_pipeline: [receiver_slot.cpp:42] receiver slot: adding audiorpr endpoint rs8m
14:37:38.778 [135554] [dbg] roc_netio: [udp_receiver_port.cpp:116] udp receiver: <udprecv 0x7fea500014f8 bind=127.0.0.1:10002>: opened port
14:37:38.778 [135553] [dbg] roc_node: [receiver.cpp:75] receiver node: configuring audioctl interface of slot 0
14:37:38.778 [135553] [inf] roc_node: [receiver.cpp:123] receiver node: binding audioctl interface of slot 0 to rtcp://lo:10003
14:37:38.778 [135553] [dbg] roc_pipeline: [receiver_slot.cpp:42] receiver slot: adding audioctl endpoint rtcp
14:37:38.778 [135554] [dbg] roc_netio: [udp_receiver_port.cpp:116] udp receiver: <udprecv 0x7fea50001888 bind=127.0.0.1:10003>: opened port
14:37:38.778 [135553] [err] roc_sndio: [pump.cpp:37] pump: buffer size is too small: required=88 actual=24
14:37:38.778 [135553] [err] roc_recv: [main.cpp:517] can't create pump
14:37:38.778 [135553] [dbg] roc_node: [receiver.cpp:47] receiver node: deinitializing
14:37:38.778 [135554] [dbg] roc_netio: [network_loop.cpp:559] network loop: removing port <udprecv 0x7fea50001168 bind=127.0.0.1:10001>
14:37:38.778 [135554] [dbg] roc_netio: [udp_receiver_port.cpp:138] udp receiver: <udprecv 0x7fea50001168 bind=127.0.0.1:10001>: initiating asynchronous close
14:37:38.778 [135554] [dbg] roc_netio: [udp_receiver_port.cpp:166] udp receiver: <udprecv 0x7fea50001168 bind=127.0.0.1:10001>: closed port
14:37:38.778 [135554] [dbg] roc_netio: [network_loop.cpp:264] network loop: asynchronous close finished: port <udprecv 0x7fea50001168 bind=127.0.0.1:10001>
14:37:38.778 [135554] [dbg] roc_netio: [network_loop.cpp:559] network loop: removing port <udprecv 0x7fea500014f8 bind=127.0.0.1:10002>
14:37:38.778 [135554] [dbg] roc_netio: [udp_receiver_port.cpp:138] udp receiver: <udprecv 0x7fea500014f8 bind=127.0.0.1:10002>: initiating asynchronous close
14:37:38.778 [135554] [dbg] roc_netio: [udp_receiver_port.cpp:166] udp receiver: <udprecv 0x7fea500014f8 bind=127.0.0.1:10002>: closed port
14:37:38.778 [135554] [dbg] roc_netio: [network_loop.cpp:264] network loop: asynchronous close finished: port <udprecv 0x7fea500014f8 bind=127.0.0.1:10002>
14:37:38.778 [135554] [dbg] roc_netio: [network_loop.cpp:559] network loop: removing port <udprecv 0x7fea50001888 bind=127.0.0.1:10003>
14:37:38.778 [135554] [dbg] roc_netio: [udp_receiver_port.cpp:138] udp receiver: <udprecv 0x7fea50001888 bind=127.0.0.1:10003>: initiating asynchronous close
14:37:38.778 [135554] [dbg] roc_netio: [udp_receiver_port.cpp:166] udp receiver: <udprecv 0x7fea50001888 bind=127.0.0.1:10003>: closed port
14:37:38.778 [135554] [dbg] roc_netio: [network_loop.cpp:264] network loop: asynchronous close finished: port <udprecv 0x7fea50001888 bind=127.0.0.1:10003>
14:37:38.778 [135553] [inf] roc_pipeline: [receiver_source.cpp:81] receiver source: removing slot
14:37:38.778 [135553] [dbg] roc_pipeline: [receiver_session_group.cpp:268] session group: removing all sessions
14:37:38.778 [135553] [dbg] roc_sndio: [pulseaudio_device.cpp:62] pulseaudio sink: closing device
14:37:38.778 [135553] [dbg] roc_node: [context.cpp:28] context: deinitializing
14:37:38.778 [135555] [dbg] roc_ctl: [control_task_queue.cpp:105] control task queue: finishing event loop
14:37:38.778 [135554] [dbg] roc_netio: [network_loop.cpp:285] network loop: finishing event loop

[nolan-veed]

OK. Ready for another glance. Thanks for your guidance.

[gavv]

Thanks for update. I tested new version, deduction of max frame size on roc-recv works, but deduction of max packet size on roc-send doesn't.

Works:

roc-recv -vv -s rtp+rs8m://lo:10001 -r rs8m://lo:10002 -c rtcp://lo:10003 --frame-len 50ms

Doesn't work:

roc-send -vv -s rtp+rs8m://lo:10001 -r rs8m://lo:10002 -c rtcp://lo:10003 -i file:stash/loituma.wav --packet-len 50ms

13:28:01.347 [882198] [dbg] roc_rtp: [composer.cpp:67] rtp composer: not enough space for rtp payload: size=8820 cap=2036
13:28:01.348 [882198] [err] roc_audio: [packetizer.cpp:176] packetizer: can't prepare packet

I guess we forgot to take header(s) size into account.

For bare RTP, header is 12 bytes (see src/internal_modules/roc_rtp/headers.h). When FEC is enabled, it's 6 more bytes for additional FEC footer (see src/internal_modules/roc_fec/headers.h). When we add encryption, DTLS may add 13 more bytes, and SRTP may add 10 bytes, AFAIK. Also, some codecs may require aligning packets, currently they may add up to 7 bytes padding to the beginning.

12 + 6 + 13 + 7 = 38.

I think we could add, say, 64 bytes, to leave some extra space, just in case.

Another concern: we forgot to think about FEC (repair) packets, which should also fit into max packet size. Their size is proportional to the byte size of media packet, but generally we don't know the coefficient.

So our formula should be: max_packet_size = (C + n_samples * sample_size) * K. Here, C is 64, sample_size is 2, and K is currently 1, which works for media packets, but we should test if it also works for repair packets, and if not, increase it.

We should test that our formula works with different protocols (rtp://, rtp+rs8m://, and rtp+ldpc://), and with different packet lengths (1ms, 5ms, 10ms, 50ms, 100ms). You can find example how to use protocols here: https://roc-streaming.org/toolkit/docs/manuals/roc_send.html

[gavv]

Can you please reorder code in roc-send so that max_packet_size_given checks would be grouped together, like you did in roc-recv?

Like:

if (max_packet_size_given) {
    ...
} else {
    ...
}

It's much easier to follow the code when these "if" and "else" for every option are in one place.

(Sorry that I didn't notice it in previous reviews).

[github-actions]

🤖 The latest upstream change made this pull request unmergeable. Please resolve the merge conflicts.

[gavv]

Sorry for conflicts, I think it's caused by 9bd9888

[nolan-veed]

--packet-len 50ms

OK. I've made the extra 64 bytes change to roc_send and all the changes requested.

With roc_recv at 50ms, I can see roc_send work at 1ms, 5ms, 10ms - the audio comes through.

But, at 50ms, and 100ms - The audio does not come through, and I see this in roc_recv, which I don't understand yet.

20:00:00.642 [626581] [dbg] roc_netio: [udp_receiver_port.cpp:264] udp receiver: <udprecv 0x7f0bf8001168 bind=127.0.0.1:10001>: ignoring partial read: num=0 src=127.0.0.1:46694 dst=127.0.0.1:10001 nread=2048

I expected at least 50ms to work.

[gavv]

Oh, if you use big packet len on sender, you also need big max packet size on receiver.

(Until we implement rtsp, it needs manual configuration)

[nolan-veed]

Oh, if you use big packet len on sender, you also need big max packet size on receiver.

Oh sorry, I was confused. The 50ms roc_recv frame len is nothing to do with 50ms roc_send packet len.

OK. Got it to work. --max-packet-size 32K on roc_recv seems to have worked.

[gavv]

Thank you!

Two follow-up commits:

• 47326a9 - update manual pages
• ea2d2c5 - reorder code in roc_send in similar way as in roc_recv (first parse io_config, then sender_config, then context_config)

[gavv]

FYI: #654