Skip to content

Commit fa348af

Browse files
authored
Merge pull request #146 from OpenIPC/fix/udp-salvage
fix: U cell / udp:// slots — lossy pushed stream decoded instead of staying black (#133)
2 parents ea5c093 + c9d3685 commit fa348af

6 files changed

Lines changed: 133 additions & 7 deletions

File tree

‎android/app/src/main/java/com/openipc/decoder/Decoder.java‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1402,6 +1402,10 @@ private Map<String, Object> debugState() {
14021402
c.put("lost_fps", cell.lostFps());
14031403
// NAL units dropped because one of their packets was lost (#113)
14041404
c.put("nal_dropped", cell.nalDropped());
1405+
// Slices passed on cut short at a lost piece (some may still be dropped by
1406+
// a full queue); absent while that is off (#133)
1407+
int salvaged = cell.nalSalvaged();
1408+
if (salvaged >= 0) c.put("nal_damaged", salvaged);
14051409
// Every frame, or key frames only while the decoder has no room (#85)
14061410
c.put("decode", cell.isKeyFramesOnly() ? "key frames" : "full");
14071411
if (cell.knowsLoad()) c.put("load", cell.isKeyFramesOnly()
@@ -1715,13 +1719,20 @@ public void run() {
17151719
int frames = videoDecoder.framesOut() + jpegFramesOut;
17161720
long bytes = client != null ? client.getStreamBytes() : 0;
17171721
boolean stalled = !quadEnabled && !udpViewShown && client != null
1718-
&& client.playsOverUdp() && "connected".equals(singleState)
1722+
&& (client.playsOverUdp() || client.listens())
1723+
&& "connected".equals(singleState)
17191724
&& bytes - lastSingleBytes > 10_000 && frames == lastSingleFrames;
17201725
lastSingleFrames = frames;
17211726
lastSingleBytes = bytes;
17221727
singleStallSeconds = stalled ? singleStallSeconds + 1 : 0;
17231728
if (singleStallSeconds >= UDP_STALL_SECONDS) {
17241729
singleStallSeconds = 0;
1730+
// Every key frame loses a piece: decode them damaged rather than not at all;
1731+
// a pushed stream has no TCP to go to (#133)
1732+
if (!nalAssembler.salvaging()) {
1733+
nalAssembler.setSalvage(true);
1734+
Log.w(TAG, "No complete key frame over UDP, decoding damaged ones");
1735+
}
17251736
client.reportUdpBroken("packets but no decoded frame for " + UDP_STALL_SECONDS
17261737
+ " s");
17271738
}

‎android/app/src/main/java/com/openipc/decoder/NalAssembler.java‎

Lines changed: 41 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,13 @@ interface Sink {
4444
private int dropped;
4545
/** Of those, dropped because a packet of them was lost (not for size) (#113). */
4646
private int lostDropped;
47+
/**
48+
* Pass a NAL unit with a lost piece on cut short at the gap instead of dropping it (#133):
49+
* for a stream that loses a piece of every key frame, where dropping them leaves nothing to
50+
* decode. The decoder conceals the missing part; a damaged picture beats a black one.
51+
*/
52+
private volatile boolean salvage;
53+
private int salvaged;
4754

4855
NalAssembler(int bufferSize, Runnable onCodecSwitch, FramePool framePool) {
4956
this.nalBuffer = new byte[bufferSize];
@@ -63,6 +70,11 @@ void reset() {
6370
/** NAL units dropped because one of their packets was lost (#113). */
6471
int lossCount() { return lostDropped; }
6572

73+
void setSalvage(boolean on) { salvage = on; }
74+
boolean salvaging() { return salvage; }
75+
/** Slices passed on to the sink cut short at a lost piece (#133); the queue may drop some. */
76+
int salvagedCount() { return salvaged; }
77+
6678
/**
6779
* Process one RTP packet ({@code length} bytes at {@code offset}, header included).
6880
* Returns false if the RTP header is malformed.
@@ -89,7 +101,7 @@ boolean assemble(byte[] buf, int offset, int length, boolean codecH265, Sink sin
89101
lastSeq = seq;
90102
if (gap && inFragment) {
91103
lostDropped++;
92-
drop("packet loss inside a fragmented NAL unit");
104+
dropOrSalvage("packet loss inside a fragmented NAL unit", sink);
93105
orphan = true; // its remaining pieces belong to the unit just counted
94106
}
95107

@@ -104,7 +116,7 @@ boolean assemble(byte[] buf, int offset, int length, boolean codecH265, Sink sin
104116
// Any other packet ends a fragmented NAL unit that did not finish; its remaining
105117
// pieces, if any arrive, belong to the unit counted here
106118
if (inFragment) {
107-
drop("fragmented NAL unit interrupted by another packet");
119+
dropOrSalvage("fragmented NAL unit interrupted by another packet", sink);
108120
orphan = true;
109121
} else {
110122
orphan = false;
@@ -131,7 +143,7 @@ private void fragmentUnit(byte[] buf, int start, int end, boolean h265, Sink sin
131143
if (first) {
132144
if (inFragment) {
133145
lostDropped++;
134-
drop("fragmented NAL unit without its end");
146+
dropOrSalvage("fragmented NAL unit without its end", sink);
135147
}
136148
orphan = false;
137149
nalSize = 0;
@@ -201,6 +213,32 @@ private void emit(byte[] buf, int pos, int size, Sink sink) {
201213
sink.onNal(out);
202214
}
203215

216+
/** The unit collected so far, cut short, when salvaging (#133); dropped otherwise. */
217+
private void dropOrSalvage(String reason, Sink sink) {
218+
// Start code and NAL header plus some slice data, or there is nothing to decode; only
219+
// slices: a cut parameter set would be taken for a valid configuration (Qodo on #146)
220+
if (!salvage || nalSize < 16 || !isSlice(nalBuffer[4], lastCodec == 1)) {
221+
drop(reason);
222+
return;
223+
}
224+
Frame out = framePool.obtain(nalSize);
225+
System.arraycopy(nalBuffer, 0, out.data(), 0, nalSize);
226+
out.setLength(nalSize);
227+
nalSize = 0;
228+
inFragment = false;
229+
if (salvaged++ % 100 == 0) {
230+
Log.w(TAG, "NAL unit passed on cut short (" + salvaged + "): " + reason);
231+
}
232+
sink.onNal(out);
233+
}
234+
235+
/** A coded slice (VCL NAL unit): H.264 types 1–5, H.265 types 0–31. */
236+
static boolean isSlice(byte nalHeader, boolean h265) {
237+
if (h265) return ((nalHeader >> 1) & 0x3F) <= 31;
238+
int type = nalHeader & 0x1F;
239+
return type >= 1 && type <= 5;
240+
}
241+
204242
private void drop(String reason) {
205243
nalSize = 0;
206244
inFragment = false;

‎android/app/src/main/java/com/openipc/decoder/QuadCell.java‎

Lines changed: 25 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -274,6 +274,9 @@ void stop() {
274274
public void onStatus(String status) {
275275
// A new connection (or a lost one) starts the UDP stall count afresh (Qodo on #125)
276276
if (!status.equals(state)) udpStallSamples = 0;
277+
// A pushed stream that went quiet may come back whole: no damaged decoding until it
278+
// stalls again (Qodo on #146)
279+
if ("disconnected".equals(status)) nalAssembler.setSalvage(false);
277280
state = status;
278281
// A connection that works again clears the last connection error
279282
if ("connected".equals(status) && problem != null && !problem.startsWith("Decoder")) {
@@ -346,9 +349,15 @@ String sample(long nowMs) {
346349
// MJPEG has no key frames: its cheap mode shows one frame per second (#132)
347350
return size + codec + (jpeg ? " reduced" : " key frames") + " · " + rate + " fps"
348351
+ ((jpeg ? jpegRenderer.isSoftware() : decoder.isSoftware())
349-
? " · software" : "");
352+
? " · software" : "") + damaged();
350353
}
351-
return size + codec + " " + Math.round(outFps) + "/" + Math.round(inFps) + " fps";
354+
return size + codec + " " + Math.round(outFps) + "/" + Math.round(inFps) + " fps"
355+
+ damaged();
356+
}
357+
358+
/** Corner note while damaged key frames are decoded (#133). */
359+
private String damaged() {
360+
return !jpeg && nalAssembler.salvaging() ? " · damaged" : "";
352361
}
353362

354363
boolean isKeyFramesOnly() { return keyFramesOnly; }
@@ -421,7 +430,9 @@ private void checkUdpStall() {
421430
lastVideoPackets = packets;
422431
lastLossCount = loss;
423432
Surface s = surface;
424-
boolean stalled = session.playsOverUdp() && "connected".equals(state)
433+
// A pushed stream (udp:// slot, the U cell) counts too: no TCP to fall back to (#133)
434+
boolean stalled = (session.playsOverUdp() || session.listens())
435+
&& "connected".equals(state)
425436
&& pausedReason == null && problem == null && s != null && s.isValid()
426437
&& arriving && outFps == 0;
427438
if (!stalled) {
@@ -433,6 +444,12 @@ private void checkUdpStall() {
433444
if (losing || udpStallSamples > 0) udpStallSamples++;
434445
if (udpStallSamples >= UDP_STALL_SAMPLES) {
435446
udpStallSamples = 0;
447+
// Every key frame loses a piece: decode them damaged rather than not at all. A
448+
// pushed stream has no TCP to go to; over RTSP, Auto goes to TCP anyway (#133)
449+
if (!jpeg && !nalAssembler.salvaging()) {
450+
nalAssembler.setSalvage(true);
451+
Log.w(TAG, tag + ": no complete key frame over UDP, decoding damaged ones");
452+
}
436453
session.reportUdpBroken("packets but no decoded frame for " + UDP_STALL_SAMPLES
437454
+ " s (" + loss + (jpeg ? " JPEG frames" : " NAL units") + " lost)");
438455
}
@@ -441,6 +458,9 @@ private void checkUdpStall() {
441458
/** NAL units dropped so far because a packet of them was lost (#113). */
442459
int nalDropped() { return nalAssembler.lossCount(); }
443460

461+
/** Slices passed on cut short at a lost piece, or -1 while that is off (#133). */
462+
int nalSalvaged() { return nalAssembler.salvaging() ? nalAssembler.salvagedCount() : -1; }
463+
444464
float inFps() { return inFps; }
445465
float outFps() { return outFps; }
446466
float lostFps() { return lostFps; }
@@ -517,6 +537,8 @@ public void onSdp(Sdp sdp) {
517537
lastLossCount = sdp.videoJpeg ? jpegAssembler.droppedCount() : nalAssembler.lossCount();
518538
udpStallSamples = 0;
519539
nalAssembler.reset();
540+
// A new stream gets a chance to be complete again (#133)
541+
nalAssembler.setSalvage(false);
520542
jpegAssembler.reset();
521543
lastUnknownPayload = -1;
522544
videoPt = sdp.videoPt;

‎android/app/src/main/java/com/openipc/decoder/RtspClient.java‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,8 @@ void configure(String host, String userAgent, Transport.Mode transport) {
7575
boolean isActive() { return session.isActive(); }
7676
/** RTP over UDP or multicast at the moment (#128). */
7777
boolean playsOverUdp() { return session.playsOverUdp(); }
78+
/** A udp:// slot listening for a pushed stream (#133). */
79+
boolean listens() { return session.listens(); }
7880
/** Packets arrive but nothing decodes: with Auto the session goes over to TCP. */
7981
void reportUdpBroken(String why) { session.reportUdpBroken(why); }
8082
/** Transport the stream really uses now, or null (#115). */
@@ -101,13 +103,17 @@ void stop() {
101103

102104
@Override
103105
public void onStatus(String status) {
106+
// A pushed stream that went quiet may come back whole (Qodo on #146)
107+
if ("disconnected".equals(status)) nalAssembler.setSalvage(false);
104108
Listener l = listenerCb;
105109
if (l != null) l.onStatusChanged(status);
106110
}
107111

108112
@Override
109113
public void onSdp(Sdp sdp) {
110114
nalAssembler.reset();
115+
// A new stream gets a chance to be complete again (#133)
116+
nalAssembler.setSalvage(false);
111117
lastUnknownPayload = -1;
112118
lastRtpTimestamp = -1;
113119
lastRtpArrivalNs = -1;

‎android/app/src/main/java/com/openipc/decoder/RtspSession.java‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,9 @@ private void markUdpFailed(Transport.Mode mode) {
140140
/** RTSP is playing with RTP over UDP or multicast (not TCP, not a udp:// URL). */
141141
boolean playsOverUdp() { return udpPlaying; }
142142

143+
/** A udp:// slot listening for a pushed stream (#133): UDP too, without RTSP. */
144+
boolean listens() { return listening; }
145+
143146
/**
144147
* Packets arrive over UDP but the stream cannot be decoded (#113: key frames keep losing
145148
* a piece). With Auto or Multicast the session goes over to TCP, as when no video comes

‎android/app/src/test/java/com/openipc/decoder/NalAssemblerTest.java‎

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,52 @@ public void lostMiddleFragmentDropsTheNal() {
138138
assertArrayEquals(b(0, 0, 0, 1, 0x65, 0xD1, 0xD2), out.get(0));
139139
}
140140

141+
@Test
142+
public void salvagePassesTheNalOnCutShortAtTheGap() {
143+
nal.setSalvage(true);
144+
// 16 slice bytes before the gap: enough to be worth decoding (#133)
145+
feed(rtp(0x7C, 0x85, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12), false);
146+
feed(rtp(0x7C, 0x05, 13, 14, 15, 16), false);
147+
seq++; // one packet lost
148+
feed(rtp(0x7C, 0x05, 0xB1), false);
149+
assertEquals(1, out.size());
150+
assertArrayEquals(b(0, 0, 0, 1, 0x65, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15,
151+
16), out.get(0));
152+
assertEquals(1, nal.salvagedCount());
153+
assertEquals(1, nal.lossCount());
154+
// The rest of that unit is skipped; the next one is complete again
155+
feed(rtp(0x7C, 0x45, 0xC1), false);
156+
feed(rtp(0x7C, 0x85, 0xD1), false);
157+
feed(rtp(0x7C, 0x45, 0xD2), false);
158+
assertEquals(2, out.size());
159+
assertArrayEquals(b(0, 0, 0, 1, 0x65, 0xD1, 0xD2), out.get(1));
160+
}
161+
162+
@Test
163+
public void salvageNeverPassesACutParameterSet() {
164+
nal.setSalvage(true);
165+
// FU-A of an SPS (type 7), then a lost packet (Qodo on #146)
166+
feed(rtp(0x7C, 0x87, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16), false);
167+
seq++;
168+
feed(rtp(0x7C, 0x47, 0xC1), false);
169+
assertEquals(0, out.size());
170+
assertEquals(0, nal.salvagedCount());
171+
assertTrue(NalAssembler.isSlice((byte) 0x65, false));
172+
assertFalse(NalAssembler.isSlice((byte) 0x68, false));
173+
// H.265: IDR_W_RADL (19) is a slice, VPS (32) is not
174+
assertTrue(NalAssembler.isSlice((byte) (19 << 1), true));
175+
assertFalse(NalAssembler.isSlice((byte) (32 << 1), true));
176+
}
177+
178+
@Test
179+
public void salvageOffDropsAsBefore() {
180+
feed(rtp(0x7C, 0x85, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16), false);
181+
seq++;
182+
feed(rtp(0x7C, 0x45, 0xC1), false);
183+
assertEquals(0, out.size());
184+
assertEquals(0, nal.salvagedCount());
185+
}
186+
141187
@Test
142188
public void continuationWithoutStartIsIgnoredAndCountedOnce() {
143189
feed(rtp(0x7C, 0x05, 0xB1), false);

0 commit comments

Comments
 (0)