app: harden mirroring lifecycle and state

1 files changed, 170 insertions, 107 deletions

diff --git a/app/src/main/java/invalid/lena/scrcpy/VideoSink.java b/app/src/main/java/invalid/lena/scrcpy/VideoSink.java
index ff8f449..8c69b31 100644
--- a/app/src/main/java/invalid/lena/scrcpy/VideoSink.java
+++ b/app/src/main/java/invalid/lena/scrcpy/VideoSink.java
@@ -10,7 +10,7 @@ import java.io.IOException;
 import java.nio.ByteBuffer;
 import java.util.ArrayDeque;
 import java.util.Deque;
-import java.util.Iterator;
+import java.util.concurrent.atomic.AtomicBoolean;
 
 // MediaCodec async-mode video decoder writing to a Surface.
 //
@@ -22,67 +22,75 @@ import java.util.Iterator;
 // Back-pressure policy: when no input buffer is free and the pending
 // queue is full, replace the oldest frame of the same kind, or the oldest
 // frame overall. The queue remains bounded even if a peer floods CSD.
-// Keyframes are not distinguishable here (the flag stays in VideoStream)
-// so they drop like any delta frame; the picture heals at the next one.
+// After a decoder rebuild, delta frames are dropped until the next keyframe;
+// feeding them first can leave MediaCodec waiting forever for missing refs.
 //
