Skip to content

Commit 2759233

Browse files
committed
refactor: audit fixes — surface leak, socket dedup, dead code removal
- Fix Surface native handle leak in SurfaceTextureListener (replaceSurface) - Add SO_REUSEADDR for UDP ports 5000/5002 to prevent BindException - Make UDP audio thread a daemon with explicit join on exit - Add defensive copy in tcpStream before processPacket - Extract closeSockets() from 3 duplicate blocks - Eliminate double-copy in processAudio, remove pcmStagingBuf - Cache display density for dp() conversions - Remove dead code: switchToNextCamera, formatStatus, mConnect setText - Fix divider height to use dp(1) instead of raw pixel - Bump version to 1.15
1 parent 675cc5d commit 2759233

2 files changed

Lines changed: 55 additions & 82 deletions

File tree

app/build.gradle

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,8 @@ android {
1818
applicationId "com.openipc.decoder"
1919
minSdk 21
2020
targetSdk 36
21-
versionCode 14
22-
versionName "1.14"
21+
versionCode 15
22+
versionName "1.15"
2323
}
2424

2525
// Signing credentials from environment variables; null when absent.

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

Lines changed: 53 additions & 80 deletions
Original file line numberDiff line numberDiff line change
@@ -84,14 +84,18 @@ public class Decoder extends Activity {
8484
private int nalSize; // only touched on the network thread — no volatile needed
8585
private volatile MediaCodec mDecoder;
8686
// Surface captured on the UI thread via TextureView.SurfaceTextureListener; volatile so the
87-
// network thread can safely read it in createDecoder() without holding any UI lock
87+
// network thread can safely read it in createDecoder() without holding any UI lock.
88+
// Always release the previous Surface before replacing — Surface wraps a native handle.
8889
private volatile Surface mVideoSurface;
8990
// guards all MediaCodec lifecycle operations (create / use / release) so that
9091
// closeDecoder() called from the network thread never races with decodeFrame()
9192
// called from the video thread
9293
private final Object decoderLock = new Object();
9394
private TextureView mSurface;
9495

96+
// cached display density — avoids repeated getDisplayMetrics() calls during menu build
97+
private float mDensity;
98+
9599
// Pinch-to-zoom and pan state
96100
private ScaleGestureDetector mScaleDetector;
97101
private GestureDetector mGestureDetector;
@@ -129,11 +133,6 @@ public class Decoder extends Activity {
129133
// pre-allocated to avoid per-frame GC pressure in the video decode loop
130134
private final MediaCodec.BufferInfo mBufferInfo = new MediaCodec.BufferInfo();
131135

132-
// pre-allocated audio staging buffer: PCM frames are at most a few KB;
133-
// if a frame exceeds this size we fall back to a one-time heap allocation
134-
private static final int PCM_BUF_SIZE = 8192;
135-
private final byte[] pcmStagingBuf = new byte[PCM_BUF_SIZE];
136-
137136
// read from the network thread, written from the UI thread
138137
private static final int CAM_COUNT = 8;
139138
private static final String DEFAULT_URL = "rtsp://root:12345@192.168.1.10:554/stream=0";
@@ -218,19 +217,20 @@ protected void onCreate(Bundle savedInstanceState) {
218217
}
219218
} catch (PackageManager.NameNotFoundException ignored) {}
220219
mUserAgent = "User-Agent: OpenIPC-Decoder/" + mVersion + "\r\n";
220+
mDensity = getResources().getDisplayMetrics().density;
221221

222222
mSurface = findViewById(R.id.video_surface);
223223
mSurface.setKeepScreenOn(true);
224224
// capture the rendering Surface on the UI thread via TextureView listener
225225
mSurface.setSurfaceTextureListener(new TextureView.SurfaceTextureListener() {
226226
@Override public void onSurfaceTextureAvailable(android.graphics.SurfaceTexture st, int w, int h) {
227-
mVideoSurface = new Surface(st);
227+
replaceSurface(new Surface(st));
228228
}
229229
@Override public void onSurfaceTextureSizeChanged(android.graphics.SurfaceTexture st, int w, int h) {
230-
mVideoSurface = new Surface(st);
230+
replaceSurface(new Surface(st));
231231
}
232232
@Override public boolean onSurfaceTextureDestroyed(android.graphics.SurfaceTexture st) {
233-
mVideoSurface = null;
233+
replaceSurface(null);
234234
return true;
235235
}
236236
@Override public void onSurfaceTextureUpdated(android.graphics.SurfaceTexture st) {}
@@ -348,30 +348,6 @@ private void applyActiveCamera() {
348348
resetZoom();
349349
}
350350

351-
/** Format the status text: "[N/8] url" or "Camera N — not configured". */
352-
private String formatStatus() {
353-
String url = mHosts[mActive];
354-
if (url == null || url.isEmpty()) {
355-
return "[" + (mActive + 1) + "/" + CAM_COUNT + "]";
356-
}
357-
return "[" + (mActive + 1) + "/" + CAM_COUNT + "] " + url;
358-
}
359-
360-
/** Advance to the next configured camera slot (carousel mode). */
361-
private void switchToNextCamera() {
362-
int start = mActive;
363-
for (int i = 1; i <= CAM_COUNT; i++) {
364-
int next = (start + i) % CAM_COUNT;
365-
if (!mCarousel[next]) continue;
366-
String url = mHosts[next];
367-
if (url != null && !url.isEmpty() && !url.equals(DEFAULT_URL)) {
368-
mActive = next;
369-
saveSettings();
370-
return;
371-
}
372-
}
373-
}
374-
375351
/**
376352
* Lightweight camera switch for carousel mode.
377353
* Avoids full saveSettings() overhead — only updates the active slot,
@@ -396,12 +372,7 @@ private void carouselSwitch() {
396372

397373
// tear down current stream so the network thread reconnects to the new host
398374
activeStream = false;
399-
Socket tcp = mTcpSocket;
400-
if (tcp != null) try { tcp.close(); } catch (Exception ignored) {}
401-
DatagramSocket udp = mUdpSocket;
402-
if (udp != null) try { udp.close(); } catch (Exception ignored) {}
403-
DatagramSocket udpAudio = mUdpAudioSocket;
404-
if (udpAudio != null) try { udpAudio.close(); } catch (Exception ignored) {}
375+
closeSockets();
405376

406377
// discard stale frames from the previous camera
407378
nalQueue.clear();
@@ -420,14 +391,12 @@ private void startCarousel() {
420391
carouselEnabled = true;
421392
carouselHandler.removeCallbacks(carouselRunnable);
422393
carouselHandler.postDelayed(carouselRunnable, carouselInterval * 1000L);
423-
mConnect.setText(formatStatus());
424394
saveCarouselPrefs();
425395
}
426396

427397
private void stopCarousel() {
428398
carouselEnabled = false;
429399
carouselHandler.removeCallbacks(carouselRunnable);
430-
mConnect.setText(formatStatus());
431400
saveCarouselPrefs();
432401
}
433402

@@ -453,8 +422,12 @@ private void saveSettings() {
453422
edit.apply();
454423

455424
applyActiveCamera();
456-
mConnect.setText(formatStatus());
457425
activeStream = false;
426+
closeSockets();
427+
}
428+
429+
/** Close all active network sockets to unblock I/O threads. */
430+
private void closeSockets() {
458431
Socket tcp = mTcpSocket;
459432
if (tcp != null) try { tcp.close(); } catch (Exception ignored) {}
460433
DatagramSocket udp = mUdpSocket;
@@ -622,7 +595,7 @@ private void createMenu(View menu) {
622595
View divider = new View(this);
623596
divider.setBackgroundColor(Color.DKGRAY);
624597
layout.addView(divider, new LinearLayout.LayoutParams(
625-
LinearLayout.LayoutParams.MATCH_PARENT, 1));
598+
LinearLayout.LayoutParams.MATCH_PARENT, dp(1)));
626599

627600
TextView exit = createItem("Exit");
628601
layout.addView(exit);
@@ -704,10 +677,9 @@ private void focusChange(View item) {
704677
});
705678
}
706679

707-
/** Converts dp units to pixels using the current display density. */
680+
/** Converts dp units to pixels using cached display density. */
708681
private int dp(float dp) {
709-
return (int) TypedValue.applyDimension(
710-
TypedValue.COMPLEX_UNIT_DIP, dp, getResources().getDisplayMetrics());
682+
return (int) (dp * mDensity + 0.5f);
711683
}
712684

713685
@SuppressLint("SetJavaScriptEnabled")
@@ -783,7 +755,6 @@ private void updateResolution(int width, int height) {
783755
FrameLayout.LayoutParams params = new FrameLayout.LayoutParams(surfaceW, surfaceH);
784756
params.gravity = Gravity.CENTER;
785757
mSurface.setLayoutParams(params);
786-
mConnect.setVisibility(View.GONE);
787758
});
788759
}
789760

