Skip to content

Commit

Permalink
Revert "feat(server): add enable_datagrams, off by default"
Browse files Browse the repository at this point in the history
This reverts commit e1dc976.

Stream-based video transfer is really painfully bad in the face of either packet
loss or latency. What we really need is FEC.
  • Loading branch information
colinmarc committed May 4, 2024
1 parent a095994 commit e6bab6c
Show file tree
Hide file tree
Showing 3 changed files with 4 additions and 40 deletions.
3 changes: 0 additions & 3 deletions mm-server/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,6 @@ mod parsed {
pub(super) tls_key: Option<PathBuf>,
pub(super) worker_threads: Option<NonZeroU32>,
pub(super) max_connections: Option<MaxConnections>,
pub(super) enable_datagrams: Option<bool>,
}

#[derive(Debug, Clone, PartialEq, Deserialize, Converge)]
Expand Down Expand Up @@ -122,7 +121,6 @@ pub struct ServerConfig {
pub tls_key: Option<PathBuf>,
pub worker_threads: NonZeroU32,
pub max_connections: Option<NonZeroU32>,
pub enable_datagrams: bool,
}

#[derive(Debug, Clone, PartialEq)]
Expand Down Expand Up @@ -184,7 +182,6 @@ impl Config {
parsed::MaxConnections::Value(n) => Some(n),
parsed::MaxConnections::Infinity => None,
},
enable_datagrams: server.enable_datagrams.unwrap(),
},
apps: HashMap::new(), // Handled below.
bug_report_dir: None, // This is only set from the command line.
Expand Down
33 changes: 4 additions & 29 deletions mm-server/src/server/handlers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -259,8 +259,6 @@ fn attach(
}
};

let use_datagrams = state.lock().unwrap().cfg.server.enable_datagrams;

let (handle, display_params, bug_report_dir) = {
let mut state = state.lock().unwrap();

Expand Down Expand Up @@ -363,7 +361,6 @@ fn attach(
// Five varints at max 5 bytes, plus a header, works out to around 32
// bytes. Double for safety.
let dgram_chunk_size = max_dgram_len - 64;
let stream_chunk_size = protocol::MAX_MESSAGE_SIZE - 64;

loop {
select! {
Expand Down Expand Up @@ -545,13 +542,7 @@ fn attach(

// TODO FEC

let chunk_size = if use_datagrams {
dgram_chunk_size
} else {
stream_chunk_size
};

for (chunk, num_chunks, data) in iter_chunks(frame, chunk_size) {
for (chunk, num_chunks, data) in iter_chunks(frame, dgram_chunk_size) {
let msg = protocol::VideoChunk {
session_id,
attachment_id: handle.attachment_id,
Expand All @@ -564,11 +555,7 @@ fn attach(
timestamp: ts,
};

if use_datagrams {
outgoing_dgrams.send(msg.into()).ok();
} else {
outgoing.send(msg.into()).ok();
}
outgoing_dgrams.send(msg.into()).ok();
}
}
Ok(CompositorEvent::AudioFrame{ stream_seq, seq, ts, frame }) => {
Expand All @@ -580,13 +567,7 @@ fn attach(
last_audio_frame_recv = time::Instant::now();


let chunk_size = if use_datagrams {
dgram_chunk_size
} else {
stream_chunk_size
};

for (chunk, num_chunks, data) in iter_chunks(frame, chunk_size) {
for (chunk, num_chunks, data) in iter_chunks(frame, dgram_chunk_size) {
let msg = protocol::AudioChunk {
session_id,
attachment_id: handle.attachment_id,
Expand All @@ -599,14 +580,8 @@ fn attach(
timestamp: ts,
};

if use_datagrams {
outgoing_dgrams.send(msg.into()).ok();
} else {
outgoing.send(msg.into()).ok();
}
outgoing_dgrams.send(msg.into()).ok();
}


}
Ok(CompositorEvent::CursorUpdate{ image, icon, hotspot_x, hotspot_y }) => {
use protocol::update_cursor::CursorIcon;
Expand Down
8 changes: 0 additions & 8 deletions mmserver.default.toml
Original file line number Diff line number Diff line change
Expand Up @@ -49,14 +49,6 @@ worker_threads = 8
# to specify no limit.
max_connections = 4

# Whether to enable the use of QUIC datagrams (RFC 9221) to transmit audio and video
# packets. Datagrams offer lower latency but decreased robustness in the face of
# packet loss, so this option can be used to decrease latency if you're on a
# local network where you know there won't be any loss.
#
# This option will likely default to true in a future release.
enable_datagrams = false

# *** Configured Applications ***
#
# Each application you want to stream must be configured in advance. An example
Expand Down

0 comments on commit e6bab6c

Please sign in to comment.