Skip to content

UDP sound sync: robustness review vs legacy WLED — fixed issues + remaining gaps #18

Description

@netmindz

Summary

Comparing legacy usermods/audioreactive/audio_reactive.cpp in wled/WLED against this repo's audio_reactive.h, several network-robustness problems in the UDP sound sync path explain field reports of sync "not working at all" on some routers/mesh systems, high latency, and bursty/stuttery playback. This repo has already fixed most of them. Filing this as a tracking issue for the remaining gaps and to document why the legacy behaviour is worse, since the underlying protocol and struct layout are shared history.

Legacy WLED behaviour and why it's fragile

  1. Multicast-only, one join, no re-join. Sender/receiver only ever join 239.0.0.1 once, in connected(), triggered on WiFi (re)connect. If IGMP snooping on the AP/mesh/switch ages out the membership (common when there's no active IGMP querier), delivery silently stops for as long as the device stays associated to WiFi — there's no periodic re-join or "receiving nothing, force a rejoin" watchdog.
  2. One packet read per loop pass. receiveAudioData() calls parsePacket()/read() once and returns. The lwIP UDP receive mailbox (6 packets on current esp32-arduino) fills up during any AP-side buffering (e.g. DTIM-delayed multicast delivery while another station is in power-save), so a burst of packets overflows the mailbox and the receiver plays back stale, queued packets rather than draining to the freshest one.
  3. No sequence/duplicate/reorder detection. The V2 packet has reserved padding bytes but no counter or timestamp, so reordered, duplicated, or stale packets are indistinguishable from fresh ones at the receiver.
  4. Only a multicast sender, so networks that filter 239.x.x.x (common on consumer mesh systems) but pass broadcast/unicast have no fallback.

What this repo (MoonModules) already fixes

  • frameCounter field (audioSyncPacket, reusing the old reserved1 gap) with 8-bit rollover-safe sequence checking in decodeAudioData(), gated by audioSyncSequence. Fixes point 3.
  • Queue-draining receive loop: receiveAudioData(maxSamples) loops over parsePacket()/read() until empty or capped, with maxSamples computed adaptively from time-since-last-good-packet and a user-configurable audioSyncPurge policy (never / auto / always drop stale queued packets). Fixes point 2.
  • 25-second idle watchdog: if previously receiving valid data (receivedFormat > 0) and nothing valid arrives for > 25000ms, the socket is closed (fftUdp.stop(), udpSyncConnected = false) and audio state reset, letting connectUDPSoundSync() reopen and re-beginMulticast() (i.e. re-send IP_ADD_MEMBERSHIP) on the next attempt. Mitigates point 1 for the "was working, then silently stopped" case.
  • audioSyncBroadcast option: sender can use subnet broadcast instead of multicast, as an escape hatch for networks that block/filter the multicast group. Mitigates point 4 (one-directional: broadcast-sending receivers on multicast sockets still get it, per code comment, but it doesn't renegotiate transport both ways).

What could still be improved

  1. No watchdog for "never received a single packet": the 25s idle-close only triggers after receivedFormat > 0 (at least one valid packet seen). If the very first beginMulticast() / IGMP join is silently dropped by a snooping switch/AP (no querier present), udpSyncConnected stays true forever and there's no forced re-join — connectUDPSoundSync() only retries when udpSyncConnected is false. Suggest: also force a close+reopen if udpSyncConnected is true but receivedFormat == 0 and enough time (e.g. same 25s window) has passed since the connection was opened, in receive mode.
  2. Multicast join uses the default interface (INADDR_ANY) via WiFiUDP::beginMulticast(), with no explicit-interface join (unlike AsyncUDP::listenMulticast(..., tcpip_if) used elsewhere for E1.31/DDP). On AP+STA or Ethernet+WiFi builds this can join on the wrong interface.
  3. No explicit multicast TTL is set on the sync socket; it's left at whatever lwIP defaults to, which matters for setups where sync needs to cross a router (TTL 1 default won't).
  4. audioSyncBroadcast is sender-only: a receiver-side fallback (e.g. auto-detect "no multicast traffic for N seconds, try binding broadcast too") would make the option more of a genuine fallback rather than something both ends must be manually configured to use.

References

  • Legacy implementation (for comparison): usermods/audioreactive/audio_reactive.cpp in wled/WLED, functions connectUDPSoundSync(), transmitAudioData(), receiveAudioData(), decodeAudioData().
  • This repo's implementation: audio_reactive.h, same function names, plus AUDIOSYNC_IDLE_MS, AR_UDP_READ_INTERVAL_MS, audioSyncPurge, audioSyncSequence, audioSyncBroadcast.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or request

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions