<!DOCTYPE html><html><head><title></title><style type="text/css">p.MsoNormal,p.MsoNoSpacing{margin:0}</style></head><body><div>Hello Natalie,<br></div><div><br></div><div>This patch seems fine to me.<br></div><div>Indeed, the timer is disarmed just before this code block. It seems logical to rearm it in case of success.<br></div><div><br></div><div>On Thu, Oct 29, 2020, at 15:50, Natalie Landsberg wrote:<br></div><blockquote type="cite" id="qt" style=""><div>> The comment seems to imply that the patch reintroduces a data race.<br></div><div><br></div><div>Looking at the code again…. It seems as though the updateTimer is never actually scheduled on an interval. <br></div><div>It’s only called as single shots. I’m not sure whether that was removed, or never there in the first place. <br></div><div>And why some streams would happily continue for quite a while without it.<br></div><div><br></div><div>My suggestion: Add vlc_timer_schedule with interval after it is confirmed <br></div><div>that UDP packet can be received on the socket.<br></div><div><br></div><div>This way, the timer is still disarmed if it was armed before (amt_recv_relay_mem_query has a single shot)<br></div><div>and it is scheduled on its intended interval.<br></div><div><br></div><div>I've tested that this indeed works with a stream and sends the membership reports on a 125 interval,<br></div><div>but will continue testing as well.<br></div><div>---<br></div><div>modules/access/amt.c | 4 ++++<br></div><div>1 file changed, 4 insertions(+)<br></div><div><br></div><div>diff --git a/modules/access/amt.c b/modules/access/amt.c<br></div><div>index 52bf0271ef..40caa5e14f 100644<br></div><div>--- a/modules/access/amt.c<br></div><div>+++ b/modules/access/amt.c<br></div><div>@@ -746,6 +746,10 @@ static bool open_amt_tunnel( stream_t *p_access )<br></div><div> {<br></div><div> block_Release( pkt );<br></div><div> msg_Dbg( p_access, "Got UDP packet from multicast group via AMT relay %s, continuing...", relay_ip );<br></div><div>+ /* Arms the timer again for a single shot from this callback. That way, the<br></div><div>+ * time spent in amt_send_mem_update() is taken into consideration. */<br></div><div>+ vlc_timer_schedule( sys->updateTimer, false,<br></div><div>+ VLC_TICK_FROM_SEC( sys->relay_igmp_query.qqic ), VLC_TICK_FROM_SEC( sys->relay_igmp_query.qqic ) );<br></div><div> break; /* found an active server sending UDP packets, so exit loop */<br></div><div> }<br></div><div> }<br></div><div>-- <br></div><div>2.24.3 (Apple Git-128)<br></div><div><br></div><div><br></div><div>_______________________________________________<br></div><div>vlc-devel mailing list<br></div><div>To unsubscribe or modify your subscription options:<br></div><div><a href="https://mailman.videolan.org/listinfo/vlc-devel">https://mailman.videolan.org/listinfo/vlc-devel</a><br></div></blockquote><div><br></div></body></html>