Reduce mutex contention and increase socket buffers#50
Conversation
- Move ALAC decode outside ab_mutex in buffer_put_packet to reduce contention with HTTP serving thread - Increase SO_SNDBUF from 128KB to 256KB on listener socket - Apply SO_SNDBUF 256KB on accepted HTTP socket (was missing)
Inspired by swyh-rs streaming architecture: - Add TransferMode.dlna.org: Streaming header to HTTP responses, telling DLNA renderers to use streaming transfer mode - Change Server header from HairTunes to AirConnect - Replace pure-zero silence frames with faint noise (~2 LSB white noise) to prevent FLAC decoder stalls on some renderers
|
Added a second commit with improvements inspired by swyh-rs:
|
|
|
||
| /*---------------------------------------------------------------------------*/ | ||
| static void buffer_put_packet(raopst_t* ctx, seq_t seqno, unsigned rtptime, bool first, char* data, int len) { | ||
| // decode ALAC outside mutex to reduce contention with HTTP thread |
There was a problem hiding this comment.
I understand the point but I have to check that I'm not using that on some lower-end CPU where memory copy is not cheap
There was a problem hiding this comment.
Have you noticed that being an issue IRL?
| } | ||
|
|
||
| kd_add(resp, "Server", "HairTunes"); | ||
| kd_add(resp, "Server", "AirConnect"); |
There was a problem hiding this comment.
No, this lib is not specific to AirConnect
| kd_add(resp, "Server", "HairTunes"); | ||
| kd_add(resp, "Server", "AirConnect"); | ||
| kd_add(resp, "Content-Type", encoder_mimetype(ctx->encoder)); | ||
| kd_add(resp, "TransferMode.dlna.org", "Streaming"); |
There was a problem hiding this comment.
You are challenging my memory here ... I would have to re-read UPnP specs to see is this is correct. As you probably know, UPnP is a consistency disaster so I'm always worried to break something
There was a problem hiding this comment.
Oh, you can't do that here. This is a generic HTTP request that works for Chromecast and UPnP. You can't have UPnP headers.
There was a problem hiding this comment.
I have a callback for that (sorry, but it just came back to me, I have not modified this code in a bit). So I'll do a modification in AirConnect
libraop is a shared library, not specific to AirConnect.
009d81c to
563c22a
Compare
| } while (ctx->http_listener < 0 && port.count < port_range); | ||
|
|
||
| int i = 128*1024; | ||
| int i = 256*1024; |
There was a problem hiding this comment.
Is this an observed issue or is this "just in case"? Because I think 128k is plenty for audio
| if (!curframe->ready) { | ||
| LOG_DEBUG("[%p]: created zero frame at %d (W:%hu R:%hu)", ctx, now - playtime, ctx->ab_write, ctx->ab_read); | ||
| memset(curframe->data, 0, ctx->frame_size * 4); | ||
| // inject faint noise (~2 LSB) instead of pure silence to prevent FLAC decoder stalls |
There was a problem hiding this comment.
I've never heard about "flac stalls" issue. Can you elaborate?
Summary
alac_decode()call beforepthread_mutex_lock(&ctx->ab_mutex)inbuffer_put_packet. Decode into a VLA stack buffer, thenmemcpyunder the lock. This reduces contention with the HTTP serving thread that also holdsab_mutex.SO_SNDBUFfrom 128KB to 256KB on the listener socket. Also applySO_SNDBUFto the accepted HTTP socket inhttp_thread_func(was only set on the listener, not the accepted connection).Test plan
ctx->frame_size * 2(frame_size int16_t samples × 2 channels)Related: philippe44/AirConnect#582