add TODOS

This commit is contained in:
henryruhs
2026-04-30 15:36:14 +02:00
parent 6047463154
commit f6ad98ba6c
4 changed files with 140 additions and 0 deletions
+76
View File
@@ -0,0 +1,76 @@
# RTC TODO
## Approach
1. vibe code mass testing to get broad coverage fast
2. vibe code refactoring (naming, dead code, restructure)
3. hand craft production code (libdatachannel, peer management, SDP)
4. hand craft testing — thin it out to essentials only
## Unit Tests
| Function | Rating | Assertions |
|---|---|---|
| `create_peer_connection` | essential | returns valid peer connection id |
| `add_audio_track` | essential | returns valid track id |
| `add_video_track` | essential | returns valid track id |
| `negotiate_sdp` | essential | returns sdp answer from valid offer, returns None on timeout |
| `delete_peers` | essential | clears peer list |
| `create_rtc_stream` | essential | initializes empty peer list |
| `destroy_rtc_stream` | essential | cleans up peers, handles missing session |
| `add_rtc_viewer` | nice to have | returns None for unknown session |
| `resolve_binary_file` | skip | covered implicitly by `create_static_rtc_library` |
| `handle_whep_offer` | skip | thin wrapper, covered by integration tests |
| `send_to_peers` | skip | needs open tracks, covered by integration tests |
| `send_rtc_frame` | skip | thin delegation to `send_to_peers` |
## Integration Tests
- [ ] test full SDP offer/answer roundtrip between two peer connections
- [ ] test `send_to_peers` delivers frame data to a connected peer
- [ ] test peer cleanup after `delete_peers` prevents further sends
- [ ] test `add_rtc_viewer` followed by `send_rtc_frame` end-to-end
- [ ] test multiple viewers receive frames from same stream
- [ ] test viewer disconnect does not break remaining peers
## Dead Code
- [ ] remove `is_peer_connected` from `rtc.py`, never called
- [ ] remove `pre_check` from `rtc.py`, never called
- [ ] remove `get_rtc_stream` from `rtc_store.py`, never called
- [ ] remove `RtcOfferSet` from `types.py`, never used
## Naming
- [ ] rename `rtc_bindings.py` to `libdatachannel.py` — owns C library loading, struct definitions, ctypes registration
- [ ] rename `RTC_CONFIGURATION` to a proper class definition
- [ ] rename `RTC_PACKETIZER_INIT` to a proper class definition
- [ ] rename `init_ctypes` to something more specific like `register_argtypes`
## Unused Library Config
- [ ] `create_peer_connection` exposes 14 params but production only uses `disable_auto_negotiation`, `enable_ice_udp_mux`, `force_media_transport` — reduce to what is needed
- [ ] `RTC_PACKETIZER_INIT` defines `nalSeparator`, `obuPacketization`, `playoutDelayId`, `playoutDelayMin`, `playoutDelayMax`, `sequenceNumber`, `timestamp` — none are set outside the struct definition, they are H264/AV1 specific and unused for VP8/Opus
- [ ] `rtcSetLocalDescription` is used in tests but not registered in `rtc_bindings.py`
## Refactor
- [ ] extract shared media description builder from `rtc.py` and `tests/stream_helper.py` (see TODO in `tests/stream_helper.py`)
- [ ] replace `type()` calls for ctypes structs in `rtc_bindings.py` with proper class definitions
- [ ] move `resolve_binary_file`, `create_static_download_set`, `create_static_rtc_library` from `rtc.py` into `libdatachannel.py` — library init belongs with the bindings, `rtc.py` just consumes it
- [ ] move `rtc_store.py` state into a proper store pattern consistent with other `*_store.py` files
- [ ] replace `time.sleep` polling loops in `negotiate_sdp` and `create_sdp_offer` (test helper) with `rtcSetGatheringStateChangeCallback` — signal a `threading.Event` on ICE gathering complete, then `event.wait(timeout=5)` instead of spinning
- [ ] `create_static_download_set` has hardcoded linux URLs with a TODO to use dynamic `binary_name`
## Violations
- `rtc.py:25` — comment on `create_static_download_set` (no comments)
- `rtc.py:34` — comment on hash url (no comments)
- `rtc.py:42` — comment on source url (no comments)
- `rtc.py:205``send_to_peers` returns None explicitly on a void function (redundant)
- `rtc_bindings.py:3,23``type()` to create structs is a class workaround (no classes, but these should be plain structs)
- `tests/stream_helper.py:45``sdp` temp variable before return (no need for temporary variable in simple cases)
## Security
- `rtc.py:157``negotiate_sdp` passes unsanitized `sdp_offer` string directly to C library, no SDP format validation before hitting native code
+60
View File
@@ -0,0 +1,60 @@
# Stream API TODO
## Approach
1. vibe code mass testing to get broad coverage fast
2. vibe code refactoring (naming, boilerplate extraction, validation)
3. hand craft production code (stream pipeline, encoder loop, IVF parsing)
4. hand craft testing — thin it out to essentials only
## Unit Tests
| Function | Rating | Assertions |
|---|---|---|
| `get_websocket_stream_mode` | essential | returns None for missing header, returns None for unknown protocol |
| `forward_rtc_frames` | essential | reads IVF header and forwards frame data, handles broken pipe |
| `run_encode_loop` | nice to have | drains deque and closes encoder |
| `receive_vision_frames` | skip | async generator, covered by integration tests |
| `submit_encoder_frame` | skip | thin glue between cv2 and subprocess stdin |
| `websocket_stream` | skip | routing only, covered by integration tests |
| `post_stream` | skip | covered by integration tests |
## Integration Tests
- [ ] test stream without session — expect rejection
- [ ] test stream with expired or invalid token
- [ ] test image stream without source selected
- [ ] test video stream without source selected
- [ ] test WHEP offer without active websocket stream
- [ ] test WHEP offer with malformed SDP body
- [ ] test WHEP offer with wrong content type
- [ ] test multiple WHEP viewers on same stream
- [ ] test websocket disconnect mid-stream triggers cleanup
## Naming
- [ ] rename `test_get_stream_mode` to `test_get_websocket_stream_mode` in `test_stream_helper.py` — does not match function name
- [ ] rename `make_scope` in `test_stream_helper.py` to `make_websocket_scope` — more descriptive
- [ ] `stream_helper.py` mixes pure helpers (`calculate_bitrate`) with async handlers (`handle_image_stream`) — consider splitting
## Dead Code
- [ ] `read_pipe_buffer` has a test but the test is disconnected from how it is actually used — the test reads from a closed pipe, production reads from a live subprocess stdout
## Refactor
- [ ] `handle_image_stream` and `handle_video_stream` share session setup boilerplate (subprotocol, access_token, session_id, source_paths, websocket accept) — extract common setup
- [ ] `forward_rtc_frames` assumes IVF container format with hardcoded header size 32 and frame header size 12 — document or make configurable
- [ ] `post_stream` does not validate content type is `application/sdp` before parsing body
- [ ] `calculate_bitrate` has a TODO about improving the calculation
- [ ] `handle_video_stream` has a hardcoded fallback `output_video_fps or 30` with a TODO to resolve from target video fps
## Violations
- `stream_helper.py:24` — comment on `calculate_bitrate` (no comments)
- `stream_helper.py:143` — comment on `output_video_fps` fallback (no comments)
- `test_stream_helper.py:30``test_get_stream_mode` does not match function name `get_websocket_stream_mode` (naming convention)
## Security
- `endpoints/stream.py:27``request.body().decode()` is not sanitized, raw user input forwarded to C library via RTC layer
+3
View File
@@ -10,6 +10,7 @@ from facefusion import rtc
from facefusion.types import RtcSdpOffer
# TODO: reuse media description building from rtc.py
def create_sdp_offer() -> Optional[RtcSdpOffer]:
rtc_library = rtc.create_static_rtc_library()
peer_connection = rtc.create_peer_connection(disable_auto_negotiation = True)
@@ -43,7 +44,9 @@ def create_sdp_offer() -> Optional[RtcSdpOffer]:
if rtc_library.rtcGetLocalDescription(peer_connection, buffer_string, buffer_size) > 0:
sdp = buffer_string.value.decode()
rtc_library.rtcDeletePeerConnection(peer_connection)
#TODO: use return buffer_string.value.decode()
return sdp
time.sleep(0.05)
rtc_library.rtcDeletePeerConnection(peer_connection)
+1
View File
@@ -132,6 +132,7 @@ def test_stream_video(test_client : TestClient) -> None:
ready_event = threading.Event()
stop_event = threading.Event()
#TODO: use asyncio
stream_thread = threading.Thread(target = open_websocket_stream, args = (test_client, [ 'access_token.' + access_token, 'video' ], source_content, ready_event, stop_event))
stream_thread.start()
ready_event.wait()