Repository navigation
feat: Jieli HF toy drones over CTP (#187) - #198
Merged
Merged
Conversation
jieli://192.168.80.1 for toys with an HF_xxxxxxxx Wi-Fi (Jieli AC79, vendor app DVRunning 2), after the PCAPdroid capture in #196 and turbodrone's research notes (Apache-2.0, behaviour reference only, no code taken over): - CTP on TCP 3333 (CTP: + u16 topic length + topic + u32 JSON length + JSON), a reader that reassembles split and joined messages and skips what is not CTP - APP_ACCESS, DATE_TIME, CTP_KEEP_ALIVE every min(timeout / 4, 7.5 s), OPEN_RT_STREAM with the size of VIDEO_PARAM (else 640x480 at 25), CLOSE_RT_STREAM on stop - video expected on the phone's UDP 6666 (unverified): PT 26 into the RTP/JPEG path, PT 96 into the H.264 path, whole JPEG datagrams too; first packets logged with sender, PT and bytes; no video for 5 s: said in the status, UDP 1234 and 2224 opened for the log only, OPEN_RT_STREAM again every 10 s - options w, h, fps, rtp, format; sockets bound to the toy's network (TCP from Android 6) - jieli object in /stats.json; subnet hints both ways between flowufo:// and jieli:// - never FLIGHT_CONTROL, nothing to UDP 3456 - docs/toys.md: HF / Jieli section, credits; unit tests with a fake toy on loopback
Collaborator
Author
|
/review |
Code Review by Qodo
1. Bad packets keep the toy marked as live
|
![qodo-free-for-open-source-projects[bot]](https://avatars.githubusercontent.com/in/1068000?s=60&v=4){kind=small}
… video (Qodo on #198) - a KEEP_ALIVE_INTERVAL announced after the first keepalive applies at once: the next one is due interval ms after the last one sent, not on the old schedule; test with the announcement after the first keepalive - the quiet status from the CTP thread and "connected" from the UDP thread are serialised, and a quiet notice is dropped when video arrived meanwhile
Collaborator
Author
|
Qodo findings addressed in 2eacdfa: (1) the keepalive is now due one interval after the last one sent, so a KEEP_ALIVE_INTERVAL announced later applies at once (new test |
…g from #188 with the shared network lookup)
PR Summary by QodoAdd video-only Jieli HF toy drone slots over CTP
AI Description
Diagram
High-Level Assessment
Files changed (13)
|
Comment on lines
+1012
to
+1013
| lastVideoAt = now(); | ||
| listener.onRtp(b, 0, len); |
There was a problem hiding this comment.
1. Bad packets keep the toy marked as live 🐞 Bug ≡ Correctness
video() updates lastVideoAt for any toy-origin packet with RTP version bits and payload type 26 or 96 before the decoder decides whether the packet is usable. If the toy repeatedly sends malformed RTP, the assemblers reject it while the slot reports receiving and neither its quiet-state logic nor its stream-reopen logic runs.
Agent Prompt
## Issue description
Malformed RTP refreshes Jieli video liveness even when the decoder rejects it, preventing quiet-state reporting and stream reopen.
## Fix Focus Areas
- android/app/src/main/java/com/openipc/decoder/Jieli.java[1005-1019]
- android/app/src/main/java/com/openipc/decoder/RtspSession.java[784-809]
## Recommended Fix
Have the media listener report whether the downstream callback accepted a packet or JPEG frame. Refresh `lastVideoAt` only for accepted media; retain separate counters for packets that arrived but were unusable.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+780
to
+784
| for (int p : diagnosticPorts) { | ||
| if (p == url.rtpPort) continue; | ||
| try { | ||
| startThread(openUdp(p, a), false, listener); | ||
| } catch (IOException e) { |
There was a problem hiding this comment.
2. Silent toy slots block other udp streams 🐞 Bug ≡ Correctness
Client.run() opens diagnostic listeners on UDP 1234 and 2224 after five seconds without video and keeps their sockets open until the Jieli session ends. In a grid, another cell configured to listen on either port cannot bind its unicast socket for as long as the silent toy slot remains active.
Agent Prompt
## Issue description
Diagnostic UDP listeners retain ports used by other grid streams for the entire silent Jieli session.
## Fix Focus Areas
- android/app/src/main/java/com/openipc/decoder/Jieli.java[774-789]
- android/app/src/main/java/com/openipc/decoder/Jieli.java[846-887]
## Recommended Fix
Give diagnostic listeners a bounded observation period or packet limit, then close their sockets while retaining the collected counts and first-packet details in stats.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+932
to
+933
| Integer c = topics.get(m.topic); | ||
| topics.put(m.topic, c == null ? 1 : c + 1); |
There was a problem hiding this comment.
3. Toy messages can exhaust phone memory 🐞 Bug ☼ Reliability
handle() adds every distinct CTP topic to topics without a size limit, although only individual message lengths are bounded. A peer sending a continuing series of valid messages with different topic names grows both the retained map and the copy made for each /stats.json request without bound.
Agent Prompt
## Issue description
A connected CTP peer can cause unlimited growth in the topic statistics map by sending distinct valid topics.
## Fix Focus Areas
- android/app/src/main/java/com/openipc/decoder/Jieli.java[929-937]
- android/app/src/main/java/com/openipc/decoder/Jieli.java[1075-1079]
## Recommended Fix
Cap the number of distinct topics retained and aggregate further unseen topics into an overflow count, so stats snapshots remain bounded too.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+817
to
+818
| }); | ||
| return true; |
There was a problem hiding this comment.
4. Toy reconnects wait up to 30 s after a dropout 🐞 Bug ☼ Reliability
Jieli.Client.run() only returns normally when the slot is stopped. A closed CTP connection or CTP silence ends it with an IOException instead, so jieli() never reaches return true and networkLoop never sets played, which means the back-off is never reset. If the toy was off for a few attempts at first (the delay doubles up to the 30 s maximum) and then streamed fine, every later dropout still waits about 30 s before reconnecting, and each one counts as another failure through onFailure, even though the comment says the method behaves "like a PLAY that succeeded".
Agent Prompt
## Issue description
A jieli:// session that connected and requested the stream still ends with an IOException when the toy drops the connection. Because of that, RtspSession.networkLoop never sees `played == true` and never resets the retry back-off (up to 30 s).
## Fix Focus Areas
- android/app/src/main/java/com/openipc/decoder/RtspSession.java[756-827]
- android/app/src/main/java/com/openipc/decoder/Jieli.java[690-838]
## Recommended Fix
Add a flag on Jieli.Client, e.g. `volatile boolean streamRequested`, and set it when OPEN_RT_STREAM is first sent. In RtspSession.jieli(), wrap `c.run(...)` in a try/catch on IOException. If `c.streamRequested` is true, log the error (and call callback.onFailure if you want), then return true so the back-off resets. Otherwise rethrow.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
|
Code review by qodo was updated up to the latest commit d094871 |
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
2 participants
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.
Refs #187, #196. Into
feat/flowufo-187(PR #188), to be merged there later. Draft until a device test on anHF_…toy.What it does
A new slot type
jieli://for toy drones with anHF_xxxxxxxxWi‑Fi (Jieli AC79 camera, vendor app DVRunning 2), in single view and grid cells like the other toy slots. Video only:FLIGHT_CONTROLis never sent, nothing goes to the toy's UDP 3456.192.168.80.1:3333):CTP:+ topic length u16 LE + topic + JSON length u32 LE + JSON (may be empty). A stream reader reassembles split/joined messages and skips bytes that are not CTP.APP_ACCESS {"ver":"25","type":"0"}→ (waits up to 1.5 s forVIDEO_PARAM) →DATE_TIME(phone time,yyyyMMddHHmmss) →CTP_KEEP_ALIVEevery min(announced timeout / 4, 7.5 s) →OPEN_RT_STREAM {"format":"0","h":…,"w":…,"fps":…}with the size fromVIDEO_PARAM(640×480@25 if none).CLOSE_RT_STREAM {"status":"1"}when the slot stops (also onRtspSession.stop(): the session thread sends it before closing). CTP silence beyond max(20 s, announced timeout) or a closed connection reconnects through the normal back-off.OPEN_RT_STREAM, socket bound to the toy's network). RTP PT 26 → existing RTP/JPEG path (JpegAssembler, RFC 2435); PT 96 → existing H.264 RTP path (as onudp://slots; the slot follows the PT the toy actually sends); a whole JPEG in a datagram without RTP → MJPEG frame path. Only packets from the toy's address are played; others are counted and logged.OPEN_RT_STREAM→ log + status No video on UDP 6666 yet…, and UDP 1234 (SDK audio port) and 2224 (SDK RT stream port) are opened for the log only (first 5 packets each, counts in stats).OPEN_RT_STREAMis repeated every 10 s without video.jieli://[host][:ctp-port][?w=&h=&fps=&rtp=&format=0|1], defaults192.168.80.1:3333, rtp 6666, format 0.jieliobject per cell and insingle:state,target,network,hint,ctp(sent/received, keepalives/replies,keepalive_ms,announced_timeout_ms,open_requests,topicscounts,last_notifiesincl.VIDEO_PARAM),video_param,requested,rtp(port, network, packets, bytes, video_packets, frames (marker bits), payload_types, not_rtp, from_others, first_packet),sources,diagnostic_ports,last_error. CTP messages are logged (first 60, then every 100th; keepalives only the first two; JSON cut at 200 chars).jieli://slot on a 192.168.169.x / 192.168.1.x phone suggestsflowufo:/// the KY UFO RTSP URL; aflowufo://slot on a 192.168.80.x phone now suggestsjieli://192.168.80.1(instead of "not supported yet").NetInfo.bindToNetworkOf(Socket, …)for the CTP socket (Android 6+), sharing the network lookup with the UDP variant.jieli://slots; transport label "Jieli RTP".Tests
JieliTest: CTP encode (exact bytes ofCTP_KEEP_ALIVE {"op":"PUT"}, empty JSON), reader byte-by-byte / joined / split across a length field / garbage and bad lengths / a large message, JSON reader, keepalive interval, URL options and errors, subnet hints, and a fake CTP toy on loopback: APP_ACCESS → DATE_TIME → CTP_KEEP_ALIVE → OPEN_RT_STREAM with the VIDEO_PARAM size, keepalives 400–900 ms apart at timeout 2000, RTP/JPEG on a loopback UDP port producing the expected JPEG byte for byte, CLOSE_RT_STREAM on stop and on close(), 640×480@25 without VIDEO_PARAM, no-video diagnostics port + repeated OPEN_RT_STREAM, unreachable toy.Not verified (no HF toy here)
OPEN_RT_STREAMneeds anything else first (e.g. the vendor app's UDP 345600packets, which we never send) or a local SDP server (TCP 6789 in the SDK; not offered).What a tester should send
App version/hash, phone and Android version; with the slot
jieli://192.168.80.1trying for ~30 s:http://127.0.0.1:8085/stats.json(thejieliobject) andhttp://127.0.0.1:8085/log(lines withJieli).