Fix PR issues: API returns, HEVC fragmentation, SDP, debug default - #3
Open
favourthemaster wants to merge 3 commits into
Open
Fix PR issues: API returns, HEVC fragmentation, SDP, debug default#3favourthemaster wants to merge 3 commits into
favourthemaster wants to merge 3 commits into
Conversation
- H264Transcoder: Add PATH env var for ffmpeg discovery - SnapshotManager: Add PATH env var for ffmpeg discovery - WebServer: Bind to 0.0.0.0 (http://*:port) instead of specific IP - Remove duplicate src/Program.cs (conflicted with root Program.cs) - Enable debug logging by default - Add frame dump instrumentation for debugging - Native AOT compatible publish configuration
There was a problem hiding this comment.
Pull request overview
This PR aims to make the project work reliably on Linux by hardening process execution (ffmpeg discovery), adjusting HTTP server binding behavior, and extending streaming/diagnostics (RTSP H.265 handling and frame-dump instrumentation).
Changes:
- Force a known
PATHfor ffmpeg invocations in both snapshot decoding and transcoding. - Rework RTSP to better handle H.265 (SDP advertising + session sending changes).
- Add frame dump instrumentation and enable debug logging by default.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| src/WebServer.cs | Web server startup + endpoint mapping; intended to improve binding/endpoint behavior. |
| src/V380Client.cs | Adds frame dump instrumentation and richer frame logging; adds a new H.265 frame handling path. |
| src/SnapshotManager.cs | Ensures ffmpeg can be located by setting PATH for availability checks. |
| src/H264Transcoder.cs | Ensures ffmpeg can be located by setting PATH for the transcoder process. |
| src/RtspServer.cs | Expands SDP generation to advertise H.265 and caches HEVC parameter sets. |
| src/RtspSession.cs | Adds H.265 packetization logic for RTSP interleaved RTP over TCP. |
| src/LogUtils.cs | Enables debug logging by default. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+104
to
+108
| app.MapPost("/api/ptz/right", () => { client.PtzRight(); LogUtils.debug("[API] PTZ Right"); Results.Ok(); }); | ||
| app.MapPost("/api/ptz/left", () => { client.PtzLeft(); LogUtils.debug("[API] PTZ Left"); Results.Ok(); }); | ||
| app.MapPost("/api/ptz/up", () => { client.PtzUp(); LogUtils.debug("[API] PTZ Up"); Results.Ok(); }); | ||
| app.MapPost("/api/ptz/down", () => { client.PtzDown(); LogUtils.debug("[API] PTZ Down"); Results.Ok(); }); | ||
| app.MapPost("/api/ptz/stop", () => { client.PtzStop(); LogUtils.debug("[API] PTZ Stop"); Results.Ok(); }); |
Comment on lines
+110
to
+112
| app.MapPost("/api/light/on", () => { client.LightOn(); LogUtils.debug("[API] Light On"); Results.Ok(); }); | ||
| app.MapPost("/api/light/off", () => { client.LightOff(); LogUtils.debug("[API] Light Off"); Results.Ok(); }); | ||
| app.MapPost("/api/light/auto", () => { client.LightAuto(); LogUtils.debug("[API] Light Auto"); Results.Ok(); }); |
Comment on lines
+114
to
+117
| app.MapPost("/api/image/color", () => { client.ImageColor(); LogUtils.debug("[API] Image Color"); Results.Ok(); }); | ||
| app.MapPost("/api/image/bw", () => { client.ImageBW(); LogUtils.debug("[API] Image B&W"); Results.Ok(); }); | ||
| app.MapPost("/api/image/auto", () => { client.ImageAuto(); LogUtils.debug("[API] Image Auto"); Results.Ok(); }); | ||
| app.MapPost("/api/image/flip", () => { client.ImageFlip(); LogUtils.debug("[API] Image Flip"); Results.Ok(); }); |
Comment on lines
+45
to
+55
| try | ||
| { | ||
| frameDumpEnabled = File.Exists("/home/favour/camera/frame_dump.bin"); | ||
| if (frameDumpEnabled) | ||
| { | ||
| frameDump = new StreamWriter(new FileStream("/home/favour/camera/frame_dump.log", FileMode.Append, FileAccess.Write, FileShare.Read)); | ||
| frameDump.AutoFlush = true; | ||
| LogUtils.debug("[DUMP] frame dump ENABLED - writing to /home/favour/camera/frame_dump.bin and .log"); | ||
| } | ||
| } | ||
| catch { } |
Comment on lines
+432
to
436
| finally | ||
| { | ||
| frameDump?.Dispose(); | ||
| } | ||
| } |
Comment on lines
+526
to
+529
| var binPath = "/home/favour/camera/frame_dump.bin"; | ||
| using var bfs = new FileStream(binPath, FileMode.Append, FileAccess.Write, FileShare.Read); | ||
| bfs.Write(full, 0, full.Length); | ||
| } |
Comment on lines
+126
to
+135
| uint rts = f.Timestamp > 0 ? (uint)(f.Timestamp * 90) : (generatedVideoTimestamp += 3600); | ||
| byte[] payload = f.Payload; | ||
| if (f.Codec == VideoCodec.H265) { | ||
| byte[] paramSets = server.GetHevcParameterSets(); | ||
| if (paramSets != null) { | ||
| payload = new byte[paramSets.Length + payload.Length]; | ||
| Buffer.BlockCopy(paramSets, 0, payload, 0, paramSets.Length); | ||
| Buffer.BlockCopy(f.Payload, 0, payload, paramSets.Length, f.Payload.Length); | ||
| } | ||
| } |
Comment on lines
+175
to
+194
| void SendHevcFragmented(byte[] nal, uint rts, int mtu) { | ||
| byte nalHdr = nal[0]; | ||
| byte fuInd = (byte)((nalHdr & 0x81) | 49); | ||
| int offset = 1; | ||
| bool first = true; | ||
| while (offset < nal.Length) { | ||
| int chunk = Math.Min(mtu - 2, nal.Length - offset); | ||
| bool last = offset + chunk >= nal.Length; | ||
| byte fuHdr = (byte)((nalHdr >> 1) & 0x3F); | ||
| if (first) fuHdr |= 0x80; | ||
| if (last) fuHdr |= 0x40; | ||
| var frag = new byte[2 + chunk]; | ||
| frag[0] = fuInd; | ||
| frag[1] = fuHdr; | ||
| Array.Copy(nal, offset, frag, 2, chunk); | ||
| SendRtp(videoCh, 97, videoSeq++, rts, videoSsrc, frag, 0, frag.Length, marker: last); | ||
| offset += chunk; | ||
| first = false; | ||
| } | ||
| } |
Comment on lines
+144
to
+145
| "m=video 0 RTP/AVP 96 97\r\n" + | ||
| rtpmap + |
| public class LogUtils | ||
| { | ||
| public static bool enableDebug = false; | ||
| public static bool enableDebug = true; |
favourthemaster
changed the title
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
Fixes all issues raised in PR review:
Fixed Issues
API endpoints return IResult explicitly (
src/WebServer.cs:104-117)MapPosthandlers nowreturn Results.Ok()instead of just calling itHEVC FU-A fragmentation per RFC 7798 (
src/RtspSession.cs:175-200)0x31(F=0, Type=49)FuType | S(0x80) | E(0x40)fuIndcalculationSDP conditional payload type 97 (
src/RtspServer.cs:119-155)m=videoline when HEVC params availablertpmapfor H265 only added when HEVC params existDebug logging opt-in (
src/LogUtils.cs:10)enableDebug = falseby default--debugCLI flagFrame dump reuse across reconnects (
src/V380Client.cs:434)frameDumpon reconnect - it's reusedAdditional Context
Hw_HsAkQQFC_4G_BP5_20230421(different from issue Patch that gets video working on my model #2'sHsAKQQCC)Testing
All endpoints verified working:
rtsp://localhost:8555/live✅http://localhost:8080/snapshot✅http://localhost:8080/stream.mjpg✅http://localhost:8080/✅