-// Output timing: releaseOutputBuffer is called with an absolute nano
-// timestamp on the System.nanoTime clock, derived from the wire PTS so
-// the surface composer interpolates frames against vsync instead of
-// rendering them as fast as they decode. Anchor is set on the first
-// frame: wallClockNs0 - ptsUs0 * 1000 = constant offset, then for each
-// subsequent buffer renderAt = ptsUs * 1000 + offset.
+// Output buffers render when decoded. The Surface compositor already
+// synchronizes presentation to vsync; translating a hostile remote PTS
+// into an absolute local clock can instead queue frames arbitrarily far
+// into the future.
 public final class VideoSink implements VideoFrames {
 
     private static final int MAX_PENDING = 8;
+    private static final int MAX_FRAME_BYTES = 8 * 1024 * 1024;
+    private static final int MAX_PENDING_BYTES = 16 * 1024 * 1024;
 
     private volatile Surface surface;
     private final Runnable onFatalError;
-    public  volatile long    frames; // public read for the status overlay
+    private final AtomicBoolean fatalReported = new AtomicBoolean();
+    private long             renderedFrames;
     private volatile MediaCodec codec;
     private HandlerThread handlerThread;
     private Handler handler;
 
     private final Object lock = new Object();
     private final Deque<Integer> freeInputs = new ArrayDeque<>(16);
-    private final Deque<Frame>   pending    = new ArrayDeque<>(MAX_PENDING);
+    private final VideoQueue pending = new VideoQueue(MAX_PENDING, MAX_PENDING_BYTES);
 
     private boolean released;
-    private long    ptsOffsetNs;  // wall-ns = ptsUs * 1000 + ptsOffsetNs
-    private boolean ptsAnchored;
 
-    private static final class Frame {
-        final byte[] data;
-        final long   ptsUs;
-        final boolean isConfig;
-        Frame(byte[] d, long pts, boolean cfg) { data = d; ptsUs = pts; isConfig = cfg; }
-    }
+    // The format the server last announced, held so the decoder can be
+    // built later if there was no output surface when it arrived.
+    private boolean haveFormat;
+    private int     fmtFourcc, fmtW, fmtH;
+
+    // Most recent CSD (SPS/PPS). A decoder built late, or rebuilt on
+    // resize, needs it before it can decode anything, and the server only
+    // sends it once per run. VideoStream hands us a fresh array per frame,
+    // so holding the reference is enough.
+    private byte[] lastConfig;
 
     public VideoSink(Surface surface, Runnable onFatalError) {
         this.surface = surface;
         this.onFatalError = onFatalError;
     }
 
-    // Swap the output Surface without rebuilding MediaCodec. Passing null
-    // detaches output: the codec keeps decoding but discards frames so the
-    // wire stays drained while the activity is backgrounded. Passing a new
-    // Surface re-attaches and rendering resumes from the next decoded frame.
+    // Swap the output Surface. A destroyed Surface cannot remain attached to
+    // MediaCodec, so null tears the decoder down but retains its format and
+    // codec config. The reader keeps draining the wire; a new Surface rebuilds
+    // the decoder and replays the cached config.
     public void setOutputSurface(Surface newSurface) {
+        if (newSurface == null) {
+            synchronized (lock) {
+                if (released) return;
+                surface = null;
+            }
+            teardownCodec(false);
+            return;
+        }
         synchronized (lock) {
             if (released) return;
             this.surface = newSurface;
             MediaCodec c = codec;
-            if (c == null) return; // not yet configured; new surface will be used at configure time
+            if (c == null) {
+                if (newSurface != null && haveFormat) startDeferredLocked();
+                return;
+            }
             try {
-                if (newSurface != null) c.setOutputSurface(newSurface);
-                // Calling setOutputSurface(null) is unsupported on some
-                // codecs; instead, ignore output buffers in the callback
-                // when surface is null (see onOutputBufferAvailable).
-            } catch (IllegalStateException e) {
-                Log.w("video sink: setOutputSurface: %s", e);
+                c.setOutputSurface(newSurface);
+            } catch (RuntimeException e) {
+                reportFatal(e, "video sink: setOutputSurface");
             }
-            // Re-anchor PTS so frames decoded against the old surface clock
-            // don't drag the new surface's render time into the past.
-            ptsAnchored = false;
         }
     }
 
@@ -90,78 +98,131 @@ public final class VideoSink implements VideoFrames {
     public void configure(int codecFourcc, int width, int height) throws IOException {
         String mime = mimeFor(codecFourcc);
         if (mime == null) throw new IOException("unsupported video codec " + Wire.fourccName(codecFourcc));
+        synchronized (lock) {
+            if (released) return;
+            haveFormat = true;
+            fmtFourcc = codecFourcc; fmtW = width; fmtH = height;
+            if (surface == null) {
+                // No output attached yet: the activity was backgrounded
+                // during bring-up, or this is a reconnect that completed
+                // while backgrounded. MediaCodec cannot be moved from
+                // ByteBuffer mode to Surface mode afterwards, so
+                // configuring with a null surface here would black the
+                // session out permanently and setOutputSurface() would
+                // throw for the rest of the session. Wait instead; the
+                // wire keeps draining and frames are dropped until a
+                // surface arrives.
+                Log.i("video sink: no output surface, deferring decoder (%s %dx%d)",
+                        mime, width, height);
+                return;
+            }
+            startCodecLocked(mime, width, height);
+        }
+    }
+
+    // Build the deferred decoder once a surface finally shows up. Failure
+    // here is fatal to the session: without a decoder there is no picture
+    // and no way to ask for one again.
+    private void startDeferredLocked() {
+        try {
+            startCodecLocked(mimeFor(fmtFourcc), fmtW, fmtH);
+        } catch (Exception e) {
+            reportFatal(e, "video sink: deferred configure failed");
+        }
+    }
+
+    // Must be called with `lock` held and `surface` non-null.
+    private void startCodecLocked(String mime, int width, int height) throws IOException {
         Log.i("video sink: configure mime=%s %dx%d", mime, width, height);
+        renderedFrames = 0;
+        freeInputs.clear();
+        pending.clear();
+
+        // Created before the HandlerThread: createDecoderByType throws
+        // IOException, which the RuntimeException cleanup below does not
+        // cover, and an orphaned thread would survive until release().
+        MediaCodec c = MediaCodec.createDecoderByType(mime);
 
         handlerThread = new HandlerThread("video-mc");
         handlerThread.start();
         handler = new Handler(handlerThread.getLooper());
 
-        codec = MediaCodec.createDecoderByType(mime);
-        codec.setCallback(new MediaCodec.Callback() {
+        MediaCodec.Callback callback = new MediaCodec.Callback() {
             @Override public void onInputBufferAvailable(MediaCodec mc, int idx) {
                 onFreeInput(mc, idx);
             }
             @Override public void onOutputBufferAvailable(MediaCodec mc, int idx, MediaCodec.BufferInfo info) {
                 try {
-                    if (surface == null) {
-                        // Activity backgrounded - discard output instead
-                        // of rendering to a dead surface.
+                    if (mc != codec) {
                         mc.releaseOutputBuffer(idx, false);
                         return;
                     }
-                    // PTS-honoured render: SurfaceFlinger queues the buffer
-                    // for renderTimestampNs and interpolates against vsync,
-                    // so bursty arrivals smooth out instead of judder.
-                    // Config frames (pts=0) fall through to render-immediately.
-                    long renderAtNs = renderTimeNs(info.presentationTimeUs);
-                    if (renderAtNs == 0L) mc.releaseOutputBuffer(idx, true);
-                    else                  mc.releaseOutputBuffer(idx, renderAtNs);
-                } catch (IllegalStateException ignored) {}
+                    mc.releaseOutputBuffer(idx, true);
+                    if (++renderedFrames == 1) {
+                        Log.i("video sink: rendered frame n=1");
+                    }
+                } catch (IllegalStateException e) {
+                    if (mc == codec) reportFatal(e, "video sink: releaseOutputBuffer");
+                }
             }
             @Override public void onError(MediaCodec mc, MediaCodec.CodecException e) {
-                Log.e(e, "video sink: codec error");
-                if (mc == codec && onFatalError != null) onFatalError.run();
+                if (mc == codec) reportFatal(e, "video sink: codec error");
             }
             @Override public void onOutputFormatChanged(MediaCodec mc, MediaFormat fmt) {
                 Log.i("video sink: output format %s", fmt);
             }
-        }, handler);
+        };
 
-        MediaFormat fmt = MediaFormat.createVideoFormat(mime, width, height);
-        codec.configure(fmt, surface, null, 0);
-        codec.start();
+        try {
+            c.setCallback(callback, handler);
+            MediaFormat fmt = MediaFormat.createVideoFormat(mime, width, height);
+            c.configure(fmt, surface, null, 0);
+            // Publish before start() so the identity checks in the
+            // callbacks match from the very first buffer.
+            codec = c;
+            c.start();
+        } catch (RuntimeException e) {
+            codec = null;
+            try { c.release(); } catch (Exception ignored) {}
+            HandlerThread ht = handlerThread;
+            handlerThread = null;
+            handler = null;
+            if (ht != null) ht.quitSafely();
+            throw e;
+        }
+
+        // A decoder built after the stream started - deferred for a
+        // missing surface, or rebuilt on resize - has missed the CSD the
+        // server only sends once. Replay it ahead of everything else.
+        if (lastConfig != null) {
+            pending.offer(new VideoQueue.Frame(lastConfig, 0L, true, false));
+        }
     }
 
     // Called by VideoStream for every encoded frame, in order.
     @Override
-    public void feed(byte[] data, long ptsUs, boolean isConfig) {
-        if (!isConfig) frames++;
+    public void feed(byte[] data, long ptsUs, boolean isConfig, boolean isKeyframe) {
         synchronized (lock) {
             if (released) return;
+            if (data == null || data.length == 0 || data.length > MAX_FRAME_BYTES) {
+                reportFatal(null, "video sink: invalid frame size");
+                return;
+            }
+            if (isConfig) lastConfig = data;
             // Try to drain immediately if there's a free input.
             while (!pending.isEmpty() && !freeInputs.isEmpty()) {
-                submit(codec, pending.pollFirst(), freeInputs.pollFirst());
+                submit(codec, pending.poll(), freeInputs.pollFirst());
             }
-            if (!freeInputs.isEmpty()) {
-                submit(codec, new Frame(data, ptsUs, isConfig), freeInputs.pollFirst());
+            boolean waiting = pending.needsKeyframe();
+            if (!pending.offer(new VideoQueue.Frame(data, ptsUs, isConfig, isKeyframe))) {
                 return;
             }
-            // Keep the queue strictly bounded. A newer config frame replaces
-            // an older one; retaining every config packet lets a hostile peer
-            // turn the queue into an unbounded allocation sink.
-            if (pending.size() >= MAX_PENDING) {
-                boolean removed = false;
-                for (Iterator<Frame> it = pending.iterator(); it.hasNext(); ) {
-                    Frame f = it.next();
-                    if (f.isConfig == isConfig) {
-                        it.remove();
-                        removed = true;
-                        break;
-                    }
-                }
-                if (!removed) pending.pollFirst();
+            if (waiting && isKeyframe && !isConfig) {
+                Log.i("video sink: accepted keyframe after configure or overflow");
+            }
+            while (!pending.isEmpty() && !freeInputs.isEmpty()) {
+                submit(codec, pending.poll(), freeInputs.pollFirst());
             }
-            pending.offerLast(new Frame(data, ptsUs, isConfig));
         }
     }
 
@@ -171,32 +232,43 @@ public final class VideoSink implements VideoFrames {
             if (released) return;
             released = true;
         }
-        teardownCodec();
+        teardownCodec(true);
     }
 
-    // Tear down the current decoder and reconfigure with new dimensions.
-    // The next CSD frame on the wire (server resets on resize) will prime
-    // the new codec instance.
+    // Tear down the current decoder and rebuild it at the new dimensions.
+    // startCodecLocked replays the cached CSD into the new instance.
     @Override
     public void reconfigure(int codecFourcc, int width, int height) throws IOException {
         synchronized (lock) {
             if (released) return;
+            // Disarm the deferred-start path for the window in which we
+            // hold no codec: a setOutputSurface() landing between the
+            // teardown and the configure below must not build a second
+            // decoder behind our back.
+            haveFormat = false;
         }
-        teardownCodec();
+        teardownCodec(false);
+        configure(codecFourcc, width, height);
+    }
+
+    // Claim the codec under `lock` so setOutputSurface() can never touch
+    // an instance that is being released, and two callers cannot both
+    // stop the same one. The stop/release themselves run unlocked: they
+    // can take a while, and they do not wait on the callback looper, so
+    // there is nothing to gain by holding the lock across them.
+    private void teardownCodec(boolean clearConfig) {
+        MediaCodec c;
+        HandlerThread ht;
         synchronized (lock) {
+            c = codec;
+            codec = null;
+            ht = handlerThread;
+            handlerThread = null;
+            handler = null;
             freeInputs.clear();
             pending.clear();
-            ptsAnchored = false; // re-anchor on the first frame of the new run
+            if (clearConfig) lastConfig = null;
         }
-        configure(codecFourcc, width, height);
-    }
-
-    private void teardownCodec() {
-        MediaCodec c = codec;
-        codec = null;
-        HandlerThread ht = handlerThread;
-        handlerThread = null;
-        handler = null;
         if (c != null) {
             try { c.stop(); } catch (Exception ignored) {}
             try { c.release(); } catch (Exception ignored) {}
@@ -208,7 +280,7 @@ public final class VideoSink implements VideoFrames {
     private void onFreeInput(MediaCodec mc, int idx) {
         synchronized (lock) {
             if (released || mc != codec) return;
-            if (!pending.isEmpty()) submit(mc, pending.pollFirst(), idx);
+            if (!pending.isEmpty()) submit(mc, pending.poll(), idx);
             else                    freeInputs.offerLast(idx);
         }
     }
@@ -216,38 +288,29 @@ public final class VideoSink implements VideoFrames {
     // Must be called with `lock` held. codec can be null mid-reconfigure
     // (teardownCodec runs unlocked); the frame is dropped like any other
     // back-pressure casualty.
-    private void submit(MediaCodec mc, Frame f, int idx) {
+    private void submit(MediaCodec mc, VideoQueue.Frame f, int idx) {
         if (mc == null || mc != codec) return;
         try {
             ByteBuffer buf = mc.getInputBuffer(idx);
             if (buf == null || f.data.length > buf.capacity()) {
-                Log.e("video sink: frame exceeds codec input (%d bytes)", f.data.length);
-                if (onFatalError != null) onFatalError.run();
+                reportFatal(null, "video sink: frame exceeds codec input ("
+                        + f.data.length + " bytes)");
                 return;
             }
             buf.clear();
             buf.put(f.data);
-            int flags = f.isConfig ? MediaCodec.BUFFER_FLAG_CODEC_CONFIG : 0;
+            int flags = f.config ? MediaCodec.BUFFER_FLAG_CODEC_CONFIG : 0;
             mc.queueInputBuffer(idx, 0, f.data.length, f.ptsUs, flags);
         } catch (IllegalStateException e) {
-            Log.w("video sink: queueInputBuffer: %s", e);
+            reportFatal(e, "video sink: queueInputBuffer");
         }
     }
 
-    // Convert a wire PTS (microseconds since some scrcpy epoch) into a
-    // System.nanoTime value the surface composer should render at.
-    // First call anchors the offset to "now" so latency stays whatever
-    // the wire produced. PTS=0 (config frames) and unanchored state
-    // both return 0 → caller falls back to render-immediately.
-    private long renderTimeNs(long ptsUs) {
-        if (ptsUs <= 0L) return 0L;
-        synchronized (lock) {
-            if (!ptsAnchored) {
-                ptsOffsetNs = System.nanoTime() - ptsUs * 1000L;
-                ptsAnchored = true;
-            }
-            return ptsUs * 1000L + ptsOffsetNs;
-        }
+    private void reportFatal(Exception error, String message) {
+        if (!fatalReported.compareAndSet(false, true)) return;
+        if (error == null) Log.e("%s", message);
+        else Log.e(error, "%s", message);
+        if (onFatalError != null) onFatalError.run();
     }
 
     private static String mimeFor(int fourcc) {