Repository navigation
fix: decoder recovery and lifecycle — Quad H.265, resume, thread leak, pause (#20 #21 #22 #23 #32) - #48
Conversation
A MediaCodec exception used to set decoderFailed for good: no decoder was created again until the user switched camera, while RTP kept flowing so the watchdog never fired either (fixes #21). - DecoderRetry: the next attempt after a failure waits 0.5 s, 1, 2, 4, 8 s, at most 10 s; a decoded frame resets it. Failed creation backs off the same way. - A new decoder skips input until the first parameter set (SPS/VPS/PPS), instead of getting P-frames it cannot decode. - setEnabled(false) releases the decoder and refuses to create another, so a late decode thread cannot leave one open after a pause or stop.
A Quad cell created its MediaCodecManager once (on the network thread) and returned early from initDecoder() when it already existed. On the first H.265 packet the codec-switch callback closed the decoder, and it was never created again: H.265 cameras stayed black (fixes #20). The cell now owns one MediaCodecManager for its lifetime; the decode thread creates the decoder whenever it is missing and feeds it the frame that triggered creation. stop() disables the manager instead of waiting up to 2 s for the executor.
…ause - loadSettings() reads back "active" and "quad_enabled", which were saved but never loaded; single view hides the Quad layout again. A shared link opens in single view (fixes #22). - startListener() shuts down the previous executor, so switching camera or saving a URL no longer leaks two idle threads (fixes #23). - onPause() disables the decoder and waits at most 300 ms for the audio thread instead of 2 s per executor, and Quad cells no longer block (fixes #32). - The frame that triggers decoder creation is passed to the new decoder. Tests: back-off delays and reset, the enable/disable gate, slot clamping.
PR Summary by QodoRecover video decoders and restore playback across lifecycle changes
AI Description
Diagram
High-Level Assessment
Files changed (5)
|
Code Review by Qodo
1.
|
- setEnabled(false) takes the decoder under the lock and releases it on a short-lived thread, so MediaCodec stop()/release() never run on the UI thread. - A new decoder waits for the complete parameter set (SPS+PPS for H.264, VPS+SPS+PPS for H.265) before it gets other frames. - Each decoder creation belongs to a lifecycle generation: one cancelled by a pause neither installs a decoder nor records a failure that would delay the next start. - AudioManager.setEnabled(false) on pause: an audio worker still running cannot create or publish an AudioTrack or an AAC decoder afterwards; onPause no longer waits for it at all. - A shared link is saved (slot URL and quad_enabled=false), so the next pause/resume does not jump back to Quad and lose it.
Disabling only sets the flags; the decoder is detached under decoderLock and released on the decoder-release thread, so the UI thread does not wait even for the few milliseconds a decode step holds the lock. While disabled, decodeFrame() feeds nothing.
clearQuadViews() cast each cell's parent to FrameLayout, but the cells were added straight to a LinearLayout row. The code never ran before, because Quad mode was never restored on resume; with #22 fixed it would throw ClassCastException on the first resume in Quad mode. Each TextureView now sits in its own FrameLayout, and the black placeholder overlays are added to and found in that container. Before, findViewWithTag() was called on the TextureView itself, so the overlays never worked.
|
Found while preparing #12 and fixed in 0a2e550: Added to the device checklist: Home → back while in Quad mode does not crash, and the black placeholders disappear when each camera starts. |
Found on the device (#53): - A link shared while the app is already open was delivered through onNewIntent(), which was not overridden, so getIntent() still held the old intent and the link was ignored. onNewIntent() now calls setIntent(); onResume() applies it. - RTSP response lines and URL errors are redacted before logging: user:password@ and password=/pass=/pwd= parameters. XM cameras put the password in the URL path, and Content-Base echoed it to logcat (fixes #55). - A connection closed by stop() is no longer logged as "RTSP connection failed". - QuadCell logs when its surface is ready and the session starts (to diagnose a black Quad cell seen once on the device).
On Android 10 the device delivered the SEND intent to the resumed activity through onNewIntent() without onPause()/onResume() (confirmed on the AG-216 box: no Link: log after the share). onNewIntent() now restarts the streams itself, which re-reads the settings and the new intent. onPause()/onResume() bodies moved into stopStreams()/startStreams().
With the standard launch mode, a SEND intent with FLAG_ACTIVITY_NEW_TASK (as 'am start' and some apps send it) only brought the existing task to the front: nothing was delivered and the link was lost (seen on the AG-216 box). singleTop delivers it to the running activity through onNewIntent(); shares without NEW_TASK still create a new instance in the sharing app's task, as before.
… H.265 Found on the device (#53): - startQuad() shut the executor down without bumping listenerGen. The audio loop kept running, and playPcmFrame() returned at once on every call with the interrupt flag set: one core at 100% for as long as Quad was shown. stopExecutor() now invalidates the generation first, and the loop also stops when interrupted. - NalAssembler treated the first H.265 packet as a switch from H.264 and closed the decoder that had just been created (two "Start video decoder" 0.6 s apart). The first packet now only sets the codec.
e8a7f2d made stopExecutor() bump listenerGen, but startListener() read the generation before calling it, so the new decode and audio threads exited at once and single view showed no video (caught by the device smoke test: 0 decoder starts).
On the Android 10 test box the Quad container sometimes stayed at 0x0 after being made visible, with no traversal scheduled, so no cell got a surface and Quad stayed black until the next resume (seen after a start brought the task to the front). 500 ms after showing Quad, if it still has no size, force a layout pass and log it.
RtspSessionTest iterated server.requests (a synchronized list) while the server thread was still appending, which threw ConcurrentModification- Exception now and then (failed once in CI on #54). Tests now read a copy taken under the list's lock.
|
Device-tested on the AG-216 Android 10 box with 3 cameras (results in #53, runs 2–4): #20, #21, #22, #23, #32 and #55 pass. Fixes found during testing are included: a shared link reaches the running app (singleTop + onNewIntent), password redaction in logcat, the spinning audio thread, the double decoder start on H.265, and a Quad layout safeguard (#56 deferred). |
Fixes #20, fixes #21, fixes #22, fixes #23, fixes #32.
DecoderRetry: the next attempt waits 0.5 → 1 → 2 → 4 → 8 → 10 s, and a decoded frame resets the back-off. A new decoder skips input until the first SPS/VPS/PPSMediaCodecManager; the decode thread creates a decoder whenever there is none. Creation on the network thread is gone (it was also a source of a race)loadSettings()readsactiveandquad_enabled. In single view the Quad layout is hidden. A shared link opens in single viewstartListener()shuts down the previous executoronPause()blocked the UI thread for secondsMediaCodecManager.setEnabled(false)releases the decoder and blocks creating a new one, so there is nothing to wait for. Only the audio thread is waited for, at most 300 ms.QuadCell.stop()does not waitAlso: the frame that triggered decoder creation (usually the SPS) is passed to the new decoder; previously it was lost and the picture waited for the next GOP.
Tests
91 JVM tests in total; 5 are new in
DecoderRetryTest:createDecoder()does nothing (onPause can block the UI thread for seconds (ANR risk), worst in Quad mode #32);assembleDebugandlintDebugpass with no new issues.Device checklist
adb shell ps -T -p <pid> | wc -l) (Thread leak on every camera switch or URL save #23)