Skip to content

Commit a0e9809

Browse files
committed
fix: Qodo findings on #84 — decoder failures shown, limits respected, slots freed late
- A decoder that fails to start is reported to the cell ('Decoder error') instead of showing normal zero-fps stats - While a codec's instance limit is unknown, its decoders start one at a time, so concurrent cells cannot exceed the limit before it is learnt (Android 5 and errors count as unlimited) - The instance slot is given back only after the codec is released, so a waiting cell cannot start while the device still holds the maximum - A decoder limit message is not replaced by a connection error - Only playing cells are demoted or paused (#85 planner)
1 parent 189afda commit a0e9809

5 files changed

Lines changed: 65 additions & 16 deletions

File tree

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

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,8 @@ static final class Cell {
4040
long fullLoad;
4141
/** The decoder lost frames of this cell in the last sample. */
4242
boolean lossy;
43+
/** Connected and decoding: only such cells are demoted or paused (Qodo on #84). */
44+
boolean playing = true;
4345

4446
Cell(int index, boolean full, boolean known, long fullLoad, boolean lossy) {
4547
this.index = index;
@@ -119,7 +121,7 @@ Decision step(List<Cell> cells, long nowMs) {
119121
lossySamples = 0;
120122
for (int i = cells.size() - 1; i >= 0; i--) {
121123
Cell c = cells.get(i);
122-
if (c.full && !c.paused) {
124+
if (c.full && !c.paused && c.playing && c.known) {
123125
// Losses at this load: the capacity is below it, but not below a proven load
124126
capacity = Math.max(proven, Math.round(load * 0.9));
125127
learnt = true;
@@ -129,7 +131,9 @@ Decision step(List<Cell> cells, long nowMs) {
129131
// Even key frames are too much: pause the lowest-priority cell as a last resort
130132
for (int i = cells.size() - 1; i >= 0; i--) {
131133
Cell c = cells.get(i);
132-
if (!c.paused && countActive(cells) > 1) return change(nowMs, Action.PAUSE, c.index);
134+
if (!c.paused && c.playing && countActive(cells) > 1) {
135+
return change(nowMs, Action.PAUSE, c.index);
136+
}
133137
}
134138
return Decision.NONE;
135139
}

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -983,6 +983,7 @@ private static DecodePlanner.Cell plannerCell(int index, QuadCell c, long now) {
983983
DecodePlanner.Cell pc = new DecodePlanner.Cell(index, !c.isKeyFramesOnly(),
984984
c.knowsLoad(), c.knowsLoad() ? c.fullLoad() : 0, lossy);
985985
pc.paused = c.isPaused();
986+
pc.playing = c.isPlaying();
986987
return pc;
987988
}
988989

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

Lines changed: 30 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,8 @@ static final class InstanceBudget {
9999
static synchronized boolean tryAcquire(String mime) {
100100
int u = used.containsKey(mime) ? used.get(mime) : 0;
101101
Integer max = limit.get(mime);
102-
if (max != null && u >= max) return false;
102+
// Until the first decoder of a type has told its limit, start one at a time
103+
if (max == null ? u > 0 : u >= max) return false;
103104
used.put(mime, u + 1);
104105
return true;
105106
}
@@ -131,18 +132,28 @@ static synchronized void reset() {
131132
*/
132133
static volatile long declaredBlocksPerSec;
133134

134-
/** Give the instance slot back; called wherever the decoder is detached (decoderLock held). */
135-
private void releaseSlot() {
136-
if (heldMime != null) {
137-
InstanceBudget.release(heldMime);
138-
heldMime = null;
139-
}
135+
/**
136+
* Detach the instance slot from this manager (decoderLock held). The caller gives it back
137+
* with {@link InstanceBudget#release} only after the codec itself is released, so another
138+
* cell cannot start a decoder while the device still holds the maximum.
139+
*/
140+
private String takeSlot() {
141+
String m = heldMime;
142+
heldMime = null;
143+
return m;
144+
}
145+
146+
private static void releaseSlot(String mime) {
147+
if (mime != null) InstanceBudget.release(mime);
140148
}
141149

142150
/** Learn the codec's instance limit and log its declared throughput, once per type. */
143151
private static void learnLimits(MediaCodec codec, String mime) {
144-
if (Build.VERSION.SDK_INT < Build.VERSION_CODES.M) return;
145152
if (InstanceBudget.limit(mime) >= 0) return;
153+
if (Build.VERSION.SDK_INT < Build.VERSION_CODES.M) {
154+
InstanceBudget.learn(mime, Integer.MAX_VALUE); // not declared before Android 6
155+
return;
156+
}
146157
try {
147158
MediaCodecInfo.CodecCapabilities caps = codec.getCodecInfo().getCapabilitiesForType(mime);
148159
int max = caps.getMaxSupportedInstances();
@@ -160,6 +171,7 @@ private static void learnLimits(MediaCodec codec, String mime) {
160171
Log.i(TAG, "Decoder " + codec.getName() + " (" + mime + "): up to " + max
161172
+ " instances, declared 1080p frame rate in total " + fps1080);
162173
} catch (Exception e) {
174+
InstanceBudget.learn(mime, Integer.MAX_VALUE);
163175
Log.w(TAG, "Decoder limits unknown: " + e.getMessage());
164176
}
165177
}
@@ -187,13 +199,15 @@ void setEnabled(boolean enabled) {
187199
final MediaCodec target = decoder;
188200
if (target == null) return;
189201
new Thread(() -> {
202+
String slot;
190203
synchronized (decoderLock) {
191204
if (decoder != target) return; // already replaced or released
192205
decoder = null;
193-
releaseSlot();
206+
slot = takeSlot();
194207
retry.onSuccess();
195208
}
196209
releaseNow(target);
210+
releaseSlot(slot);
197211
}, "decoder-release").start();
198212
}
199213

@@ -215,14 +229,15 @@ void closeDecoder() {
215229
lastFrameMs = 0;
216230
if (codec == null) return;
217231
decoder = null;
218-
releaseSlot();
232+
String slot = takeSlot();
219233
Log.i(TAG, "Close video decoder");
220234
try {
221235
codec.stop();
222236
codec.release();
223237
} catch (Exception e) {
224238
Log.e(TAG, "Decoder close exception", e);
225239
}
240+
releaseSlot(slot);
226241
}
227242
}
228243

@@ -254,6 +269,8 @@ void createDecoder(boolean codecH265) {
254269
synchronized (decoderLock) {
255270
retry.onFailure(nowMs());
256271
}
272+
// Limit still unknown: another cell is creating the first decoder, just retry
273+
if (limit < 0) return;
257274
Log.w(TAG, "Decoder limit reached: " + limit + " × " + type);
258275
if (listener != null) listener.onDecoderLimit(type, limit);
259276
return;
@@ -280,6 +297,7 @@ void createDecoder(boolean codecH265) {
280297
retry.onFailure(nowMs());
281298
}
282299
Log.e(TAG, "Cannot setup decoder (attempt " + retry.failures() + "): " + e.getMessage());
300+
if (listener != null) listener.onDecoderFailed();
283301
return;
284302
}
285303

@@ -359,12 +377,13 @@ boolean decodeFrame(Frame buffer, boolean codecH265, long watchdogRef) {
359377
drainOutput(codec);
360378
} catch (Exception e) {
361379
decoder = null;
362-
releaseSlot();
380+
String slot = takeSlot();
363381
retry.onFailure(nowMs());
364382
Log.e(TAG, "Codec exception, retry in "
365383
+ DecoderRetry.delayMs(retry.failures()) + " ms: " + e.getMessage());
366384
try { codec.stop(); } catch (Exception ignored) {}
367385
try { codec.release(); } catch (Exception ignored) {}
386+
releaseSlot(slot);
368387
if (listener != null) listener.onDecoderFailed();
369388
}
370389
}

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

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,10 @@ void setAudioSink(AudioManager sink, int route) {
109109
decoderStartedMs = System.nanoTime() / 1_000_000;
110110
if (listener != null) listener.onRemoveOverlay(index);
111111
}
112-
@Override public void onDecoderFailed() {}
112+
@Override public void onDecoderFailed() {
113+
// A decoder limit message says more; keep it
114+
if (problem == null || !problem.startsWith("Decoder limit")) problem = "Decoder error";
115+
}
113116
@Override public void onDecoderLimit(String mime, int limit) {
114117
problem = "Decoder limit: " + (limit > 0 ? limit + " × " : "")
115118
+ (mime.contains("hevc") ? "H.265" : "H.264");
@@ -229,7 +232,9 @@ public void onStatus(String status) {
229232

230233
@Override
231234
public void onFailure(String message) {
232-
problem = shortError(message);
235+
// A decoder problem stays visible; the connection retries on its own
236+
String p = problem;
237+
if (p == null || !p.startsWith("Decoder")) problem = shortError(message);
233238
}
234239

235240
/** A connection error short enough for a cell corner. */
@@ -333,6 +338,11 @@ boolean showsVideo(long ms) {
333338
return pausedReason == null && "connected".equals(state) && decoder.renderedWithin(ms);
334339
}
335340

341+
/** Connected and decoding: its load is real, so demoting or pausing it helps (#85). */
342+
boolean isPlaying() {
343+
return pausedReason == null && "connected".equals(state) && decoder.hasDecoder();
344+
}
345+
336346
/** Playing long enough for its decode rate to mean something (#83). */
337347
boolean isSettled(long nowMs) {
338348
long started = decoderStartedMs;

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

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,8 +102,23 @@ public void instanceBudgetHoldsTheLearntLimit() {
102102
MediaCodecManager.InstanceBudget.tryAcquire(hevc));
103103
MediaCodecManager.InstanceBudget.release(hevc);
104104
assertTrue(MediaCodecManager.InstanceBudget.tryAcquire(hevc));
105-
// Other codecs are counted separately
105+
// Other codecs are counted separately; while a limit is unknown, one at a time
106106
assertTrue(MediaCodecManager.InstanceBudget.tryAcquire("video/avc"));
107107
assertEquals(-1, MediaCodecManager.InstanceBudget.limit("video/avc"));
108+
assertFalse("second decoder waits until the first has told its limit",
109+
MediaCodecManager.InstanceBudget.tryAcquire("video/avc"));
110+
MediaCodecManager.InstanceBudget.learn("video/avc", 16);
111+
assertTrue(MediaCodecManager.InstanceBudget.tryAcquire("video/avc"));
112+
}
113+
114+
@Test
115+
public void decoderLimitIsNotHiddenByAConnectionError() {
116+
QuadCell c = cell();
117+
c.onStatus("connected");
118+
c.setKeyFramesOnly(false);
119+
c.onFailure("ECONNREFUSED");
120+
assertEquals("Connection refused", c.sample(1000));
121+
c.onStatus("connected");
122+
assertEquals("H.264 0/0 fps", c.sample(2000));
108123
}
109124
}

0 commit comments

Comments
 (0)