Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 37 additions & 2 deletions android/app/src/main/java/com/openipc/decoder/DecodePlanner.java
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,11 @@ final class DecodePlanner {
static final double FAILED_MARGIN = 0.9;
/** Quiet time after which a load that failed is tried once more: it may have been a fluke. */
static final long FAILED_RETRY_MS = 60 * 60_000;
/**
* After the grid starts, every decoder starts at once and all of them lose frames for a
* second or two: no decision on losses that early (#148). Set by the app; 0 in tests.
*/
static final long WARMUP_MS = 10_000;

static final class Cell {
int index;
Expand Down Expand Up @@ -70,7 +75,7 @@ static final class Cell {
}
}

enum Action { NONE, PROMOTE, DEMOTE, PAUSE }
enum Action { NONE, PROMOTE, DEMOTE, PAUSE, RESUME }

static final class Decision {
final Action action;
Expand Down Expand Up @@ -98,6 +103,15 @@ static final class Decision {
private final Map<Integer, Long> unknownSince = new HashMap<>();
private long quietSinceMs = -1;
private long lastChangeMs = -1;
private long warmupMs;
private long startMs = -1;
/** Quiet time before a paused cell gets another try; doubles with every pause (#148). */
private long resumeWaitMs = PROBE_QUIET_MS;
/** The last lossy sample; promotions do not move it, unlike quietSinceMs. */
private long lastLossMs = -1;

/** No loss-based decision for this long after the first step (#148). */
void setWarmup(long ms) { warmupMs = ms; }

/**
* Never learnt below this (#126): losses at a lower load come from a stalled system (a hung
Expand Down Expand Up @@ -149,21 +163,28 @@ static long fullLoad(int width, int height, float fps) {

Decision step(List<Cell> cells, long nowMs) {
if (quietSinceMs < 0) quietSinceMs = nowMs;
if (startMs < 0) startMs = nowMs;
// Starting decoders lose frames together: losses then decide nothing, and the time is
// not quiet either; promotions that fit go on (Qodo on #149)
boolean warming = nowMs - startMs < warmupMs;
if (warming) quietSinceMs = nowMs;
Comment on lines +169 to +170

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Action required

1. Startup losses can still pause cells 📎 Requirement gap ≡ Correctness

The planner suppresses only the current lossy sample during one global warm-up window, while each
cell reports loss-rate deltas accumulated since its previous sample. When cells settle at different
times or a loss interval crosses the 10-second boundary, transient startup losses enter lossyTimes
after warm-up and can trigger demotion or pausing within the loss window.
Agent Prompt
## Issue description
Startup losses can be sampled after the planner-wide warm-up expires because each cell's loss-rate interval is independent and is not reset or discarded at the warm-up boundary. Staggered decoder startup can therefore produce enough post-warm-up samples to trigger overload actions for a transient startup burst.

## Fix Focus Areas
- android/app/src/main/java/com/openipc/decoder/DecodePlanner.java[166-187]
- android/app/src/main/java/com/openipc/decoder/Decoder.java[1161-1166]
- android/app/src/main/java/com/openipc/decoder/QuadCell.java[314-330]

## Recommended Fix
Ensure loss samples covering each decoder's startup period are discarded rather than counted after the global warm-up expires. Reset or baseline the per-cell loss counters at the warm-up boundary, or otherwise tag and ignore startup-derived samples until each cell has completed its startup settling interval, while continuing to accept losses that occur after startup as overload evidence.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

long load = 0;
boolean anyLossy = false;
for (Cell c : cells) {
// A software cell neither loads the hardware decoder nor shows its limits (#88)
if (c.paused || c.pinned) continue;
if (c.full && c.known) load += c.fullLoad;
else if (!c.full) load += c.keyLoad;
anyLossy |= c.lossy;
anyLossy |= c.lossy && !warming;
}
while (!lossyTimes.isEmpty() && nowMs - lossyTimes.peekFirst() >= LOSS_WINDOW_MS) {
lossyTimes.pollFirst();
}
if (lastLossMs < 0) lastLossMs = nowMs;
if (anyLossy) {
lossyTimes.addLast(nowMs);
quietSinceMs = nowMs;
lastLossMs = nowMs;
} else if (nowMs - quietSinceMs >= PROBE_QUIET_MS && load > proven) {
// A load that played without losses long enough is within this device's means
proven = load;
Expand Down Expand Up @@ -202,13 +223,27 @@ Decision step(List<Cell> cells, long nowMs) {
for (int i = cells.size() - 1; i >= 0; i--) {
Cell c = cells.get(i);
if (!c.paused && c.playing && !c.pinned) {
// The next try of a paused cell waits longer each time
resumeWaitMs = Math.min(resumeWaitMs * 2, PROBE_MAX_WAIT_MS);
return change(nowMs, Action.PAUSE, c.index);
}
}
return Decision.NONE;
}
if (anyLossy) return Decision.NONE;

// A paused cell gets another try on key frames after a quiet period, the highest
// priority first: a pause is not for ever (#148)
if (nowMs - lastLossMs >= resumeWaitMs) {
for (Cell c : cells) {
if (c.paused) {
quietSinceMs = nowMs;
lastLossMs = nowMs;
return change(nowMs, Action.RESUME, c.index);
}
}
}

// Promote the highest-priority key-frame cell whose load is known
for (Cell c : cells) {
if (c.full || c.paused || !c.playing || c.pinned) {
Expand Down
23 changes: 17 additions & 6 deletions android/app/src/main/java/com/openipc/decoder/Decoder.java
Original file line number Diff line number Diff line change
Expand Up @@ -387,7 +387,8 @@ private void setSlides(boolean on) {
for (QuadCell c : cells) if (c != null) c.setKeyFramesOnly(true);
}
// Back to adaptive: plan from scratch, cells are promoted again where they fit
if (!on && quadEnabled) decodePlanner = newPlanner();
// The decoders keep running: no warm-up (Qodo on #149)
if (!on && quadEnabled) decodePlanner = newPlanner(false);
Toast.makeText(this, on ? "Slides: key frames only" : "Multi-view",
Toast.LENGTH_SHORT).show();
}
Expand Down Expand Up @@ -1025,7 +1026,7 @@ public void run() {

private void startStatsTick() {
quadContainer.removeCallbacks(quadStatsTick);
decodePlanner = newPlanner();
decodePlanner = newPlanner(true);
quadContainer.postDelayed(quadStatsTick, 1000);
}

Expand All @@ -1042,17 +1043,21 @@ private void startStatsTick() {
static final long MIN_CAPACITY = DecodePlanner.fullLoad(1920, 1080, 30);
private DecodePlanner decodePlanner;

private DecodePlanner newPlanner() {
/** {@code warmup}: the grid's decoders start now, together (#148). */
private DecodePlanner newPlanner(boolean warmup) {
SharedPreferences pref = getSharedPreferences("settings", MODE_PRIVATE);
if (pref.contains("decoder_capacity")) {
pref.edit().remove("decoder_capacity").remove("decoder_failed_load").apply();
}
long learnt = pref.getLong(PREF_DECODER_CAPACITY, 0);
long failed = pref.getLong(PREF_DECODER_FAILED, 0);
if (learnt > 0) return new DecodePlanner(learnt, true, failed, MIN_CAPACITY);
long declared = MediaCodecManager.declaredBlocksPerSec;
return new DecodePlanner(declared > 0 ? declared : DEFAULT_CAPACITY, false, failed,
MIN_CAPACITY);
DecodePlanner p = learnt > 0 ? new DecodePlanner(learnt, true, failed, MIN_CAPACITY)
: new DecodePlanner(declared > 0 ? declared : DEFAULT_CAPACITY, false, failed,
MIN_CAPACITY);
// The decoders start together and lose frames for a moment (#148)
if (warmup) p.setWarmup(DecodePlanner.WARMUP_MS);
return p;
}

/**
Expand Down Expand Up @@ -1126,6 +1131,12 @@ private void planDecoding(QuadCell[] cells, long now) {
showQuadOverlay(d.index);
if (quadStats[d.index] != null) quadStats[d.index].setText(reason);
break;
case RESUME:
Log.i(TAG, "Quiet for a while: Quad-" + d.index + " plays again");
c.resume();
// stop() took its audio route; the selected cell gets it back (Qodo on #149)
applyQuadAudio();
break;
default:
break;
}
Expand Down
9 changes: 9 additions & 0 deletions android/app/src/main/java/com/openipc/decoder/QuadCell.java
Original file line number Diff line number Diff line change
Expand Up @@ -512,6 +512,15 @@ void pause(String reason) {
stop();
}

/** Back after a pause by the planner (#148): on key frames, as a cell starts. */
void resume() {
if (pausedReason == null) return;
pausedReason = null;
problem = null;
setKeyFramesOnly(true);
start();
Comment thread
qodo-free-for-open-source-projects[bot] marked this conversation as resolved.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remediation recommended

3. Old stream callbacks can stall resumed video 🐞 Bug ☼ Reliability

QuadCell.resume() restarts the same cell while an SDP callback from its stopped session can still
reach onSdp() and reset the resumed decoder's state. If the old callback passes its generation
check just before the pause and runs after the new session starts, it can clear the new stream's
queues or close its decoder.
Agent Prompt
## Issue description
Resuming a paused cell reuses its `QuadCell`, but an SDP callback from the stopped RTSP generation can run after the new session starts and reset its decoder state.
## Fix Focus Areas
- android/app/src/main/java/com/openipc/decoder/QuadCell.java[515-522]
- android/app/src/main/java/com/openipc/decoder/RtspSession.java[443-445]
## Recommended Fix
Make SDP callback delivery atomic with respect to session stop and generation replacement, or tag callbacks with their generation and reject stale callbacks before they mutate the cell.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

}

@Override
public void onSdp(Sdp sdp) {
// First the path, so a JPEG frame still being drawn no longer counts as current
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ private static void apply(List<DecodePlanner.Cell> cells, DecodePlanner.Decision
if (d.action == DecodePlanner.Action.PROMOTE) c.full = true;
if (d.action == DecodePlanner.Action.DEMOTE) c.full = false;
if (d.action == DecodePlanner.Action.PAUSE) c.paused = true;
if (d.action == DecodePlanner.Action.RESUME) c.paused = false;
}
}

Expand Down Expand Up @@ -86,6 +87,56 @@ public void keyFrameLossesWithNothingToDemotePauseAsLastResort() {
assertEquals(2, d.index);
}

@Test
public void lossesDuringTheWarmupDecideNothing() {
DecodePlanner p = new DecodePlanner(1, false);
p.setWarmup(DecodePlanner.WARMUP_MS);
List<DecodePlanner.Cell> cells = keyCells(3, P1080_30);
// Every decoder starting at once loses frames (#148)
for (DecodePlanner.Cell c : cells) c.lossy = true;
long t = 0;
for (; t < DecodePlanner.WARMUP_MS; t += 1000) {
assertEquals(DecodePlanner.Action.NONE, p.step(cells, t).action);
}
// The same losses after the warm-up still count
DecodePlanner.Decision d = DecodePlanner.Decision.NONE;
for (int i = 0; i < DecodePlanner.LOSS_SAMPLES; i++, t += 1000) d = p.step(cells, t);
assertEquals(DecodePlanner.Action.PAUSE, d.action);
}

@Test
public void aFittingCellIsPromotedDuringTheWarmup() {
DecodePlanner p = new DecodePlanner(BUDGET, false);
p.setWarmup(DecodePlanner.WARMUP_MS);
List<DecodePlanner.Cell> cells = keyCells(4, P1080_30);
// Losses of the start do not hold back what fits (Qodo on #149)
for (DecodePlanner.Cell c : cells) c.lossy = true;
assertEquals(DecodePlanner.Action.PROMOTE, p.step(cells, 0).action);
}

@Test
public void aPausedCellPlaysAgainAfterAQuietPeriod() {
DecodePlanner p = new DecodePlanner(1, false);
List<DecodePlanner.Cell> cells = keyCells(3, P1080_30);
for (DecodePlanner.Cell c : cells) c.lossy = true;
long t = 0;
DecodePlanner.Decision d = DecodePlanner.Decision.NONE;
for (int i = 0; i < DecodePlanner.LOSS_SAMPLES; i++, t += 1000) d = p.step(cells, t);
apply(cells, d);
assertTrue(cells.get(2).paused);
cells.get(2).playing = false;
for (DecodePlanner.Cell c : cells) c.lossy = false;
// Quiet: after the (doubled) wait the paused cell gets another try (#148)
d = DecodePlanner.Decision.NONE;
long pausedAt = t;
for (; d.action != DecodePlanner.Action.RESUME && t < pausedAt + 10 * 60_000; t += 1000) {
d = p.step(cells, t);
}
assertEquals(DecodePlanner.Action.RESUME, d.action);
assertEquals(2, d.index);
assertTrue(t - pausedAt >= DecodePlanner.PROBE_QUIET_MS);
}

@Test
public void aQuietLoadIsProvenAndAProbeMayExceedTheCapacity() {
DecodePlanner p = new DecodePlanner(P1080_30, false); // one camera fits
Expand Down
Loading