-
Notifications
You must be signed in to change notification settings - Fork 5
Pluggable backends and Cloudflare Live Stream backend addition #53
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
r0d8lsh0p
wants to merge
32
commits into
v0l:main
Choose a base branch
from
r0d8lsh0p:pluggable-backends-and-cloudflare-stream
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Pluggable backends and Cloudflare Live Stream backend addition #53
r0d8lsh0p
wants to merge
32
commits into
v0l:main
from
r0d8lsh0p:pluggable-backends-and-cloudflare-stream
Conversation
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
- Protects docker-compose.override.yaml from git tracking - Protects compose-config.local.yaml and other .local files - Ensures local customizations survive all git operations
- Add backend and cloudflare fields to OverseerConfig in settings.rs - Add backend selection logic in main.rs (both backends point to existing implementation) - Add CloudflareSettings struct for future use - Update testing documentation with Rust unit tests section - Add Docker detached mode guidance for AI assistants - All tests passing: Rust unit tests + Docker integration test
- Add definitive method using 'grep generated.*warning' - Explain why 7 vs 18 warning count confusion happened - Provide step-by-step baseline comparison process - Document common mistakes previous AIs made - Allow for exceptions with user explanation This resolves confusion where AIs counted warnings differently: - First AI: saw 7 (only looked at one package summary) - Later AI: saw 18 (counted all warning lines including summaries) - Correct: Use per-package 'generated X warnings' comparison
- Create API discovery bash script to test real Cloudflare responses - Add .env.example for secure credential management - Update .gitignore to protect .env files and test output - Add comprehensive README with setup and usage instructions - Script tests Live Input creation, retrieval, and streaming behavior - Captures all JSON responses for documentation - Finds actual HLS URL location (currently undocumented) This enables empirical API testing before implementing Step 3A
CRITICAL DISCOVERY from API testing: - Live Input response does NOT contain HLS playback URLs - NO created.uid field (created is just a timestamp, not an object) - Unvalidated third-party documentation was WRONG What DOES exist in Live Input response: ✅ webRTCPlayback.url - WebRTC playback (<1s latency) ✅ rtmpsPlayback.url + streamKey - RTMPS playback ✅ srtPlayback.url + streamId + passphrase - SRT playback Implications for Step 3A: - Cannot use result.playback.hls (doesn't exist) - Must use WebRTC playback URL OR - Find where HLS URLs come from (Videos API? Active stream only?) - May need to rethink playback strategy Test data saved in scripts/cloudflare-api-responses-*/
✅ PROOF: End-to-end test confirms architecture
- Created Live Input via API
- Started actual ffmpeg stream
- Polled Videos API with liveInput filter
- Video Asset appeared on 1st attempt (immediate)
- Extracted HLS URL from asset.playback.hls
- Verified HLS URL accessible (HTTP 200)
- Downloaded and validated HLS manifest
Key Findings:
✅ Asset created IMMEDIATELY after streaming starts
✅ HLS URL at: GET /stream?liveInput={uid} -> result[0].playback.hls
✅ Manifest contains adaptive bitrate (720p, 480p, 360p)
✅ Architecture validated: advisor was 100% CORRECT
Implementation proven with real API + real streaming + real HLS playback
Test script: scripts/test-hls-discovery.sh
Documentation: notes/Cloudflare-Live-Stream-API-docs.md
…237) - Created backends/ module structure for peer-based backend organization - Moved RmlRtmpBackend to backends/rml_rtmp.rs - Updated streaming_backend.rs to contain only trait and shared types - All imports updated accordingly - Zero functional changes - pure code organization refactor - All tests pass (15/15 unit tests, Docker integration verified) - Stream START and END both tested successfully - No new warnings introduced Updated documentation: - Added Git Workflow Check section to prevent unauthorized branch creation - Split testing checklist into separate START and END verification steps - Added new mistakes to Common AI Mistakes table - Improved clarity to prevent future AI errors This refactor prepares the codebase for CloudflareBackend implementation by establishing backends as architectural peers rather than hierarchical.
- Created CloudflareClient HTTP wrapper for Cloudflare Stream API - Implemented CloudflareBackend with StreamingBackend trait - Added mockito dev-dependency for HTTP mocking - Added 5 unit tests validating CloudflareClient methods - All tests pass (20/20) Note: End-to-end streaming requires Step 3B (webhooks) to detect stream start/end events
…ms then make segment processing optional for local backend only and remove cruft dummy ingest
…l and streamkey at time of stream to stream to existing users keys
…needs more intensive cf testing
…e and use npub in name of cf dashboard
- Add StreamManager to RmlRtmpBackend constructor - Implement get_viewer_count() using StreamManager - Move StreamManager initialization earlier in overseer to support RML backend - Maintain symmetry with Cloudflare backend's approach - All unit tests pass, Docker integration tests pass (stream start/end verified) Issue #240
- Add StreamInfo struct to store live_input_uid + HLS URL together - Update live_input_cache to use StreamInfo (unified data storage) - Implement get_viewer_count() using cached HLS URL - Transform HLS URL to Cloudflare /views endpoint - No authentication required for viewer count API - Graceful fallback to 0 on errors - Cache HLS URLs after first fetch (eliminates repeated 60s polling) Testing: - Rust unit tests: 25 passed - E2E test: 11/11 passed (webhooks, streaming, Nostr events) - Verified in logs: 'Cloudflare viewer count for stream...0' Issue #240
…tidy up other webhook connection and logs
…g in the upstream repo about ordering of database operations
Owner
|
Going to refactor this branch dont push any more changes pls |
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.
Hi Kieran,
I hope you are well.
This PR introduces a backend-plugin architecture to Zap Stream Core, along with an optional Cloudflare Live Stream backend. The goal is to make ingest/output providers pluggable while keeping the existing RML RTMP engine fully intact and unchanged as the default.
What this PR does
StreamingBackendtrait defining ingest/output behaviourtos_url,alt_tag)/scriptsKey objectives
Architecture overview
The design principle is separation of concerns:
streaming_backendmodule defines the required behaviour (generate_stream_key, get_recording_url, etc)backends/rml_rtmpandbackends/cloudflareprovide concrete implementationsoverseerthen defers to the selected backend without knowing implementation detailsoverseer.rs,api.rs,http.rs, andmain.rsThis keeps the codebase open for additional backends in the future (e.g. API.Video, SRS, whatever).
Notes
You’ve had quite a bit of activity in the repo recently — I’m ~21 commits behind. If you’re interested in this direction, I’m happy to further commit to update the PR to the latest
mainbranch and adjust structure/naming to match your preferences.Happy to iterate
If you'd prefer this split into smaller PRs or want to discuss any parts of it, just let me know. I'm happy to adjust based on your review style.
Thanks for your interest in taking a look!