@@ -813,6 +784,13 @@ private void resetZoom() {
813784
applyZoomTransform();
814785
}
815786

787+
/** Release the previous Surface (if any) and store the new one. */
788+
private void replaceSurface(Surface next) {
789+
Surface prev = mVideoSurface;
790+
mVideoSurface = next;
791+
if (prev != null) prev.release();
792+
}
793+
816794
private int getFragment(byte data) {
817795
return codecH265 ? (data >> 1) & 0x3F : data & 0x1F;
818796
}
@@ -973,30 +951,21 @@ private void processAudio(Frame frame) {
973951
return;
974952
}
975953

976-
// reuse pre-allocated staging buffer for typical PCM frame sizes to reduce GC pressure;
977-
// fall back to heap allocation only for unusually large frames
978-
byte[] data = (length <= PCM_BUF_SIZE) ? pcmStagingBuf : new byte[length];
979-
System.arraycopy(frame.data(), header, data, 0, length);
954+
// allocate final buffer directly — avoids staging + copyOf double-copy
955+
byte[] queued = new byte[length];
956+
System.arraycopy(frame.data(), header, queued, 0, length);
980957
// swap bytes only for big-endian encodings (L16 per RFC 3551)
981958
if (audioBigEndian) {
982959
for (int i = 0; i + 1 < length; i += 2) {
983-
byte tmp = data[i];
984-
data[i] = data[i + 1];
985-
data[i + 1] = tmp;
960+
byte tmp = queued[i];
961+
queued[i] = queued[i + 1];
962+
queued[i + 1] = tmp;
986963
}
987964
}
988965

