Conversation
…the TextureView OpenIPC#108 replaced the main video SurfaceView with a TextureView so MediaPipe can grab frames via getBitmap(). That swap is unconditional, so every user pays for it even with object detection turned off: the video is no longer eligible for a hardware overlay plane and instead goes through the view hierarchy's GPU composition, which costs GPU time, power and about one frame of latency. The layout now carries both renderers and the active one is picked from the existing "od_enabled" preference: - object detection off (default) -> mainVideoSurface (SurfaceView), the pre-OpenIPC#108 behaviour - object detection on -> mainVideo (TextureView), unchanged Toggling detection in the menu swaps the renderer, which means the decoder needs a different surface. VideoPlayer.stopAndRemoveReceiverDecoder() also stops the UDP receiver and nothing restarts it, so a hot swap is not safe today; the toggle restarts the app instead, the same way the VR mode toggle already does. startObjectDetectionLoop() bails out if the TextureView is not the active renderer.
writeAndroidPerformanceParams() was never called: both call sites in AndroidMediaFormatHelper.h were commented out, and the same keys sat commented out a second time in VideoDecoder::configureStartDecoder(). So every decoder was configured without "low-latency" and without "priority", i.e. MediaCodec kept its default reorder/output queue, which on a live stream with no B-frames only adds latency. The keys are now written, but behind a switch so a device whose decoder does not like them can be put back on the stock pipeline: - Settings -> Video -> Low latency, persisted as "low_latency_decoder", default on - plumbed through VideoPlayer.setLowLatency() / nativeSetLowLatency() to VideoDecoder, applied when the decoder is configured - writeAndroidPerformanceParams() also gained the vendor low-latency keys that were commented out in VideoDecoder.cpp (Qualcomm, HiSilicon, rtc-ext) and is now static like the two functions next to it Unknown AMediaFormat keys are ignored by MediaCodec, so writing all variants is safe across vendors and Android versions.
onNewNALU() allocated a copy of every NALU with new uint8_t[] and pushed it
into naluQueue as a NALU. NALU is documented as a non-owning view ("it does
not do any memory management", NALU.hpp) and its destructor is defaulted, so
popping the queue never freed anything.
The buffer leaks for as long as a recording runs - roughly the video bitrate,
so ~1 MB/s at 8 Mbit/s. A ten minute recording leaks a few hundred MB and the
app eventually gets killed by the OOM killer, mid flight.
The queue now holds an owning DvrNalu { std::vector<uint8_t>, bool } that is
moved in and out, so the bytes are freed with the queue entry and anything
still queued is released when the writer thread stops.
Note: the queued NALU is also read after the h265 flag was needed for
mp4_h26x_write_init(), so the flag is read from the front element before the
move instead.
Four separate ways the adapter path can take the app down or wedge it. All of them are easy to hit on a powered hub that re-enumerates the dongle, which is how a lot of ground stations are wired. 1. Deliberate null deref. WfbngLink::stop() ran a CRASH() macro (`int *i = 0; *i = 42;`) when the fd was no longer in rtl_devices. That is a recoverable state - the adapter was already gone - and it killed the process. Removed, now a warning and return. 2. NPE on openDevice(). UsbManager.openDevice() returns null when the permission was revoked or the device disappeared between the permission check and the open; getFileDescriptor() was called on it unconditionally. start() now returns false instead, WfbLinkManager reports it and leaves the adapter out of activeWifiAdapters so the next refresh retries it. Before, a failed adapter was recorded as active and never retried. 3. Leaked usbfs descriptors. UsbDeviceConnection was never closed and linkConns was never cleared, so every attach/detach cycle leaked one fd plus the map entry. 4. USB permission dialog on Android 14. requestPermission() got a PendingIntent built from an implicit Intent. Android 14 refuses to deliver those to a runtime registered receiver, so the result never arrived and the app sat on "No permission for wifi adapter(s)". setPackage() added. Also: refreshAdapters() dereferenced getAttachedAdapters() without checking for the null it returns when the device filter fails to parse, and the wfb thread name indexed split()[1] without checking the device name matched /dev/bus/usb/.
VideoActivity declares no android:configChanges, so every configuration change destroys and recreates it. onCreate() re-runs the whole bring-up and onPause()/onStop() tear the link down first, which means a few seconds of black screen and a fresh USB/wfb-ng/decoder init. That fires more often than it looks: - window resize in multi-window / freeform / desktop mode (screenSize, smallestScreenSize, screenLayout) - also how the app is presented on Android XR headsets, where the panel is user resizeable - attaching a keyboard or a dock (keyboard, keyboardHidden, navigation) - rotation (orientation) Handling those in-process is enough: the layout is ConstraintLayout based and re-measures itself, the activity keeps no configuration dependent state, and none of those qualifiers select alternative resources in this project, so no onConfigurationChanged() override is needed. uiMode and density are deliberately not in the list: values-night/ and the mipmap-*dpi buckets do depend on them, so those two still need a recreate to pick up the right resources. Also: - android:resizeableActivity="true" - be explicit rather than relying on the target SDK default - android.hardware.touchscreen android:required="false" - the implicit default is required=true, which marks the app incompatible with any ground station driven by a pointer or a gamepad instead of a touchscreen
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Integration branch that carries the five latency/robustness fixes proposed upstream, so
work stacked on top of them (the OpenXR goggle mode) has a single base to build on.
Nothing new here — it is
OpenIPC/PixelPilot@masterplus a merge of each of these:writeAndroidPerformanceParams()was never called.SurfaceViewunless object detection needs theTextureView. Undoes an unconditional latency regression from OpenIPC#108.android:configChangesso a panel resize (how headsets present 2D apps) stops destroying the activity and re-initialising USB/wfb-ng/decoder.NALUis a non-owning view. ~1 MB/s at 8 Mbit/s, OOM kill mid flight.CRASH()macro,openDevice()NPE, leaked usbfs descriptors, Android 14 implicit-PendingIntentbreaking the permission dialog.All five merged without conflicts and the result compiles for
arm64-v8a+armeabi-v7a.This branch exists so the immersive mode can be reviewed as its own diff instead of one
giant change, and so it does not have to wait on upstream review. Once the upstream PRs land
this can be dropped and rebased onto
master.