Conversation
Fully containerized implementation of the hydrophone streaming node. JACK, ffmpeg, and Python uploaders all run inside the container; the host only needs Docker. Includes internet outage recovery via catchup_s3.py and automatic restart via Docker restart: always. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a Dockerized hydrophone node: image, compose, Pi provisioning, entrypoint orchestration (JACK + ffmpeg), live and catch-up S3 uploaders, manifest-repair tooling, environment templates, and deployment/troubleshooting documentation. ChangesOrcasound Hydrophone Streaming Node
Sequence DiagramssequenceDiagram
participant Operator
participant Pi_OS
participant setup.sh
participant Docker_Daemon
participant Dockerfile
participant Container
Operator->>setup.sh: Run setup.sh
setup.sh->>Pi_OS: apt update/upgrade, install Docker
setup.sh->>Pi_OS: Add user to docker & audio groups
setup.sh->>Docker_Daemon: Write daemon.json (disable IPv6)
Operator->>Docker_Daemon: docker compose up
Docker_Daemon->>Dockerfile: Build image
Dockerfile->>Container: Install ALSA/JACK/ffmpeg and /venv deps
Docker_Daemon->>Container: Mount /dev/snd, /etc/localtime, /var/run/chrony
Container->>Container: Run stream_sync.sh entrypoint
sequenceDiagram
participant Container
participant stream_sync
participant ALSA_Device
participant jackd
participant ffmpeg
participant JACK_System
participant Python_Uploaders
Container->>stream_sync: Entrypoint
stream_sync->>stream_sync: Preflight checks + wait_for_sync
stream_sync->>ALSA_Device: aplay -l (discover device)
stream_sync->>jackd: Start jackd with ALSA device
stream_sync->>jackd: Wait for JACK ready
stream_sync->>ffmpeg: Start ffmpeg (HLS ± FLAC)
ffmpeg->>Filesystem: Emit HLS .ts segments
stream_sync->>JACK_System: Connect capture ports -> ffjack inputs
stream_sync->>Python_Uploaders: Start upload_s3.py & catchup_s3.py
sequenceDiagram
participant ffmpeg
participant HLS_Dir
participant upload_s3
participant inotify
participant ffmpeg_RMS
participant boto3
participant S3
ffmpeg->>HLS_Dir: Write .ts segment
inotify->>upload_s3: IN_CLOSE_WRITE / IN_MOVED_TO
upload_s3->>HLS_Dir: Verify file exists and size>0
alt .ts segment
upload_s3->>ffmpeg_RMS: Decode + compute RMS
ffmpeg_RMS->>upload_s3: RMS value
upload_s3->>upload_s3: Log RMS (warn if low)
end
upload_s3->>boto3: upload_file(local_path, s3_key)
boto3->>S3: Store object
S3->>upload_s3: Success
upload_s3->>HLS_Dir: Delete local file
alt First .ts segment
upload_s3->>S3: Upload latest.txt
end
sequenceDiagram
participant CatchupMain
participant Filesystem
participant DiskGuard
participant Connectivity
participant S3
loop every SCAN_INTERVAL
CatchupMain->>Filesystem: find_stranded_segments (age>STALE_AGE)
Filesystem->>CatchupMain: list of .ts files
CatchupMain->>DiskGuard: enforce_disk_guard(total_size)
alt size > MAX_STRANDED_BYTES
DiskGuard->>Filesystem: delete oldest files
end
CatchupMain->>Connectivity: is_connected()
alt connected
CatchupMain->>Filesystem: write catchup.m3u8
CatchupMain->>S3: upload manifest then segments (throttled)
S3->>Filesystem: on success -> delete uploaded locals
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
🧹 Nitpick comments (2)
node_2026/setup.sh (1)
23-23: ⚡ Quick winAvoid piping a remote script directly to
shfor Docker installation.This weakens supply-chain integrity controls. Prefer a pinned/reviewable install path (official apt repo steps with keyring and explicit package versions).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@node_2026/setup.sh` at line 23, Replace the unsafe one-liner "curl -fsSL https://get.docker.com | sudo sh" with an explicit, reviewable install sequence: either (A) add the official Docker apt repository and GPG key using apt-key replacement (store key in /usr/share/keyrings/docker-archive-keyring.gpg), add the repository with "signed-by", run apt update, and install pinned package versions like docker-ce=<version> docker-ce-cli=<version> containerd.io, or (B) if you must use the upstream install script, download it first (curl -fsSL -o get-docker.sh), verify its checksum or signature, review it, then run it locally with sudo sh get-docker.sh; update setup.sh to implement one of these secure flows instead of piping directly to sh.node_2026/docker-compose.yml (1)
2-8: ⚡ Quick winAdd an init process for cleaner shutdown/restart behavior.
Using
init: truehelps forward signals and reap zombies for the bash entrypoint, improving stop/restart reliability.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@node_2026/docker-compose.yml` around lines 2 - 8, The docker-compose service "streaming" lacks an init process; add the "init: true" key under the streaming service definition (at the same indentation level as image/build/command) so Docker will use tini to forward signals and reap zombies for the ./stream_sync.sh entrypoint, improving shutdown/restart behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@node_2026/catchup_s3.py`:
- Around line 28-35: The module-level BUCKET logic can leave BUCKET empty when
BUCKET_TYPE is unset; update the top-level code around BUCKET, BUCKET_TYPE and
BUCKET_STREAMING in catchup_s3.py to fail fast: if BUCKET_TYPE is missing or
empty raise a clear RuntimeError (or SystemExit) explaining BUCKET_TYPE must be
set; when BUCKET_TYPE == "custom" also validate BUCKET_STREAMING exists and
raise a clear error if not; otherwise continue to set BUCKET to
"audio-orcasound-net" for "prod" and "dev-streaming-orcasound-net" for other
values.
- Around line 83-94: The sorted() key can raise FileNotFoundError when
os.path.getmtime(p) is called for a file removed concurrently; update
enforce_disk_guard to use a safe mtime getter (e.g. safe_getmtime(path) that
wraps os.path.getmtime in try/except and returns a sentinel like float('inf') or
filters out missing files) and/or pre-filter stranded to only existing paths
before sorting; then sort using key=safe_getmtime, proceed with the existing
loop that pops victims and calls total_size(aged) against MAX_STRANDED_BYTES,
and ensure aged only contains existing files when returned.
In `@node_2026/docker-compose.yml`:
- Around line 20-35: Remove the broad "privileged: true" setting and instead
grant only the needed hardware and kernel capabilities: keep the existing
devices mapping (devices: - "/dev/snd:/dev/snd"), preserve shm_size and ulimits
(rtprio and memlock), and add targeted fields such as cap_add:
["SYS_NICE","IPC_LOCK"] and group_add if a specific group is required; ensure
logging options remain unchanged. Update the docker-compose service block where
"privileged" is set to replace it with the explicit cap_add and group_add
entries and verify the device list and ulimits are intact so the container
retains required real-time and memory-lock privileges without full host
privilege escalation.
In `@node_2026/Dockerfile`:
- Around line 36-46: The container currently runs as root (see RUN usermod -aG
audio root and CMD ["./stream_sync.sh"]); create a dedicated non-root runtime
user (e.g., "appuser"), add that user to the audio group, chown /app and any
needed sockets/dev files to that user, remove the usermod root modification, and
switch to that user with USER appuser before the final CMD so the process
executes without root privileges while still retaining access to audio via group
membership; ensure stream_sync.sh remains executable and owned by the non-root
user.
In `@node_2026/README.txt`:
- Around line 53-56: The README references the wrong project directory name:
replace usages of the old directory string "node_val_docker" with the new
workflow directory "node_2026" in all occurrences (e.g., the commands that
change into ~/orcanode/node_val_docker and any subsequent references), update
the example cd/git clone steps and any paths mentioned around the spots flagged
(previously at lines referencing cd ~/orcanode/node_val_docker) so they
consistently use ~/orcanode/node_2026, and verify other references (e.g., in
setup, build, and run instructions) are updated to the unique identifiers
"node_val_docker" -> "node_2026" to avoid broken navigation.
- Around line 142-145: Update the watch command so it targets actual .ts segment
files instead of directories: replace the current '/tmp/<NODE_NAME>/hls/*/' glob
with a file glob that matches segment files (e.g. '/tmp/<NODE_NAME>/hls/*.ts' or
if segments live in per-stream subfolders use '/tmp/<NODE_NAME>/hls/*/*.ts') in
the README command so operators can see real-time .ts segment churn.
In `@node_2026/setup.sh`:
- Around line 53-58: The script currently unconditionally overwrites
/etc/docker/daemon.json via the heredoc written with "sudo tee
/etc/docker/daemon.json" (the DAEMONJSON heredoc); change this to preserve
existing settings by first checking for an existing file and merging the desired
keys instead of clobbering: if /etc/docker/daemon.json exists, load it and
merge/patch the "ipv6" and "dns" entries using jq (or create a temp JSON merge),
then write the merged result atomically back to /etc/docker/daemon.json; if it
doesn’t exist, create it as before. Ensure you handle permissions via sudo and
avoid partial writes by writing to a temp file and moving it into place.
- Around line 37-51: Remove the entire curl-based IPv4 extraction and /etc/hosts
mutation block in setup.sh (the code that sets DOCKER_IP and DOCKER_AUTH_IP, the
grep -qF checks, the sudo tee -a /etc/hosts lines, and the corresponding echo
warnings), since daemon.json already enforces IPv4/dns behavior; leave the
daemon.json configuration unchanged and ensure no other part of setup.sh depends
on DOCKER_IP or DOCKER_AUTH_IP before committing.
In `@node_2026/stream_sync.sh`:
- Around line 1-159: Save the jackd PID immediately after launching it (capture
PID into JACK_PID right after the jackd start line) and stop relying on implicit
background jobs; move FFMPEG_PID assignment into each NODE_TYPE branch where
ffmpeg is started so it's always set, then add a trap for SIGTERM, SIGINT, and
EXIT that explicitly kills and waits for JACK_PID, FFMPEG_PID and any background
python uploader PIDs (e.g., catchup_s3, upload_s3, upload_flac_s3) using kill
-TERM <PID> and wait <PID> (avoid kill 0); ensure the trap runs before launching
background uploaders and that all spawned PIDs are recorded so the trap can
terminate them cleanly.
In `@node_2026/upload_s3.py`:
- Around line 116-118: Replace the bare "except:" in upload_s3.py with a safe
exception handler: catch "Exception as e" instead of all exceptions so
KeyboardInterrupt/SystemExit are not swallowed, and log the full traceback (for
example via log.exception or traceback.format_exc()) when reporting "error
uploading to S3". Target the existing except block that currently does "except:"
/ "e = sys.exc_info()[0]" / "log.critical(...)" and change it to use "except
Exception as e" and emit the error details.
- Around line 129-135: The IN_MOVED_TO branch currently uploads every moved file
while the IN_CLOSE_WRITE branch skips temp files; update the IN_MOVED_TO handler
to apply the same guard by checking that 'tmp' not in filename before calling
s3_copy_file(path, filename). Locate the block that inspects type_names[0] ==
'IN_MOVED_TO' and add the same conditional used for the IN_CLOSE_WRITE branch
(the 'tmp' check and log.debug call) so only non-temporary files get passed to
s3_copy_file.
- Around line 28-39: The BUCKET variable can remain empty when BUCKET_TYPE is
unset causing obscure S3 errors; update upload_s3.py to validate
BUCKET_TYPE/BUCKET after the existing conditional (or when declaring BUCKET) and
fail fast with a clear error message (raise an exception or exit) if BUCKET_TYPE
is missing or BUCKET is still empty; reference the BUCKET and BUCKET_TYPE
environment checks and the BUCKET variable so the change is applied right after
that logic so any boto3 upload code using BUCKET sees a clear, early failure.
---
Nitpick comments:
In `@node_2026/docker-compose.yml`:
- Around line 2-8: The docker-compose service "streaming" lacks an init process;
add the "init: true" key under the streaming service definition (at the same
indentation level as image/build/command) so Docker will use tini to forward
signals and reap zombies for the ./stream_sync.sh entrypoint, improving
shutdown/restart behavior.
In `@node_2026/setup.sh`:
- Line 23: Replace the unsafe one-liner "curl -fsSL https://get.docker.com |
sudo sh" with an explicit, reviewable install sequence: either (A) add the
official Docker apt repository and GPG key using apt-key replacement (store key
in /usr/share/keyrings/docker-archive-keyring.gpg), add the repository with
"signed-by", run apt update, and install pinned package versions like
docker-ce=<version> docker-ce-cli=<version> containerd.io, or (B) if you must
use the upstream install script, download it first (curl -fsSL -o
get-docker.sh), verify its checksum or signature, review it, then run it locally
with sudo sh get-docker.sh; update setup.sh to implement one of these secure
flows instead of piping directly to sh.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: aa297f84-1fb3-44e8-9eed-ab5b593113ee
📒 Files selected for processing (10)
node_2026/.dockerignorenode_2026/.env.templatenode_2026/.gitignorenode_2026/Dockerfilenode_2026/README.txtnode_2026/catchup_s3.pynode_2026/docker-compose.ymlnode_2026/setup.shnode_2026/stream_sync.shnode_2026/upload_s3.py
Comment on lines
+28
to
+35
| BUCKET = "" | ||
| if "BUCKET_TYPE" in os.environ: | ||
| if os.environ["BUCKET_TYPE"] == "prod": | ||
| BUCKET = "audio-orcasound-net" | ||
| elif os.environ["BUCKET_TYPE"] == "custom": | ||
| BUCKET = os.environ["BUCKET_STREAMING"] | ||
| else: | ||
| BUCKET = "dev-streaming-orcasound-net" |
There was a problem hiding this comment.
Same issue: empty BUCKET when BUCKET_TYPE is unset.
Same problem as in upload_s3.py—if BUCKET_TYPE is not set, BUCKET remains empty and S3 operations will fail with an unclear error. Add a fail-fast check.
Proposed fix
BUCKET = ""
if "BUCKET_TYPE" in os.environ:
if os.environ["BUCKET_TYPE"] == "prod":
BUCKET = "audio-orcasound-net"
elif os.environ["BUCKET_TYPE"] == "custom":
BUCKET = os.environ["BUCKET_STREAMING"]
else:
BUCKET = "dev-streaming-orcasound-net"
+else:
+ log.critical("BUCKET_TYPE not set; exiting")
+ sys.exit(1)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| BUCKET = "" | |
| if "BUCKET_TYPE" in os.environ: | |
| if os.environ["BUCKET_TYPE"] == "prod": | |
| BUCKET = "audio-orcasound-net" | |
| elif os.environ["BUCKET_TYPE"] == "custom": | |
| BUCKET = os.environ["BUCKET_STREAMING"] | |
| else: | |
| BUCKET = "dev-streaming-orcasound-net" | |
| BUCKET = "" | |
| if "BUCKET_TYPE" in os.environ: | |
| if os.environ["BUCKET_TYPE"] == "prod": | |
| BUCKET = "audio-orcasound-net" | |
| elif os.environ["BUCKET_TYPE"] == "custom": | |
| BUCKET = os.environ["BUCKET_STREAMING"] | |
| else: | |
| BUCKET = "dev-streaming-orcasound-net" | |
| else: | |
| log.critical("BUCKET_TYPE not set; exiting") | |
| sys.exit(1) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@node_2026/catchup_s3.py` around lines 28 - 35, The module-level BUCKET logic
can leave BUCKET empty when BUCKET_TYPE is unset; update the top-level code
around BUCKET, BUCKET_TYPE and BUCKET_STREAMING in catchup_s3.py to fail fast:
if BUCKET_TYPE is missing or empty raise a clear RuntimeError (or SystemExit)
explaining BUCKET_TYPE must be set; when BUCKET_TYPE == "custom" also validate
BUCKET_STREAMING exists and raise a clear error if not; otherwise continue to
set BUCKET to "audio-orcasound-net" for "prod" and "dev-streaming-orcasound-net"
for other values.
Comment on lines
+83
to
+94
| def enforce_disk_guard(stranded): | ||
| """Delete oldest stranded segments if total exceeds MAX_STRANDED_BYTES.""" | ||
| # Sort oldest first (by mtime) | ||
| aged = sorted(stranded, key=lambda p: os.path.getmtime(p)) | ||
| while total_size(aged) > MAX_STRANDED_BYTES and aged: | ||
| victim = aged.pop(0) | ||
| try: | ||
| os.remove(victim) | ||
| log.warning(f"Disk guard: deleted {os.path.basename(victim)}") | ||
| except FileNotFoundError: | ||
| pass | ||
| return aged # updated list after deletions |
There was a problem hiding this comment.
sorted() key can raise FileNotFoundError if a file is deleted mid-operation.
If a file in stranded is deleted between finding it and sorting, os.path.getmtime(p) in the key function will raise FileNotFoundError, crashing the disk guard.
Proposed fix using a safe getter
def enforce_disk_guard(stranded):
"""Delete oldest stranded segments if total exceeds MAX_STRANDED_BYTES."""
- # Sort oldest first (by mtime)
- aged = sorted(stranded, key=lambda p: os.path.getmtime(p))
+ # Sort oldest first (by mtime); deleted files sort to end
+ def safe_mtime(p):
+ try:
+ return os.path.getmtime(p)
+ except FileNotFoundError:
+ return float('inf')
+ aged = sorted(stranded, key=safe_mtime)
while total_size(aged) > MAX_STRANDED_BYTES and aged:🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@node_2026/catchup_s3.py` around lines 83 - 94, The sorted() key can raise
FileNotFoundError when os.path.getmtime(p) is called for a file removed
concurrently; update enforce_disk_guard to use a safe mtime getter (e.g.
safe_getmtime(path) that wraps os.path.getmtime in try/except and returns a
sentinel like float('inf') or filters out missing files) and/or pre-filter
stranded to only existing paths before sorting; then sort using
key=safe_getmtime, proceed with the existing loop that pops victims and calls
total_size(aged) against MAX_STRANDED_BYTES, and ensure aged only contains
existing files when returned.
Comment on lines
+20
to
+35
| devices: | ||
| - "/dev/snd:/dev/snd" | ||
|
|
||
| # 2. Grant high-level hardware access | ||
| logging: | ||
| driver: json-file | ||
| options: | ||
| max-size: "10m" | ||
| max-file: "3" | ||
| privileged: true | ||
| shm_size: '256m' | ||
| ulimits: | ||
| rtprio: 95 | ||
| memlock: | ||
| soft: -1 | ||
| hard: -1 |
There was a problem hiding this comment.
Avoid full privileged mode; grant only required capabilities/devices.
privileged: true grants near-host-level control. For this workload, prefer explicit devices, group_add, and targeted cap_add (for example, SYS_NICE, possibly IPC_LOCK) plus ulimits. This materially reduces security risk.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@node_2026/docker-compose.yml` around lines 20 - 35, Remove the broad
"privileged: true" setting and instead grant only the needed hardware and kernel
capabilities: keep the existing devices mapping (devices: -
"/dev/snd:/dev/snd"), preserve shm_size and ulimits (rtprio and memlock), and
add targeted fields such as cap_add: ["SYS_NICE","IPC_LOCK"] and group_add if a
specific group is required; ensure logging options remain unchanged. Update the
docker-compose service block where "privileged" is set to replace it with the
explicit cap_add and group_add entries and verify the device list and ulimits
are intact so the container retains required real-time and memory-lock
privileges without full host privilege escalation.
Comment on lines
+36
to
+46
| RUN usermod -aG audio root | ||
|
|
||
| ###### Set working directory and copy files ############################ | ||
|
|
||
| WORKDIR /app | ||
| COPY . . | ||
|
|
||
| RUN chmod +x stream_sync.sh | ||
| ###### Runtime ######################################################### | ||
|
|
||
| CMD ["./stream_sync.sh"] |
There was a problem hiding this comment.
Drop root runtime user for the container process.
The image runs as root, and this stacks with privileged: true in compose, making host compromise impact much worse. Add a non-root runtime user and only keep the minimum permissions required for audio access.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@node_2026/Dockerfile` around lines 36 - 46, The container currently runs as
root (see RUN usermod -aG audio root and CMD ["./stream_sync.sh"]); create a
dedicated non-root runtime user (e.g., "appuser"), add that user to the audio
group, chown /app and any needed sockets/dev files to that user, remove the
usermod root modification, and switch to that user with USER appuser before the
final CMD so the process executes without root privileges while still retaining
access to audio via group membership; ensure stream_sync.sh remains executable
and owned by the non-root user.
Comment on lines
+53
to
+56
| sudo apt-get install -y git | ||
| git clone https://github.com/orcasound/orcanode.git ~/orcanode | ||
| cd ~/orcanode/node_val_docker | ||
|
|
There was a problem hiding this comment.
Fix inconsistent project path references (node_val_docker vs node_2026).
Line 55, Line 101, Line 116, Line 216, and Line 218 point users to ~/orcanode/node_val_docker, but this PR introduces the workflow under ~/orcanode/node_2026. These commands will send users to the wrong directory.
Suggested doc fix
- cd ~/orcanode/node_val_docker
+ cd ~/orcanode/node_2026
...
- cd ~/orcanode/node_val_docker
+ cd ~/orcanode/node_2026
...
- cd ~/orcanode/node_val_docker
+ cd ~/orcanode/node_2026
...
- file ~/orcanode/node_val_docker/.env
+ file ~/orcanode/node_2026/.env
...
- sed -i 's/\r//' ~/orcanode/node_val_docker/.env
+ sed -i 's/\r//' ~/orcanode/node_2026/.envAlso applies to: 101-103, 116-117, 216-219
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@node_2026/README.txt` around lines 53 - 56, The README references the wrong
project directory name: replace usages of the old directory string
"node_val_docker" with the new workflow directory "node_2026" in all occurrences
(e.g., the commands that change into ~/orcanode/node_val_docker and any
subsequent references), update the example cd/git clone steps and any paths
mentioned around the spots flagged (previously at lines referencing cd
~/orcanode/node_val_docker) so they consistently use ~/orcanode/node_2026, and
verify other references (e.g., in setup, build, and run instructions) are
updated to the unique identifiers "node_val_docker" -> "node_2026" to avoid
broken navigation.
Comment on lines
+53
to
+58
| sudo tee /etc/docker/daemon.json > /dev/null << 'DAEMONJSON' | ||
| { | ||
| "ipv6": false, | ||
| "dns": ["8.8.8.8", "8.8.4.4"] | ||
| } | ||
| DAEMONJSON |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cd /repo && find . -name "setup.sh" -path "*/node_2026/*" 2>/dev/null | head -20Repository: orcasound/orcanode
Length of output: 118
🏁 Script executed:
cd /repo && git ls-files | grep -i "node_2026/setup.sh"Repository: orcasound/orcanode
Length of output: 118
🏁 Script executed:
find . -name "setup.sh" -path "*/node_2026/*" 2>/dev/nullRepository: orcasound/orcanode
Length of output: 83
🏁 Script executed:
git ls-files | grep "node_2026/setup.sh"Repository: orcasound/orcanode
Length of output: 81
🏁 Script executed:
wc -l node_2026/setup.shRepository: orcasound/orcanode
Length of output: 84
🏁 Script executed:
cat -n node_2026/setup.sh | sed -n '45,70p'Repository: orcasound/orcanode
Length of output: 1203
🏁 Script executed:
cat -n node_2026/setup.sh | head -60Repository: orcasound/orcanode
Length of output: 2551
🏁 Script executed:
grep -n "daemon.json" node_2026/setup.shRepository: orcasound/orcanode
Length of output: 126
🏁 Script executed:
grep -n "docker" node_2026/setup.sh -i | head -20Repository: orcasound/orcanode
Length of output: 1173
Avoid overwriting /etc/docker/daemon.json unconditionally.
This script uses sudo tee to write the daemon configuration without checking for or preserving existing settings. On fresh Docker installs this may not matter, but the script has no safeguards to prevent silent loss of pre-existing daemon settings if run on a system that already has Docker configured. Use jq to merge settings into the existing file, or check if the file exists before writing.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@node_2026/setup.sh` around lines 53 - 58, The script currently
unconditionally overwrites /etc/docker/daemon.json via the heredoc written with
"sudo tee /etc/docker/daemon.json" (the DAEMONJSON heredoc); change this to
preserve existing settings by first checking for an existing file and merging
the desired keys instead of clobbering: if /etc/docker/daemon.json exists, load
it and merge/patch the "ipv6" and "dns" entries using jq (or create a temp JSON
merge), then write the merged result atomically back to /etc/docker/daemon.json;
if it doesn’t exist, create it as before. Ensure you handle permissions via sudo
and avoid partial writes by writing to a temp file and moving it into place.
Comment on lines
+1
to
+159
| #!/bin/bash | ||
| # Script for live DASH/HLS streaming lossy audio as AAC and/or archiving lossless audio as FLAC | ||
|
|
||
| # Ensure system binaries are in PATH | ||
| export PATH=/usr/bin:/usr/local/bin:/usr/sbin:/bin:$PATH | ||
|
|
||
| SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" | ||
|
|
||
| # --- 0. PREFLIGHT CHECKS --- | ||
| # The .env sourcing is commented out because Docker Compose handles variable injection. | ||
| # if [ -f "$SCRIPT_DIR/.env" ]; then | ||
| # set -a | ||
| # source "$SCRIPT_DIR/.env" | ||
| # set +a | ||
| # echo "Loaded .env from $SCRIPT_DIR" | ||
| # fi | ||
|
|
||
| if ! command -v jackd &> /dev/null; then | ||
| echo "ERROR: jackd is not installed." | ||
| exit 1 | ||
| fi | ||
| if ! command -v jack_wait &> /dev/null; then | ||
| echo "ERROR: jack_wait is not installed." | ||
| exit 1 | ||
| fi | ||
| if ! command -v ffmpeg &> /dev/null; then | ||
| echo "ERROR: ffmpeg is not installed." | ||
| exit 1 | ||
| fi | ||
|
|
||
| # --- 1. TIME SYNCHRONIZATION --- | ||
| wait_for_sync() { | ||
| echo "Checking for sane system time..." | ||
| local max_wait=60 | ||
| local elapsed=0 | ||
|
|
||
| # Check if the year is 2025 or later | ||
| while [ $(date +%Y) -lt 2025 ]; do | ||
| if [ $elapsed -ge $max_wait ]; then | ||
| echo "ERROR: Time sync timed out, aborting." | ||
| exit 1 | ||
| fi | ||
| echo "Waiting for time sync (current year: $(date +%Y))..." | ||
| sleep 2 | ||
| elapsed=$((elapsed + 2)) | ||
| done | ||
| echo "Time looks sane: $(date)" | ||
| } | ||
|
|
||
| wait_for_sync | ||
|
|
||
| # --- 2. ACTIVATE VIRTUAL ENVIRONMENT --- | ||
| # Updated to use the absolute path within the container. | ||
| source /venv/bin/activate | ||
|
|
||
| # --- 3. DYNAMIC AUDIO DEVICE DISCOVERY --- | ||
| echo "Searching for ALSA device: $AUDIO_HW_ID..." | ||
|
|
||
| # Loop to find the hardware index (e.g., card 3) and set the correct hw address. | ||
| for i in $(seq 1 30); do | ||
| if aplay -l 2>/dev/null | grep -qi "$AUDIO_HW_ID"; then | ||
| # Extracts the number from a line like "card 3: pisound [pisound]". | ||
| CARD_NUM=$(aplay -l | grep -i "$AUDIO_HW_ID" | head -n 1 | awk -F'[: ]+' '{print $2}') | ||
|
|
||
| if [ -n "$CARD_NUM" ]; then | ||
| SOUND_CARD="hw:$CARD_NUM,0" | ||
| echo "Success! $AUDIO_HW_ID found at index $CARD_NUM. Using address: $SOUND_CARD" | ||
| break | ||
| fi | ||
| fi | ||
| echo "Audio device $AUDIO_HW_ID not ready yet, attempt $i..." | ||
| sleep 1 | ||
| done | ||
|
|
||
| if [ -z "$SOUND_CARD" ]; then | ||
| echo "ERROR: ALSA device $AUDIO_HW_ID not found after 30s, aborting." | ||
| exit 1 | ||
| fi | ||
|
|
||
| # --- 4. VARIABLES & DIRECTORIES --- | ||
| timestamp=$(date +%s) | ||
| echo "Node started at $timestamp. Node name: $NODE_NAME. Sound card address: $SOUND_CARD" | ||
|
|
||
| mkdir -p /tmp/$NODE_NAME/flac | ||
| mkdir -p /tmp/$NODE_NAME/hls/$timestamp | ||
| echo $timestamp > /tmp/$NODE_NAME/latest.txt | ||
|
|
||
| STREAM_RATE=48000 | ||
| SAMPLE_RATE=${SAMPLE_RATE:-48000} | ||
|
|
||
| # --- 5. SETUP JACK --- | ||
| # Start jackd using the discovered $SOUND_CARD index. | ||
| JACK_NO_AUDIO_RESERVATION=1 jackd -t 2000 -P 75 -m -s -d alsa -d $SOUND_CARD -r $SAMPLE_RATE -p 1024 -n 10 & | ||
|
|
||
| echo "Waiting for JACK server to be ready..." | ||
| for i in $(seq 1 15); do | ||
| jack_wait -w -t 1 && break | ||
| if [ $i -eq 15 ]; then | ||
| echo "ERROR: JACK failed to start after 15s." | ||
| exit 1 | ||
| fi | ||
| sleep 1 | ||
| done | ||
| echo "JACK is ready." | ||
|
|
||
| # --- 6. FFMPEG STREAMING --- | ||
| FFMPEG_PID="" | ||
|
|
||
| if [ "$NODE_TYPE" = "research" ]; then | ||
| nice -n -10 ffmpeg -f jack -i ffjack \ | ||
| -f segment -segment_time "00:00:$FLAC_DURATION.00" -strftime 1 "/tmp/$NODE_NAME/flac/%Y-%m-%d_%H-%M-%S_$NODE_NAME-$SAMPLE_RATE-$CHANNELS.flac" \ | ||
| -f segment -segment_list "/tmp/$NODE_NAME/hls/$timestamp/live.m3u8" -segment_list_flags +live -segment_list_size 5 -segment_time $SEGMENT_DURATION -segment_format \ | ||
| mpegts -ar $STREAM_RATE -ac 2 -acodec aac "/tmp/$NODE_NAME/hls/$timestamp/live%03d.ts" \ | ||
| >/tmp/$NODE_NAME/ffmpeg.log 2>&1 & | ||
| FFMPEG_PID=$! | ||
| elif [ "$NODE_TYPE" = "hls-only" ]; then | ||
| nice -n -10 ffmpeg -f jack -i ffjack -f segment -segment_list "/tmp/$NODE_NAME/hls/$timestamp/live.m3u8" -segment_list_flags +live -segment_list_size 5 -segment_time $SEGMENT_DURATION -segment_format mpegts -ar $STREAM_RATE -ac $CHANNELS -threads 3 -acodec aac "/tmp/$NODE_NAME/hls/$timestamp/live%03d.ts" \ | ||
| >/tmp/$NODE_NAME/ffmpeg.log 2>&1 & | ||
| FFMPEG_PID=$! | ||
| else | ||
| echo "Unsupported NODE_TYPE. Please use research or hls-only." | ||
| exit 1 | ||
| fi | ||
|
|
||
| # --- 7. CONNECT JACK PORTS --- | ||
| echo "Waiting for ffjack ports..." | ||
| for i in $(seq 1 15); do | ||
| jack_lsp | grep -q "ffjack:input_1" && break | ||
| sleep 1 | ||
| done | ||
|
|
||
| if ! jack_lsp | grep -q "ffjack:input_1"; then | ||
| echo "ERROR: ffjack ports never appeared." | ||
| cat /tmp/$NODE_NAME/ffmpeg.log | ||
| exit 1 | ||
| fi | ||
|
|
||
| jack_connect -s default system:capture_1 ffjack:input_1 | ||
| jack_connect -s default system:capture_2 ffjack:input_2 | ||
|
|
||
| if [ "$NODE_LOOPBACK" = "true" ]; then | ||
| jack_connect system:capture_1 system:playback_1 | ||
| jack_connect system:capture_2 system:playback_2 | ||
| fi | ||
|
|
||
| # Launch Python uploaders | ||
| if [ "${NO_UPLOAD:-false}" = "true" ]; then | ||
| echo "NO_UPLOAD=true, skipping S3 upload. Segments will accumulate in /tmp/$NODE_NAME/hls/" | ||
| wait $FFMPEG_PID | ||
| elif [ "$NODE_TYPE" = "research" ]; then | ||
| nice -n 10 python3 catchup_s3.py & | ||
| python3 upload_s3.py & | ||
| python3 upload_flac_s3.py | ||
| else | ||
| nice -n 10 python3 catchup_s3.py & | ||
| python3 upload_s3.py | ||
| fi | ||
|
|
||
| echo "All processes started successfully." No newline at end of file |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "stream_sync.sh" -type f 2>/dev/null | head -20Repository: orcasound/orcanode
Length of output: 89
🏁 Script executed:
cat -n ./node_2026/stream_sync.shRepository: orcasound/orcanode
Length of output: 6361
Add signal trap and JACK process tracking to enable clean shutdown in container lifecycle.
The script launches jackd and ffmpeg as background jobs but lacks signal handlers and process tracking. This causes orphaned processes during container stop/restart, leading to restart failures and resource leaks. JACK is started at line 93 without saving its PID, preventing cleanup.
Key fixes:
- Save jackd PID immediately after launch
- Add trap on SIGTERM/SIGINT/EXIT to kill tracked processes
- Ensure all background jobs are properly terminated on exit
Suggested approach
Add at the top after shebang:
#!/bin/bash
# Script for live DASH/HLS streaming lossy audio as AAC and/or archiving lossless audio as FLAC
+
+# Process cleanup on exit/signal
+FFMPEG_PID=""
+JACK_PID=""
+cleanup() {
+ local code=$?
+ [ -n "${FFMPEG_PID}" ] && kill -TERM "$FFMPEG_PID" 2>/dev/null || true
+ [ -n "${JACK_PID}" ] && kill -TERM "$JACK_PID" 2>/dev/null || true
+ exit $code
+}
+trap cleanup SIGINT SIGTERM EXITThen track JACK_PID at line 93:
JACK_NO_AUDIO_RESERVATION=1 jackd -t 2000 -P 75 -m -s -d alsa -d $SOUND_CARD -r $SAMPLE_RATE -p 1024 -n 10 &
+JACK_PID=$!And move FFMPEG_PID initialization from line 107 into the conditional blocks where it's set.
Note: Avoid kill 0 (kills entire process group); instead kill only tracked processes explicitly.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| #!/bin/bash | |
| # Script for live DASH/HLS streaming lossy audio as AAC and/or archiving lossless audio as FLAC | |
| # Ensure system binaries are in PATH | |
| export PATH=/usr/bin:/usr/local/bin:/usr/sbin:/bin:$PATH | |
| SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" | |
| # --- 0. PREFLIGHT CHECKS --- | |
| # The .env sourcing is commented out because Docker Compose handles variable injection. | |
| # if [ -f "$SCRIPT_DIR/.env" ]; then | |
| # set -a | |
| # source "$SCRIPT_DIR/.env" | |
| # set +a | |
| # echo "Loaded .env from $SCRIPT_DIR" | |
| # fi | |
| if ! command -v jackd &> /dev/null; then | |
| echo "ERROR: jackd is not installed." | |
| exit 1 | |
| fi | |
| if ! command -v jack_wait &> /dev/null; then | |
| echo "ERROR: jack_wait is not installed." | |
| exit 1 | |
| fi | |
| if ! command -v ffmpeg &> /dev/null; then | |
| echo "ERROR: ffmpeg is not installed." | |
| exit 1 | |
| fi | |
| # --- 1. TIME SYNCHRONIZATION --- | |
| wait_for_sync() { | |
| echo "Checking for sane system time..." | |
| local max_wait=60 | |
| local elapsed=0 | |
| # Check if the year is 2025 or later | |
| while [ $(date +%Y) -lt 2025 ]; do | |
| if [ $elapsed -ge $max_wait ]; then | |
| echo "ERROR: Time sync timed out, aborting." | |
| exit 1 | |
| fi | |
| echo "Waiting for time sync (current year: $(date +%Y))..." | |
| sleep 2 | |
| elapsed=$((elapsed + 2)) | |
| done | |
| echo "Time looks sane: $(date)" | |
| } | |
| wait_for_sync | |
| # --- 2. ACTIVATE VIRTUAL ENVIRONMENT --- | |
| # Updated to use the absolute path within the container. | |
| source /venv/bin/activate | |
| # --- 3. DYNAMIC AUDIO DEVICE DISCOVERY --- | |
| echo "Searching for ALSA device: $AUDIO_HW_ID..." | |
| # Loop to find the hardware index (e.g., card 3) and set the correct hw address. | |
| for i in $(seq 1 30); do | |
| if aplay -l 2>/dev/null | grep -qi "$AUDIO_HW_ID"; then | |
| # Extracts the number from a line like "card 3: pisound [pisound]". | |
| CARD_NUM=$(aplay -l | grep -i "$AUDIO_HW_ID" | head -n 1 | awk -F'[: ]+' '{print $2}') | |
| if [ -n "$CARD_NUM" ]; then | |
| SOUND_CARD="hw:$CARD_NUM,0" | |
| echo "Success! $AUDIO_HW_ID found at index $CARD_NUM. Using address: $SOUND_CARD" | |
| break | |
| fi | |
| fi | |
| echo "Audio device $AUDIO_HW_ID not ready yet, attempt $i..." | |
| sleep 1 | |
| done | |
| if [ -z "$SOUND_CARD" ]; then | |
| echo "ERROR: ALSA device $AUDIO_HW_ID not found after 30s, aborting." | |
| exit 1 | |
| fi | |
| # --- 4. VARIABLES & DIRECTORIES --- | |
| timestamp=$(date +%s) | |
| echo "Node started at $timestamp. Node name: $NODE_NAME. Sound card address: $SOUND_CARD" | |
| mkdir -p /tmp/$NODE_NAME/flac | |
| mkdir -p /tmp/$NODE_NAME/hls/$timestamp | |
| echo $timestamp > /tmp/$NODE_NAME/latest.txt | |
| STREAM_RATE=48000 | |
| SAMPLE_RATE=${SAMPLE_RATE:-48000} | |
| # --- 5. SETUP JACK --- | |
| # Start jackd using the discovered $SOUND_CARD index. | |
| JACK_NO_AUDIO_RESERVATION=1 jackd -t 2000 -P 75 -m -s -d alsa -d $SOUND_CARD -r $SAMPLE_RATE -p 1024 -n 10 & | |
| echo "Waiting for JACK server to be ready..." | |
| for i in $(seq 1 15); do | |
| jack_wait -w -t 1 && break | |
| if [ $i -eq 15 ]; then | |
| echo "ERROR: JACK failed to start after 15s." | |
| exit 1 | |
| fi | |
| sleep 1 | |
| done | |
| echo "JACK is ready." | |
| # --- 6. FFMPEG STREAMING --- | |
| FFMPEG_PID="" | |
| if [ "$NODE_TYPE" = "research" ]; then | |
| nice -n -10 ffmpeg -f jack -i ffjack \ | |
| -f segment -segment_time "00:00:$FLAC_DURATION.00" -strftime 1 "/tmp/$NODE_NAME/flac/%Y-%m-%d_%H-%M-%S_$NODE_NAME-$SAMPLE_RATE-$CHANNELS.flac" \ | |
| -f segment -segment_list "/tmp/$NODE_NAME/hls/$timestamp/live.m3u8" -segment_list_flags +live -segment_list_size 5 -segment_time $SEGMENT_DURATION -segment_format \ | |
| mpegts -ar $STREAM_RATE -ac 2 -acodec aac "/tmp/$NODE_NAME/hls/$timestamp/live%03d.ts" \ | |
| >/tmp/$NODE_NAME/ffmpeg.log 2>&1 & | |
| FFMPEG_PID=$! | |
| elif [ "$NODE_TYPE" = "hls-only" ]; then | |
| nice -n -10 ffmpeg -f jack -i ffjack -f segment -segment_list "/tmp/$NODE_NAME/hls/$timestamp/live.m3u8" -segment_list_flags +live -segment_list_size 5 -segment_time $SEGMENT_DURATION -segment_format mpegts -ar $STREAM_RATE -ac $CHANNELS -threads 3 -acodec aac "/tmp/$NODE_NAME/hls/$timestamp/live%03d.ts" \ | |
| >/tmp/$NODE_NAME/ffmpeg.log 2>&1 & | |
| FFMPEG_PID=$! | |
| else | |
| echo "Unsupported NODE_TYPE. Please use research or hls-only." | |
| exit 1 | |
| fi | |
| # --- 7. CONNECT JACK PORTS --- | |
| echo "Waiting for ffjack ports..." | |
| for i in $(seq 1 15); do | |
| jack_lsp | grep -q "ffjack:input_1" && break | |
| sleep 1 | |
| done | |
| if ! jack_lsp | grep -q "ffjack:input_1"; then | |
| echo "ERROR: ffjack ports never appeared." | |
| cat /tmp/$NODE_NAME/ffmpeg.log | |
| exit 1 | |
| fi | |
| jack_connect -s default system:capture_1 ffjack:input_1 | |
| jack_connect -s default system:capture_2 ffjack:input_2 | |
| if [ "$NODE_LOOPBACK" = "true" ]; then | |
| jack_connect system:capture_1 system:playback_1 | |
| jack_connect system:capture_2 system:playback_2 | |
| fi | |
| # Launch Python uploaders | |
| if [ "${NO_UPLOAD:-false}" = "true" ]; then | |
| echo "NO_UPLOAD=true, skipping S3 upload. Segments will accumulate in /tmp/$NODE_NAME/hls/" | |
| wait $FFMPEG_PID | |
| elif [ "$NODE_TYPE" = "research" ]; then | |
| nice -n 10 python3 catchup_s3.py & | |
| python3 upload_s3.py & | |
| python3 upload_flac_s3.py | |
| else | |
| nice -n 10 python3 catchup_s3.py & | |
| python3 upload_s3.py | |
| fi | |
| echo "All processes started successfully." | |
| #!/bin/bash | |
| # Script for live DASH/HLS streaming lossy audio as AAC and/or archiving lossless audio as FLAC | |
| # Process cleanup on exit/signal | |
| FFMPEG_PID="" | |
| JACK_PID="" | |
| cleanup() { | |
| local code=$? | |
| [ -n "${FFMPEG_PID}" ] && kill -TERM "$FFMPEG_PID" 2>/dev/null || true | |
| [ -n "${JACK_PID}" ] && kill -TERM "$JACK_PID" 2>/dev/null || true | |
| exit $code | |
| } | |
| trap cleanup SIGINT SIGTERM EXIT | |
| # Ensure system binaries are in PATH | |
| export PATH=/usr/bin:/usr/local/bin:/usr/sbin:/bin:$PATH | |
| SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" | |
| # --- 0. PREFLIGHT CHECKS --- | |
| # The .env sourcing is commented out because Docker Compose handles variable injection. | |
| # if [ -f "$SCRIPT_DIR/.env" ]; then | |
| # set -a | |
| # source "$SCRIPT_DIR/.env" | |
| # set +a | |
| # echo "Loaded .env from $SCRIPT_DIR" | |
| # fi | |
| if ! command -v jackd &> /dev/null; then | |
| echo "ERROR: jackd is not installed." | |
| exit 1 | |
| fi | |
| if ! command -v jack_wait &> /dev/null; then | |
| echo "ERROR: jack_wait is not installed." | |
| exit 1 | |
| fi | |
| if ! command -v ffmpeg &> /dev/null; then | |
| echo "ERROR: ffmpeg is not installed." | |
| exit 1 | |
| fi | |
| # --- 1. TIME SYNCHRONIZATION --- | |
| wait_for_sync() { | |
| echo "Checking for sane system time..." | |
| local max_wait=60 | |
| local elapsed=0 | |
| # Check if the year is 2025 or later | |
| while [ $(date +%Y) -lt 2025 ]; do | |
| if [ $elapsed -ge $max_wait ]; then | |
| echo "ERROR: Time sync timed out, aborting." | |
| exit 1 | |
| fi | |
| echo "Waiting for time sync (current year: $(date +%Y))..." | |
| sleep 2 | |
| elapsed=$((elapsed + 2)) | |
| done | |
| echo "Time looks sane: $(date)" | |
| } | |
| wait_for_sync | |
| # --- 2. ACTIVATE VIRTUAL ENVIRONMENT --- | |
| # Updated to use the absolute path within the container. | |
| source /venv/bin/activate | |
| # --- 3. DYNAMIC AUDIO DEVICE DISCOVERY --- | |
| echo "Searching for ALSA device: $AUDIO_HW_ID..." | |
| # Loop to find the hardware index (e.g., card 3) and set the correct hw address. | |
| for i in $(seq 1 30); do | |
| if aplay -l 2>/dev/null | grep -qi "$AUDIO_HW_ID"; then | |
| # Extracts the number from a line like "card 3: pisound [pisound]". | |
| CARD_NUM=$(aplay -l | grep -i "$AUDIO_HW_ID" | head -n 1 | awk -F'[: ]+' '{print $2}') | |
| if [ -n "$CARD_NUM" ]; then | |
| SOUND_CARD="hw:$CARD_NUM,0" | |
| echo "Success! $AUDIO_HW_ID found at index $CARD_NUM. Using address: $SOUND_CARD" | |
| break | |
| fi | |
| fi | |
| echo "Audio device $AUDIO_HW_ID not ready yet, attempt $i..." | |
| sleep 1 | |
| done | |
| if [ -z "$SOUND_CARD" ]; then | |
| echo "ERROR: ALSA device $AUDIO_HW_ID not found after 30s, aborting." | |
| exit 1 | |
| fi | |
| # --- 4. VARIABLES & DIRECTORIES --- | |
| timestamp=$(date +%s) | |
| echo "Node started at $timestamp. Node name: $NODE_NAME. Sound card address: $SOUND_CARD" | |
| mkdir -p /tmp/$NODE_NAME/flac | |
| mkdir -p /tmp/$NODE_NAME/hls/$timestamp | |
| echo $timestamp > /tmp/$NODE_NAME/latest.txt | |
| STREAM_RATE=48000 | |
| SAMPLE_RATE=${SAMPLE_RATE:-48000} | |
| # --- 5. SETUP JACK --- | |
| # Start jackd using the discovered $SOUND_CARD index. | |
| JACK_NO_AUDIO_RESERVATION=1 jackd -t 2000 -P 75 -m -s -d alsa -d $SOUND_CARD -r $SAMPLE_RATE -p 1024 -n 10 & | |
| JACK_PID=$! | |
| echo "Waiting for JACK server to be ready..." | |
| for i in $(seq 1 15); do | |
| jack_wait -w -t 1 && break | |
| if [ $i -eq 15 ]; then | |
| echo "ERROR: JACK failed to start after 15s." | |
| exit 1 | |
| fi | |
| sleep 1 | |
| done | |
| echo "JACK is ready." | |
| # --- 6. FFMPEG STREAMING --- | |
| FFMPEG_PID="" | |
| if [ "$NODE_TYPE" = "research" ]; then | |
| nice -n -10 ffmpeg -f jack -i ffjack \ | |
| -f segment -segment_time "00:00:$FLAC_DURATION.00" -strftime 1 "/tmp/$NODE_NAME/flac/%Y-%m-%d_%H-%M-%S_$NODE_NAME-$SAMPLE_RATE-$CHANNELS.flac" \ | |
| -f segment -segment_list "/tmp/$NODE_NAME/hls/$timestamp/live.m3u8" -segment_list_flags +live -segment_list_size 5 -segment_time $SEGMENT_DURATION -segment_format \ | |
| mpegts -ar $STREAM_RATE -ac 2 -acodec aac "/tmp/$NODE_NAME/hls/$timestamp/live%03d.ts" \ | |
| >/tmp/$NODE_NAME/ffmpeg.log 2>&1 & | |
| FFMPEG_PID=$! | |
| elif [ "$NODE_TYPE" = "hls-only" ]; then | |
| nice -n -10 ffmpeg -f jack -i ffjack -f segment -segment_list "/tmp/$NODE_NAME/hls/$timestamp/live.m3u8" -segment_list_flags +live -segment_list_size 5 -segment_time $SEGMENT_DURATION -segment_format mpegts -ar $STREAM_RATE -ac $CHANNELS -threads 3 -acodec aac "/tmp/$NODE_NAME/hls/$timestamp/live%03d.ts" \ | |
| >/tmp/$NODE_NAME/ffmpeg.log 2>&1 & | |
| FFMPEG_PID=$! | |
| else | |
| echo "Unsupported NODE_TYPE. Please use research or hls-only." | |
| exit 1 | |
| fi | |
| # --- 7. CONNECT JACK PORTS --- | |
| echo "Waiting for ffjack ports..." | |
| for i in $(seq 1 15); do | |
| jack_lsp | grep -q "ffjack:input_1" && break | |
| sleep 1 | |
| done | |
| if ! jack_lsp | grep -q "ffjack:input_1"; then | |
| echo "ERROR: ffjack ports never appeared." | |
| cat /tmp/$NODE_NAME/ffmpeg.log | |
| exit 1 | |
| fi | |
| jack_connect -s default system:capture_1 ffjack:input_1 | |
| jack_connect -s default system:capture_2 ffjack:input_2 | |
| if [ "$NODE_LOOPBACK" = "true" ]; then | |
| jack_connect system:capture_1 system:playback_1 | |
| jack_connect system:capture_2 system:playback_2 | |
| fi | |
| # Launch Python uploaders | |
| if [ "${NO_UPLOAD:-false}" = "true" ]; then | |
| echo "NO_UPLOAD=true, skipping S3 upload. Segments will accumulate in /tmp/$NODE_NAME/hls/" | |
| wait $FFMPEG_PID | |
| elif [ "$NODE_TYPE" = "research" ]; then | |
| nice -n 10 python3 catchup_s3.py & | |
| python3 upload_s3.py & | |
| python3 upload_flac_s3.py | |
| else | |
| nice -n 10 python3 catchup_s3.py & | |
| python3 upload_s3.py | |
| fi | |
| echo "All processes started successfully." |
🧰 Tools
🪛 Shellcheck (0.11.0)
[warning] 7-7: SCRIPT_DIR appears unused. Verify use (or export if used externally).
(SC2034)
[warning] 38-38: Quote this to prevent word splitting.
(SC2046)
[info] 54-54: Not following: /venv/bin/activate was not specified as input (see shellcheck -x).
(SC1091)
[info] 84-84: Double quote to prevent globbing and word splitting.
(SC2086)
[info] 85-85: Double quote to prevent globbing and word splitting.
(SC2086)
[info] 85-85: Double quote to prevent globbing and word splitting.
(SC2086)
[info] 86-86: Double quote to prevent globbing and word splitting.
(SC2086)
[info] 86-86: Double quote to prevent globbing and word splitting.
(SC2086)
[info] 93-93: Double quote to prevent globbing and word splitting.
(SC2086)
[info] 93-93: Double quote to prevent globbing and word splitting.
(SC2086)
[info] 98-98: Double quote to prevent globbing and word splitting.
(SC2086)
[info] 112-112: Double quote to prevent globbing and word splitting.
(SC2086)
[info] 114-114: Double quote to prevent globbing and word splitting.
(SC2086)
[info] 117-117: Double quote to prevent globbing and word splitting.
(SC2086)
[info] 117-117: Double quote to prevent globbing and word splitting.
(SC2086)
[info] 118-118: Double quote to prevent globbing and word splitting.
(SC2086)
[info] 134-134: Double quote to prevent globbing and word splitting.
(SC2086)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@node_2026/stream_sync.sh` around lines 1 - 159, Save the jackd PID
immediately after launching it (capture PID into JACK_PID right after the jackd
start line) and stop relying on implicit background jobs; move FFMPEG_PID
assignment into each NODE_TYPE branch where ffmpeg is started so it's always
set, then add a trap for SIGTERM, SIGINT, and EXIT that explicitly kills and
waits for JACK_PID, FFMPEG_PID and any background python uploader PIDs (e.g.,
catchup_s3, upload_s3, upload_flac_s3) using kill -TERM <PID> and wait <PID>
(avoid kill 0); ensure the trap runs before launching background uploaders and
that all spawned PIDs are recorded so the trap can terminate them cleanly.
Comment on lines
+28
to
+39
| BUCKET = "" | ||
| if "BUCKET_TYPE" in os.environ: | ||
| if(os.environ["BUCKET_TYPE"] == "prod"): | ||
| print("using production bucket") | ||
| BUCKET = 'audio-orcasound-net' | ||
| elif (os.environ["BUCKET_TYPE"] == "custom"): | ||
| print("using custom bucket") | ||
| BUCKET = os.environ["BUCKET_STREAMING"] | ||
| else: | ||
| BUCKET = "dev-streaming-orcasound-net" | ||
|
|
||
| log.debug("hls bucket set to "+BUCKET) |
There was a problem hiding this comment.
Empty BUCKET when BUCKET_TYPE is unset causes unclear S3 failures.
If BUCKET_TYPE is not in the environment, BUCKET remains an empty string, and boto3 uploads will fail with a confusing error. Consider failing fast with a clear message.
Proposed fix
BUCKET = ""
if "BUCKET_TYPE" in os.environ:
if(os.environ["BUCKET_TYPE"] == "prod"):
print("using production bucket")
BUCKET = 'audio-orcasound-net'
elif (os.environ["BUCKET_TYPE"] == "custom"):
print("using custom bucket")
BUCKET = os.environ["BUCKET_STREAMING"]
else:
BUCKET = "dev-streaming-orcasound-net"
log.debug("hls bucket set to "+BUCKET)
+else:
+ log.critical("BUCKET_TYPE not set; exiting")
+ sys.exit(1)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| BUCKET = "" | |
| if "BUCKET_TYPE" in os.environ: | |
| if(os.environ["BUCKET_TYPE"] == "prod"): | |
| print("using production bucket") | |
| BUCKET = 'audio-orcasound-net' | |
| elif (os.environ["BUCKET_TYPE"] == "custom"): | |
| print("using custom bucket") | |
| BUCKET = os.environ["BUCKET_STREAMING"] | |
| else: | |
| BUCKET = "dev-streaming-orcasound-net" | |
| log.debug("hls bucket set to "+BUCKET) | |
| BUCKET = "" | |
| if "BUCKET_TYPE" in os.environ: | |
| if(os.environ["BUCKET_TYPE"] == "prod"): | |
| print("using production bucket") | |
| BUCKET = 'audio-orcasound-net' | |
| elif (os.environ["BUCKET_TYPE"] == "custom"): | |
| print("using custom bucket") | |
| BUCKET = os.environ["BUCKET_STREAMING"] | |
| else: | |
| BUCKET = "dev-streaming-orcasound-net" | |
| log.debug("hls bucket set to "+BUCKET) | |
| else: | |
| log.critical("BUCKET_TYPE not set; exiting") | |
| sys.exit(1) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@node_2026/upload_s3.py` around lines 28 - 39, The BUCKET variable can remain
empty when BUCKET_TYPE is unset causing obscure S3 errors; update upload_s3.py
to validate BUCKET_TYPE/BUCKET after the existing conditional (or when declaring
BUCKET) and fail fast with a clear error message (raise an exception or exit) if
BUCKET_TYPE is missing or BUCKET is still empty; reference the BUCKET and
BUCKET_TYPE environment checks and the BUCKET variable so the change is applied
right after that logic so any boto3 upload code using BUCKET sees a clear, early
failure.
Comment on lines
+116
to
+118
| except: | ||
| e = sys.exc_info()[0] | ||
| log.critical('error uploading to S3: '+str(e)) |
There was a problem hiding this comment.
Bare except: catches KeyboardInterrupt and SystemExit, masking shutdown signals.
The bare except clause captures all exceptions including system signals. This prevents clean container shutdown and hides unexpected errors. Catch a specific exception type instead.
Proposed fix
- except:
- e = sys.exc_info()[0]
- log.critical('error uploading to S3: '+str(e))
+ except Exception as e:
+ log.critical(f'error uploading to S3: {e}')📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| except: | |
| e = sys.exc_info()[0] | |
| log.critical('error uploading to S3: '+str(e)) | |
| except Exception as e: | |
| log.critical(f'error uploading to S3: {e}') |
🧰 Tools
🪛 Ruff (0.15.13)
[error] 116-116: Do not use bare except
(E722)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@node_2026/upload_s3.py` around lines 116 - 118, Replace the bare "except:" in
upload_s3.py with a safe exception handler: catch "Exception as e" instead of
all exceptions so KeyboardInterrupt/SystemExit are not swallowed, and log the
full traceback (for example via log.exception or traceback.format_exc()) when
reporting "error uploading to S3". Target the existing except block that
currently does "except:" / "e = sys.exc_info()[0]" / "log.critical(...)" and
change it to use "except Exception as e" and emit the error details.
Comment on lines
+129
to
+135
| if type_names[0] == 'IN_CLOSE_WRITE': | ||
| if 'tmp' not in filename: | ||
| log.debug('Recieved a new file ' + filename) | ||
| s3_copy_file(path, filename) | ||
| if type_names[0] == 'IN_MOVED_TO': | ||
| log.debug('Recieved a new file ' + filename) | ||
| s3_copy_file(path, filename) |
There was a problem hiding this comment.
IN_MOVED_TO handler lacks the 'tmp' not in filename guard.
The IN_CLOSE_WRITE branch (line 130) excludes temporary files, but IN_MOVED_TO (lines 133-135) processes any moved file. If ffmpeg moves a temp file, it will be uploaded. Apply the same filter for consistency.
Proposed fix
if type_names[0] == 'IN_CLOSE_WRITE':
if 'tmp' not in filename:
- log.debug('Recieved a new file ' + filename)
+ log.debug('Received a new file ' + filename)
s3_copy_file(path, filename)
if type_names[0] == 'IN_MOVED_TO':
- log.debug('Recieved a new file ' + filename)
+ if 'tmp' not in filename:
+ log.debug('Received a new file ' + filename)
s3_copy_file(path, filename)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if type_names[0] == 'IN_CLOSE_WRITE': | |
| if 'tmp' not in filename: | |
| log.debug('Recieved a new file ' + filename) | |
| s3_copy_file(path, filename) | |
| if type_names[0] == 'IN_MOVED_TO': | |
| log.debug('Recieved a new file ' + filename) | |
| s3_copy_file(path, filename) | |
| if type_names[0] == 'IN_CLOSE_WRITE': | |
| if 'tmp' not in filename: | |
| log.debug('Received a new file ' + filename) | |
| s3_copy_file(path, filename) | |
| if type_names[0] == 'IN_MOVED_TO': | |
| if 'tmp' not in filename: | |
| log.debug('Received a new file ' + filename) | |
| s3_copy_file(path, filename) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@node_2026/upload_s3.py` around lines 129 - 135, The IN_MOVED_TO branch
currently uploads every moved file while the IN_CLOSE_WRITE branch skips temp
files; update the IN_MOVED_TO handler to apply the same guard by checking that
'tmp' not in filename before calling s3_copy_file(path, filename). Locate the
block that inspects type_names[0] == 'IN_MOVED_TO' and add the same conditional
used for the IN_CLOSE_WRITE branch (the 'tmp' check and log.debug call) so only
non-temporary files get passed to s3_copy_file.
There was a problem hiding this comment.
Pull request overview
Adds a new node_2026/ implementation of the Raspberry Pi 4 + Pisound hydrophone streaming node that runs the capture/segment/upload pipeline inside Docker, including an outage catch-up uploader and setup/ops documentation.
Changes:
- Introduces a Debian-based Docker image + Compose service to run JACK + ffmpeg + Python uploaders in a container.
- Adds real-time HLS segment uploading with per-segment RMS monitoring (
upload_s3.py). - Adds “catch-up” logic to backfill stranded segments after connectivity returns (
catchup_s3.py), plus host setup and runbook docs.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| node_2026/upload_s3.py | New inotify-based uploader for HLS artifacts with RMS logging and latest.txt publishing. |
| node_2026/catchup_s3.py | New catch-up uploader that scans for stranded segments, generates a VOD manifest, and uploads at low priority. |
| node_2026/stream_sync.sh | Container entrypoint to time-check, discover ALSA device, start JACK/ffmpeg, and launch uploaders. |
| node_2026/docker-compose.yml | Compose service definition (privileged audio access, shm_size, logging limits, env injection). |
| node_2026/Dockerfile | Debian Trixie image installing JACK/ffmpeg and Python deps for the node pipeline. |
| node_2026/setup.sh | One-time host provisioning script (Docker install, IPv6 workaround, group permissions). |
| node_2026/README.txt | Operational documentation for provisioning, running, and troubleshooting the containerized node. |
| node_2026/.env.template | Configuration template for node identity, audio parameters, and AWS credentials. |
| node_2026/.gitignore | Prevents committing local env/log/venv artifacts for this node implementation. |
| node_2026/.dockerignore | Prevents baking env/log artifacts into the Docker build context. |
Comments suppressed due to low confidence (3)
node_2026/upload_s3.py:139
- The
latest.txtpublish is triggered purely by seeing a.tsevent, not by confirming that the first segment upload succeeded. If S3 is unreachable at startup, this can publishlatest.txtpointing clients at a timestamp directory with no uploaded segments. Consider havings3_copy_filereturn success/failure and only uploadinglatest.txt(and flippinglatest_txt_uploaded) after a successful.tsupload.
if not latest_txt_uploaded and filename.endswith('.ts'):
log.debug('First segment uploaded — now publishing latest.txt')
s3_copy_file(BASEPATH, 'latest.txt')
latest_txt_uploaded = True
node_2026/upload_s3.py:105
boto3.resource('s3')is created for every segment upload. Since this runs per-file, consider creating the S3 client/resource once at module init and reusing it to reduce per-segment overhead and connection churn.
try:
resource = boto3.resource('s3')
uploadpath = os.path.relpath(path, "/tmp")
uploadkey = os.path.join(uploadpath, filename)
log.debug('upload key: ' + uploadkey)
try:
resource.meta.client.upload_file(uploadfile, BUCKET, uploadkey)
node_2026/catchup_s3.py:87
enforce_disk_guard()sorts withos.path.getmtime(p)without handlingFileNotFoundError. Since segments can be concurrently deleted/rotated, this can crash the catch-up loop. Consider filtering out missing paths before sorting or wrappinggetmtimein a safe helper.
def enforce_disk_guard(stranded):
"""Delete oldest stranded segments if total exceeds MAX_STRANDED_BYTES."""
# Sort oldest first (by mtime)
aged = sorted(stranded, key=lambda p: os.path.getmtime(p))
while total_size(aged) > MAX_STRANDED_BYTES and aged:
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+28
to
+39
| BUCKET = "" | ||
| if "BUCKET_TYPE" in os.environ: | ||
| if(os.environ["BUCKET_TYPE"] == "prod"): | ||
| print("using production bucket") | ||
| BUCKET = 'audio-orcasound-net' | ||
| elif (os.environ["BUCKET_TYPE"] == "custom"): | ||
| print("using custom bucket") | ||
| BUCKET = os.environ["BUCKET_STREAMING"] | ||
| else: | ||
| BUCKET = "dev-streaming-orcasound-net" | ||
|
|
||
| log.debug("hls bucket set to "+BUCKET) |
Comment on lines
+131
to
+134
| log.debug('Recieved a new file ' + filename) | ||
| s3_copy_file(path, filename) | ||
| if type_names[0] == 'IN_MOVED_TO': | ||
| log.debug('Recieved a new file ' + filename) |
Comment on lines
+4
to
+7
| from boto3.s3.transfer import S3Transfer | ||
| import inotify.adapters | ||
| import logging | ||
| import logging.handlers |
Comment on lines
+151
to
+156
| nice -n 10 python3 catchup_s3.py & | ||
| python3 upload_s3.py & | ||
| python3 upload_flac_s3.py | ||
| else | ||
| nice -n 10 python3 catchup_s3.py & | ||
| python3 upload_s3.py |
| # Detect the actual user (works whether run as root or with sudo) | ||
| REAL_USER=${SUDO_USER:-$USER} | ||
| REAL_HOME=$(eval echo "~$REAL_USER") | ||
| PROJECT_DIR="$REAL_HOME/orcanode/node_val_docker" |
|
|
||
| sudo apt-get install -y git | ||
| git clone https://github.com/orcasound/orcanode.git ~/orcanode | ||
| cd ~/orcanode/node_val_docker |
Comment on lines
+16
to
+19
| ports: | ||
| - "1234:1234" | ||
| - "8080:8080" | ||
|
|
Comment on lines
+28
to
+36
| BUCKET = "" | ||
| if "BUCKET_TYPE" in os.environ: | ||
| if os.environ["BUCKET_TYPE"] == "prod": | ||
| BUCKET = "audio-orcasound-net" | ||
| elif os.environ["BUCKET_TYPE"] == "custom": | ||
| BUCKET = os.environ["BUCKET_STREAMING"] | ||
| else: | ||
| BUCKET = "dev-streaming-orcasound-net" | ||
|
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@node_2026/.env_template`:
- Line 17: The .env_template currently hardcodes a LogDNA ingestion key into the
SYSLOG_STRUCTURED_DATA value, which exposes a secret; replace the real-looking
key with a neutral placeholder (e.g., KEY_HERE or <LOGDNA_KEY>) in the
SYSLOG_STRUCTURED_DATA entry and update documentation to instruct developers to
set the real ingestion key in their untracked .env file or environment
variables; specifically edit the SYSLOG_STRUCTURED_DATA line to remove the
embedded secret and leave a clear placeholder so the real key is never checked
into source control.
In `@node_2026/README.txt`:
- Line 158: The README currently hardcodes the production bucket in verification
commands (e.g., "aws s3 ls s3://audio-orcasound-net/<NODE_NAME>/hls/
--human-readable"); update those commands to use the BUCKET_TYPE/BUCKET_NAME
mechanism instead (e.g., reference the resolved bucket variable or $BUCKET_NAME
built from BUCKET_TYPE=prod|dev|custom) so dev/custom deployments work; replace
every hardcoded "audio-orcasound-net" occurrence (notably the shown command and
the other occurrences mentioned) with the dynamic bucket variable and add a
short note/example showing how BUCKET_NAME is derived from BUCKET_TYPE.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a31f0e53-809f-4bfc-b68c-6ddcb21e268c
📒 Files selected for processing (4)
node_2026/.env_templatenode_2026/.gitignorenode_2026/README.txtnode_2026/stream_sync.sh
✅ Files skipped from review due to trivial changes (1)
- node_2026/.gitignore
| AUDIO_HW_ID=pisound | ||
| CHANNELS=2 | ||
| SYSLOG_URL=syslog://syslog-a.logdna.com:37043 | ||
| SYSLOG_STRUCTURED_DATA='logdna@48950 key="313dbd82f35ccbe462e6e3483984f464" tag="docker"' |
There was a problem hiding this comment.
Remove the hardcoded LogDNA ingestion key from the template.
This embeds a real-looking secret in a tracked file. Keep only a placeholder and inject the real value via an untracked .env.
Suggested fix
-SYSLOG_STRUCTURED_DATA='logdna@48950 key="313dbd82f35ccbe462e6e3483984f464" tag="docker"'
+SYSLOG_STRUCTURED_DATA='logdna@48950 key="REPLACE_WITH_LOGDNA_INGESTION_KEY" tag="docker"'📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| SYSLOG_STRUCTURED_DATA='logdna@48950 key="313dbd82f35ccbe462e6e3483984f464" tag="docker"' | |
| SYSLOG_STRUCTURED_DATA='logdna@48950 key="REPLACE_WITH_LOGDNA_INGESTION_KEY" tag="docker"' |
🧰 Tools
🪛 Betterleaks (1.2.0)
[high] 17-17: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@node_2026/.env_template` at line 17, The .env_template currently hardcodes a
LogDNA ingestion key into the SYSLOG_STRUCTURED_DATA value, which exposes a
secret; replace the real-looking key with a neutral placeholder (e.g., KEY_HERE
or <LOGDNA_KEY>) in the SYSLOG_STRUCTURED_DATA entry and update documentation to
instruct developers to set the real ingestion key in their untracked .env file
or environment variables; specifically edit the SYSLOG_STRUCTURED_DATA line to
remove the embedded secret and leave a clear placeholder so the real key is
never checked into source control.
|
|
||
| If NO_UPLOAD=false, verify segments are reaching S3: | ||
|
|
||
| aws s3 ls s3://audio-orcasound-net/<NODE_NAME>/hls/ --human-readable |
There was a problem hiding this comment.
Avoid hardcoding the prod bucket in verification commands.
The README supports BUCKET_TYPE=prod|dev|custom, but these commands always point to audio-orcasound-net, which is incorrect for dev/custom deployments.
Suggested doc fix
- aws s3 ls s3://audio-orcasound-net/<NODE_NAME>/hls/ --human-readable
+ aws s3 ls s3://<BUCKET_NAME>/<NODE_NAME>/hls/ --human-readable
...
- s3://audio-orcasound-net/<NODE_NAME>/hls/<timestamp>/live000.ts
+ s3://<BUCKET_NAME>/<NODE_NAME>/hls/<timestamp>/live000.ts
...
- aws s3 cp s3://audio-orcasound-net/<NODE_NAME>/hls/<timestamp>/live.m3u8 -
+ aws s3 cp s3://<BUCKET_NAME>/<NODE_NAME>/hls/<timestamp>/live.m3u8 -Also applies to: 161-161, 166-166
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@node_2026/README.txt` at line 158, The README currently hardcodes the
production bucket in verification commands (e.g., "aws s3 ls
s3://audio-orcasound-net/<NODE_NAME>/hls/ --human-readable"); update those
commands to use the BUCKET_TYPE/BUCKET_NAME mechanism instead (e.g., reference
the resolved bucket variable or $BUCKET_NAME built from
BUCKET_TYPE=prod|dev|custom) so dev/custom deployments work; replace every
hardcoded "audio-orcasound-net" occurrence (notably the shown command and the
other occurrences mentioned) with the dynamic bucket variable and add a short
note/example showing how BUCKET_NAME is derived from BUCKET_TYPE.
Updated AWS credentials placeholders in .env_template
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
node_2026/.env_template (1)
6-9:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd BUCKET_STREAMING placeholder for custom bucket configuration.
When
BUCKET_TYPE=custom,upload_s3.py(line 33) expectsBUCKET_STREAMINGto be defined. This template lacks that variable, which would cause aKeyErrorat runtime if someone uncomments or addsBUCKET_TYPE=custom.📝 Suggested addition
BUCKET_TYPE=prod +#BUCKET_STREAMING=your-custom-bucket-name `#NODE_TYPE`=researchAs per context snippet 1 from upload_s3.py:14-40, the uploader code branches on BUCKET_TYPE and reads BUCKET_STREAMING when the type is "custom".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@node_2026/.env_template` around lines 6 - 9, Add a BUCKET_STREAMING placeholder to the environment template so code that expects it when BUCKET_TYPE=custom won't KeyError; update the .env_template to include a commented example like BUCKET_STREAMING=your-streaming-bucket (alongside the existing BUCKET_TYPE and NODE_TYPE entries) so upload_s3.py's branch that reads BUCKET_STREAMING has a clear default/example to follow.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@node_2026/.env_template`:
- Around line 6-9: Add a BUCKET_STREAMING placeholder to the environment
template so code that expects it when BUCKET_TYPE=custom won't KeyError; update
the .env_template to include a commented example like
BUCKET_STREAMING=your-streaming-bucket (alongside the existing BUCKET_TYPE and
NODE_TYPE entries) so upload_s3.py's branch that reads BUCKET_STREAMING has a
clear default/example to follow.
Switch hls_list_size from 5 to 0 so live.m3u8 grows to include every segment written since the last container start. Add a midnight crontab in setup.sh to restart the container daily, giving each calendar day its own S3 timestamp directory with a complete manifest. This makes all segments discoverable by the orcasite server for spectrogram generation. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
When hls_list_size was 5, live.m3u8 only retained the last 5 segments of each session, leaving earlier segments undiscoverable by the orcasite server. repair_manifests.py scans all S3 timestamp directories, compares .ts file count against manifest entries, and reconstructs a complete live.m3u8 with approximate EXT-X-PROGRAM-DATE-TIME tags derived by anchoring on the exact timestamps still present in the tail of the old manifest. README_REPAIR.md documents the algorithm and usage. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@node_2026/README_REPAIR.md`:
- Line 7: Add explicit language identifiers to the fenced code blocks in
README_REPAIR.md to satisfy markdownlint MD040: change the fence before the
"s3://audio-orcasound-net/rpi_orcasound_lab/hls/1748383745/" snippet to ```text,
the fence before the "time(N) ≈ anchor_time + (N − anchor_seq) × avg_duration"
equation to ```text, the fence for the "python3 repair_manifests.py [--node
NODE]..." command to ```bash, and the fence for the "Scanning
s3://audio-orcasound-net/rpi_orcasound_lab/hls/ (dry_run=False)..." output to
```text (apply the same pattern to the other occurrences noted at the end of the
review).
In `@node_2026/repair_manifests.py`:
- Around line 293-317: Filter out non-timestamp prefixes (where
_dir_unix(prefix) == 0) immediately after calling list_timestamp_dirs and before
computing most_recent, reversing, and applying stop-at logic; update the code
around vars dirs, most_recent and the stop_at check so you discard entries with
_dir_unix(...) == 0 (or otherwise non-numeric) and then compute most_recent =
dirs[-1] if dirs else None, reverse dirs, and run the stop-at comparison using
the numeric _dir_unix(prefix) value to avoid non-timestamp entries becoming
most_recent or prematurely triggering the stop-at break; keep references to
list_timestamp_dirs, _dir_unix, most_recent, dirs, stop_at, and process_dir.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: bbfd3456-8124-4a5b-8d31-e7be632e61d6
📒 Files selected for processing (2)
node_2026/README_REPAIR.mdnode_2026/repair_manifests.py
|
|
||
| Each recording session creates a timestamp directory on S3 such as: | ||
|
|
||
| ``` |
There was a problem hiding this comment.
Add language identifiers to fenced code blocks to satisfy markdownlint.
These fences currently trigger MD040 and may fail markdown linting in CI.
💡 Suggested patch
-```
+```text
s3://audio-orcasound-net/rpi_orcasound_lab/hls/1748383745/@@
-
time(N) ≈ anchor_time + (N − anchor_seq) × avg_duration
@@
- +bash
python3 repair_manifests.py [--node NODE] [--bucket BUCKET]
[--days N] [--stop-at TIMESTAMP]
[--dry-run]
@@
-```
+```text
Scanning s3://audio-orcasound-net/rpi_orcasound_lab/hls/ (dry_run=False)
Found 47 directories to examine
...
Done. fixed=44 ok=2 skip=1 error=0
</details>
Also applies to: 48-48, 81-81, 113-113
<details>
<summary>🧰 Tools</summary>
<details>
<summary>🪛 markdownlint-cli2 (0.22.1)</summary>
[warning] 7-7: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
</details>
</details>
<details>
<summary>🤖 Prompt for AI Agents</summary>
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @node_2026/README_REPAIR.md at line 7, Add explicit language identifiers to
the fenced code blocks in README_REPAIR.md to satisfy markdownlint MD040: change
the fence before the
"s3://audio-orcasound-net/rpi_orcasound_lab/hls/1748383745/" snippet to text, the fence before the "time(N) ≈ anchor_time + (N − anchor_seq) × avg_duration" equation to text, the fence for the "python3 repair_manifests.py [--node
NODE]..." command to ```bash, and the fence for the "Scanning
s3://audio-orcasound-net/rpi_orcasound_lab/hls/ (dry_run=False)..." output to
review).
Comment on lines
+293
to
+317
| dirs = list_timestamp_dirs(s3, args.bucket, args.node) | ||
| if not dirs: | ||
| print("No timestamp directories found.") | ||
| sys.exit(0) | ||
|
|
||
| # Optionally limit to recent N days | ||
| if args.days > 0: | ||
| cutoff = datetime.now(tz=timezone.utc) - timedelta(days=args.days) | ||
| cutoff_unix = int(cutoff.timestamp()) | ||
| dirs = [d for d in dirs | ||
| if _dir_unix(d) >= cutoff_unix] | ||
|
|
||
| print(f"Found {len(dirs)} director{'y' if len(dirs)==1 else 'ies'} to examine\n") | ||
|
|
||
| most_recent = dirs[-1] if dirs else None | ||
| dirs = list(reversed(dirs)) | ||
| counts = {'fixed': 0, 'ok': 0, 'skip': 0, 'error': 0} | ||
|
|
||
| for prefix in dirs: | ||
| ts_str = prefix.rstrip('/').split('/')[-1] | ||
| if args.stop_at and _dir_unix(prefix) < args.stop_at: | ||
| print(f" Reached stop-at threshold ({args.stop_at}), stopping.") | ||
| break | ||
| is_live = (prefix == most_recent) | ||
| status, msg = process_dir(s3, args.bucket, prefix, is_live, args.dry_run, s3_write) |
There was a problem hiding this comment.
Filter non-timestamp prefixes before ordering and stop-at logic.
_dir_unix() returns 0 for non-numeric prefixes; with current flow, those can become most_recent or trip the stop-at break early, skipping valid directories.
💡 Suggested patch
- dirs = list_timestamp_dirs(s3, args.bucket, args.node)
- if not dirs:
+ dirs = list_timestamp_dirs(s3, args.bucket, args.node)
+ dirs = [d for d in dirs if _dir_unix(d) > 0]
+ dirs.sort(key=_dir_unix)
+ if not dirs:
print("No timestamp directories found.")
sys.exit(0)
@@
- most_recent = dirs[-1] if dirs else None
+ most_recent = dirs[-1]
dirs = list(reversed(dirs))
@@
- if args.stop_at and _dir_unix(prefix) < args.stop_at:
+ ts = _dir_unix(prefix)
+ if args.stop_at and ts < args.stop_at:
print(f" Reached stop-at threshold ({args.stop_at}), stopping.")
break🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@node_2026/repair_manifests.py` around lines 293 - 317, Filter out
non-timestamp prefixes (where _dir_unix(prefix) == 0) immediately after calling
list_timestamp_dirs and before computing most_recent, reversing, and applying
stop-at logic; update the code around vars dirs, most_recent and the stop_at
check so you discard entries with _dir_unix(...) == 0 (or otherwise non-numeric)
and then compute most_recent = dirs[-1] if dirs else None, reverse dirs, and run
the stop-at comparison using the numeric _dir_unix(prefix) value to avoid
non-timestamp entries becoming most_recent or prematurely triggering the stop-at
break; keep references to list_timestamp_dirs, _dir_unix, most_recent, dirs,
stop_at, and process_dir.
dthaler
reviewed
Jun 17, 2026
|
|
||
| FROM debian:trixie | ||
|
|
||
| LABEL maintainer="Orcasound <orcanode-devs@orcasound.net>" |
Contributor
There was a problem hiding this comment.
Who does orcanode-devs@orcasound.net currently go to?
Comment on lines
+74
to
+75
| NODE_NAME=rpi_orcasound_lab # unique name for this node | ||
| NODE_TYPE=hls-only # hls-only or research |
Contributor
There was a problem hiding this comment.
Suggested change
| NODE_NAME=rpi_orcasound_lab # unique name for this node | |
| NODE_TYPE=hls-only # hls-only or research | |
| NODE_NAME=rpi_orcasound_lab # unique name for this node | |
| NODE_TYPE=hls-only # hls-only or research |
nit: align spacing
| After the first build, Docker manages the container automatically: | ||
| - restart: always — restarts on crash without any action needed | ||
| - Docker enabled at boot — container starts on every reboot | ||
| - crontab (installed by setup.sh) restarts at midnight each night so |
Contributor
There was a problem hiding this comment.
Is this still true that it restarts at midnight each night?
| SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" | ||
|
|
||
| # --- 0. PREFLIGHT CHECKS --- | ||
| # The .env sourcing is commented out because Docker Compose handles variable injection. |
Contributor
There was a problem hiding this comment.
I didn't follow this, but do we need to keep these lines at all or should they be removed?
| BASEPATH = os.path.join("/tmp", NODE) | ||
| PATH = os.path.join(BASEPATH, "hls") | ||
|
|
||
| # REGION = os.environ["REGION"] |
Contributor
There was a problem hiding this comment.
Should line 18 be removed?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This adds
node_2026/, a new self-contained implementation of the hydrophone streaming node targeting Raspberry Pi 4 with a Pisound HAT. It runs the full audio stack — JACK, ffmpeg, and the Python uploaders — inside a Docker container, replacing the previous approach of installing those dependencies directly on the host OS.The existing
node/directory is untouched. This is a parallel implementation for evaluation.What's different from the existing node
Fully containerized
setup.shhandles thatAutomatic recovery
restart: always+ Docker enabled at boot replaces the systemd serviceInternet outage recovery (
catchup_s3.py)catchup_s3.pygenerates a VOD manifest (catchup.m3u8) for the gap and uploads all stranded segments at low priority (nice -n 10, 2s between uploads) so catch-up does not compete with the live streamJACK in Docker fixes
shm_size: 256m— Docker's default 64 MB/dev/shmis too small for JACK's shared memoryjackd -mflag disables memory locking (mlock fails inside containers regardless of ulimit settings)RMS monitoring
upload_s3.pycomputes and logs RMS level for every.tssegmentTest mode
NO_UPLOAD=truein.envruns the full pipeline (JACK → ffmpeg → HLS segments) without touching S3, for validating a new node before going liveLog rotation
Files
Dockerfiledocker-compose.ymlstream_sync.shupload_s3.pycatchup_s3.pysetup.sh.env.template.envand fill in credentialsREADME.txtTesting
Tested on Raspberry Pi 4 (4 GB) running Raspberry Pi OS Trixie with a Pisound HAT. Verified:
docker compose restartand after rebootNO_UPLOAD=truemode accumulates segments locally without errorsHow to evaluate
Summary by CodeRabbit
New Features
Documentation
Chores