Skip to content

Commit 68cc684

Browse files
committed
Fix datagram forwarding: use broadcast channel for proper queueing
The datagram serve layer was only keeping the latest datagram, causing most datagrams to be dropped when they arrive faster than the reader can consume them (e.g., 50/sec audio). Changed to use tokio broadcast channel (1024 buffer) so datagrams are queued and forwarded in order. Broadcast allows cloning the reader. Logs warning if reader lags behind.
1 parent 4e33675 commit 68cc684

1 file changed

Lines changed: 40 additions & 65 deletions

File tree

moq-transport/src/serve/datagram.rs

Lines changed: 40 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -1,123 +1,98 @@
11
use std::{fmt, sync::Arc};
22

3-
use crate::watch::State;
3+
use tokio::sync::broadcast;
44

55
use super::{ServeError, Track};
66

7+
const DATAGRAM_CHANNEL_SIZE: usize = 1024;
8+
79
pub struct Datagrams {
810
pub track: Arc<Track>,
911
}
1012

1113
impl Datagrams {
1214
pub fn produce(self) -> (DatagramsWriter, DatagramsReader) {
13-
let (writer, reader) = State::default().split();
15+
let (tx, rx) = broadcast::channel(DATAGRAM_CHANNEL_SIZE);
1416

15-
let writer = DatagramsWriter::new(writer, self.track.clone());
16-
let reader = DatagramsReader::new(reader, self.track);
17+
let writer = DatagramsWriter::new(tx, self.track.clone());
18+
let reader = DatagramsReader::new(rx, self.track);
1719

1820
(writer, reader)
1921
}
2022
}
2123

22-
struct DatagramsState {
23-
// The latest datagram
24-
latest: Option<Datagram>,
25-
26-
// Increased each time datagram changes.
27-
epoch: u64,
28-
29-
// Set when the writer or all readers are dropped.
30-
closed: Result<(), ServeError>,
31-
}
32-
33-
impl Default for DatagramsState {
34-
fn default() -> Self {
35-
Self {
36-
latest: None,
37-
epoch: 0,
38-
closed: Ok(()),
39-
}
40-
}
41-
}
42-
4324
pub struct DatagramsWriter {
44-
state: State<DatagramsState>,
25+
tx: broadcast::Sender<Datagram>,
4526
pub track: Arc<Track>,
4627
}
4728

4829
impl DatagramsWriter {
49-
fn new(state: State<DatagramsState>, track: Arc<Track>) -> Self {
50-
Self { state, track }
30+
fn new(tx: broadcast::Sender<Datagram>, track: Arc<Track>) -> Self {
31+
Self { tx, track }
5132
}
5233

5334
pub fn write(&mut self, datagram: Datagram) -> Result<(), ServeError> {
54-
let mut state = self.state.lock_mut().ok_or(ServeError::Cancel)?;
55-
56-
state.latest = Some(datagram);
57-
state.epoch += 1;
58-
35+
// Ignore send errors (no receivers) - datagrams are fire-and-forget
36+
let _ = self.tx.send(datagram);
5937
Ok(())
6038
}
6139

62-
pub fn close(self, err: ServeError) -> Result<(), ServeError> {
63-
let state = self.state.lock();
64-
state.closed.clone()?;
65-
66-
let mut state = state.into_mut().ok_or(ServeError::Cancel)?;
67-
state.closed = Err(err);
68-
40+
pub fn close(self, _err: ServeError) -> Result<(), ServeError> {
41+
// Channel closes when tx is dropped
6942
Ok(())
7043
}
7144
}
7245

73-
#[derive(Clone)]
7446
pub struct DatagramsReader {
75-
state: State<DatagramsState>,
47+
rx: broadcast::Receiver<Datagram>,
7648
pub track: Arc<Track>,
49+
latest: Option<(u64, u64)>,
50+
}
7751

78-
epoch: u64,
52+
impl Clone for DatagramsReader {
53+
fn clone(&self) -> Self {
54+
Self {
55+
rx: self.rx.resubscribe(),
56+
track: self.track.clone(),
57+
latest: self.latest,
58+
}
59+
}
7960
}
8061

8162
impl DatagramsReader {
82-
fn new(state: State<DatagramsState>, track: Arc<Track>) -> Self {
63+
fn new(rx: broadcast::Receiver<Datagram>, track: Arc<Track>) -> Self {
8364
Self {
84-
state,
65+
rx,
8566
track,
86-
epoch: 0,
67+
latest: None,
8768
}
8869
}
8970

9071
pub async fn read(&mut self) -> Result<Option<Datagram>, ServeError> {
9172
loop {
92-
{
93-
let state = self.state.lock();
94-
if self.epoch < state.epoch {
95-
self.epoch = state.epoch;
96-
return Ok(state.latest.clone());
73+
match self.rx.recv().await {
74+
Ok(datagram) => {
75+
self.latest = Some((datagram.group_id, datagram.object_id));
76+
return Ok(Some(datagram));
9777
}
98-
99-
state.closed.clone()?;
100-
match state.modified() {
101-
Some(notify) => notify,
102-
None => return Ok(None), // No more updates will come
78+
Err(broadcast::error::RecvError::Lagged(n)) => {
79+
log::warn!("[DATAGRAMS] reader lagged by {} datagrams", n);
80+
// Continue reading - we'll get the next available datagram
81+
}
82+
Err(broadcast::error::RecvError::Closed) => {
83+
return Ok(None); // Channel closed
10384
}
10485
}
105-
.await;
10686
}
10787
}
10888

109-
// Returns the largest group/sequence
11089
pub fn latest(&self) -> Option<(u64, u64)> {
111-
let state = self.state.lock();
112-
state
113-
.latest
114-
.as_ref()
115-
.map(|datagram| (datagram.group_id, datagram.object_id))
90+
self.latest
11691
}
11792

11893
pub fn is_closed(&self) -> bool {
119-
let state = self.state.lock();
120-
state.closed.is_err() || state.modified().is_none()
94+
// Check if channel is closed by seeing if there are no more senders
95+
self.rx.len() == 0
12196
}
12297
}
12398

0 commit comments

Comments
 (0)