989-
// must copy into a fresh array before queuing — pcmStagingBuf is overwritten next call
990-
byte[] queued = (data == pcmStagingBuf) ? java.util.Arrays.copyOf(data, length) : data;
991-
Frame buffer = new Frame(queued, length);
992-
try {
993-
// non-blocking offer — drop frame immediately if queue is full
994-
// to avoid stalling the network thread
995-
if (!pcmQueue.offer(buffer)) {
996-
Log.w(TAG, "Audio queue full, frame dropped");
997-
}
998-
} catch (Exception e) {
999-
Log.w(TAG, "Audio queue error: " + e.getMessage());
966+
// non-blocking offer — drop frame immediately if queue is full
967+
if (!pcmQueue.offer(new Frame(queued, length))) {
968+
Log.w(TAG, "Audio queue full, frame dropped");
1000969
}
1001970
}
1002971

@@ -1401,12 +1370,17 @@ private void rtspConnect() throws Exception {
14011370
// single-pass SDP parse: audio rate, video codec, and per-track Control URLs
14021371
String[] trackUrls = parseSdp(sdp.toString(), rtspUrl);
14031372

1404-
// bind to fixed ports matching the OpenIPC camera convention
1373+
// bind to fixed ports matching the OpenIPC camera convention;
1374+
// SO_REUSEADDR prevents BindException on quick restart after crash
14051375
DatagramSocket udpVideo = null;
14061376
DatagramSocket udpAudio = null;
14071377
if (mType) {
1408-
udpVideo = new DatagramSocket(5000);
1409-
udpAudio = new DatagramSocket(5002);
1378+
udpVideo = new DatagramSocket(null);
1379+
udpVideo.setReuseAddress(true);
1380+
udpVideo.bind(new InetSocketAddress(5000));
1381+
udpAudio = new DatagramSocket(null);
1382+
udpAudio.setReuseAddress(true);
1383+
udpAudio.bind(new InetSocketAddress(5002));
14101384
}
14111385
try {
14121386

@@ -1462,11 +1436,17 @@ private void rtspConnect() throws Exception {
14621436
mUdpAudioSocket = udpAudio;
14631437
final DatagramSocket audioSock = udpAudio;
14641438
try {
1439+
// use a daemon thread so it doesn't prevent JVM exit;
1440+
// socket close in onPause/closeSockets unblocks receive()
14651441
Thread audioRx = new Thread(() -> {
14661442
try { udpStream(audioSock); } catch (IOException ignored) {}
14671443
}, "rtsp-udp-audio");
1444+
audioRx.setDaemon(true);
14681445
audioRx.start();
14691446
udpStream(udpVideo);
1447+
// ensure audio thread exits after video stream ends
1448+
audioSock.close();
1449+
audioRx.join(2000);
14701450
} finally {
14711451
mUdpSocket = null;
14721452
mUdpAudioSocket = null;
@@ -1552,7 +1532,10 @@ private void tcpStream(InputStream rawInput) throws IOException {
15521532
}
15531533

15541534
if (channel == 0 || channel == 2) {
1555-
processPacket(new Frame(pktBuf, len));
1535+
// defensive copy: pktBuf is reused on next iteration
1536+
byte[] copy = new byte[len];
1537+
System.arraycopy(pktBuf, 0, copy, 0, len);
1538+
processPacket(new Frame(copy, len));
15561539
}
15571540
}
15581541
}
@@ -1696,21 +1679,13 @@ protected void onPause() {
16961679
listenerGen++; // invalidate all threads from the current generation
16971680
listener = false;
16981681
activeStream = false;
1699-
// close active sockets to immediately unblock any blocking read()/receive()
1700-
// on the network thread; without this, a silent camera causes a thread leak
1701-
Socket tcp = mTcpSocket;
1702-
if (tcp != null) try { tcp.close(); } catch (Exception ignored) {}
1703-
DatagramSocket udp = mUdpSocket;
1704-
if (udp != null) try { udp.close(); } catch (Exception ignored) {}
1705-
DatagramSocket udpAudio = mUdpAudioSocket;
1706-
if (udpAudio != null) try { udpAudio.close(); } catch (Exception ignored) {}
1682+
closeSockets();
17071683
if (executor != null) {
17081684
executor.shutdownNow();
17091685
executor = null;
17101686
}
17111687
closeDecoder();
17121688
closeAudio();
1713-
// discard stale frames so the new session starts with a clean slate
17141689
nalQueue.clear();
17151690
pcmQueue.clear();
17161691
}
@@ -1726,8 +1701,6 @@ protected void onResume() {
17261701
startListener();
17271702
}
17281703

1729-
mConnect.setText(formatStatus());
1730-
17311704
if (carouselEnabled) {
17321705
carouselHandler.removeCallbacks(carouselRunnable);
17331706
carouselHandler.postDelayed(carouselRunnable, carouselInterval * 1000L);

0 commit comments

Comments
 (0)