fix: fixed avc latency, timeouts, proper shutdown and repeating frames bug #12
Conversation
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
WalkthroughAvcServer.kt: FPS parsing now validates and falls back to DEFAULT_FPS on invalid input; introduced centralized cleanupResources to close stdoutChannel, codec, and virtualDisplay; added MediaFormat KEY_REPEAT_PREVIOUS_FRAME_AFTER = 100_000L; increased codec output dequeue timeout to 100_000L; write-failure now calls cleanup and exits. Changes
Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
app/src/main/java/com/mobilenext/devicekit/AvcServer.kt (3)
64-69: Consider using Kotlin's range operator for readability.The validation logic is correct and the fallback behavior is a good improvement for robustness. For more idiomatic Kotlin:
- fps = if (parsedFps != null && parsedFps >= MIN_FPS && parsedFps <= MAX_FPS) { + fps = if (parsedFps != null && parsedFps in MIN_FPS..MAX_FPS) {
210-210: Comment is slightly misleading.The comment says "responsive shutdown" but increasing timeout from 10ms to 100ms actually makes shutdown less responsive (up to 100ms delay before detecting shutdown signal). The alignment with
KEY_REPEAT_PREVIOUS_FRAME_AFTERis sensible, but consider updating the comment to reflect the actual trade-off.- val timeout = 100_000L // 100ms timeout for responsive shutdown (matches REPEAT_FRAME_DELAY) + val timeout = 100_000L // 100ms timeout (matches KEY_REPEAT_PREVIOUS_FRAME_AFTER)
245-258: Duplicate cleanup paths create maintenance risk.The explicit cleanup here duplicates the
finallyblock (lines 307-313). WhileexitProcess(0)ensures thefinallyblock isn't reached (avoiding double-release), this creates two cleanup paths that must be kept in sync.Consider extracting cleanup into a reusable method:
+ private fun cleanupResources( + stdoutChannel: java.nio.channels.FileChannel?, + codec: MediaCodec?, + virtualDisplay: VirtualDisplay? + ) { + try { + stdoutChannel?.close() + codec?.stop() + codec?.release() + virtualDisplay?.release() + } catch (ex: Exception) { + Log.e(TAG, "Error during cleanup", ex) + } + }Then use this method in both the catch block and the finally block to ensure consistent cleanup behavior.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/src/main/java/com/mobilenext/devicekit/AvcServer.kt(4 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: lint-and-detekt
- GitHub Check: Analyze (java-kotlin)
🔇 Additional comments (1)
app/src/main/java/com/mobilenext/devicekit/AvcServer.kt (1)
165-166: No API level concerns — this key is well-supported.
KEY_REPEAT_PREVIOUS_FRAME_AFTERhas been available since API 19, and your project's minSdk of 29 fully supports it. The 100ms value is appropriate for keeping the stream alive during static screens without degrading responsiveness.
Member
Author
|
@coderabbitai generate docstring |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 14023 @gmegidish Here are the docstrings for the AvcServer class: package com.mobilenext.devicekit
import android.hardware.display.VirtualDisplay
import android.media.MediaCodec
import android.media.MediaCodecInfo
import android.media.MediaFormat
import android.os.IBinder
import android.util.Log
import android.view.Display
import android.view.Surface
import java.io.FileDescriptor
import java.io.FileOutputStream
import java.io.IOException
import java.nio.channels.Channels
import java.util.concurrent.CountDownLatch
import kotlin.system.exitProcess
/**
* AVC (H.264) encoder server that captures and streams the device screen using hardware-accelerated encoding.
*
* This class creates a virtual display, captures screen content, encodes it to H.264/AVC format,
* and outputs the encoded stream to stdout for consumption by clients.
*
* @property bitrate The target bitrate for video encoding in bits per second (e.g., 10_000_000 for 10 Mbps)
* @property scale The scaling factor to apply to the display resolution (e.g., 0.5 for half resolution, 1.0 for native)
* @property fps The target frames per second for the video stream (range: 1-60)
*/
class AvcServer(private val bitrate: Int, private val scale: Float, private val fps: Int) {
companion object {
private const val TAG = "AvcServer"
private const val DEFAULT_BITRATE = 10_000_000 // 10 Mbps (Google's default)
private const val DEFAULT_SCALE = 1.0f
private const val DEFAULT_FPS = 30
private const val MIN_FPS = 1
private const val MAX_FPS = 60
private const val I_FRAME_INTERVAL = 1 // 1 second
/**
* Entry point for the AVC server application.
*
* Parses command-line arguments, creates an AvcServer instance with the specified configuration,
* and starts the streaming process.
*
* @param args Command-line arguments:
* - `--bitrate <value>`: Target bitrate in bps (minimum: 100,000)
* - `--scale <value>`: Display scaling factor (range: 0.1-2.0)
* - `--fps <value>`: Target frame rate (range: 1-60)
*/
@JvmStatic
fun main(args: Array<String>) {
try {
val (bitrate, scale, fps) = parseArguments(args)
val server = AvcServer(bitrate, scale, fps)
server.start()
} catch (e: Exception) {
Log.e(TAG, "Failed to start AVC stream", e)
System.err.println("Error: ${e.message}")
exitProcess(1)
}
}
/**
* Parses command-line arguments and returns validated configuration values.
*
* Invalid or out-of-range values are coerced to safe defaults. For FPS, if the value
* is invalid or out of range, a warning is logged and the default is used.
*
* @param args Array of command-line arguments
* @return Triple containing (bitrate, scale, fps) with validated values
*/
private fun parseArguments(args: Array<String>): Triple<Int, Float, Int> {
var bitrate = DEFAULT_BITRATE
var scale = DEFAULT_SCALE
var fps = DEFAULT_FPS
var i = 0
while (i < args.size) {
when (args[i]) {
"--bitrate" -> {
if (i + 1 < args.size) {
bitrate = args[i + 1].toIntOrNull()?.coerceAtLeast(100_000) ?: DEFAULT_BITRATE
i++
}
}
"--scale" -> {
if (i + 1 < args.size) {
scale = args[i + 1].toFloatOrNull()?.coerceIn(0.1f, 2.0f) ?: DEFAULT_SCALE
i++
}
}
"--fps" -> {
if (i + 1 < args.size) {
val parsedFps = args[i + 1].toIntOrNull()
fps = if (parsedFps != null && parsedFps >= MIN_FPS && parsedFps <= MAX_FPS) {
parsedFps
} else {
Log.w(TAG, "Invalid fps value: ${args[i + 1]}. Using default: $DEFAULT_FPS")
DEFAULT_FPS
}
i++
}
}
}
i++
}
return Triple(bitrate, scale, fps)
}
}
private val shutdownLatch = CountDownLatch(1)
/**
* Starts the AVC streaming server.
*
* Registers a shutdown hook for graceful termination and begins streaming H.264 encoded
* screen frames to stdout. This method blocks until the stream is terminated.
*
* @throws Exception if an error occurs during stream initialization or operation
*/
private fun start() {
try {
// Register shutdown hook for graceful termination
Runtime.getRuntime().addShutdownHook(Thread {
Log.d(TAG, "Shutdown hook triggered")
shutdown()
})
// Start H.264 streaming
streamAvcFrames()
} catch (e: Exception) {
Log.e(TAG, "Error in AVC stream", e)
System.err.println("Error: ${e.message}")
exitProcess(1)
}
}
/**
* Initiates graceful shutdown of the streaming server.
*
* Signals the encoding loop to terminate by releasing the shutdown latch.
*/
private fun shutdown() {
shutdownLatch.countDown()
}
/**
* Captures and streams H.264 encoded frames from the device screen.
*
* This method:
* 1. Creates a virtual display with scaled dimensions
* 2. Configures a MediaCodec H.264 encoder with optimal settings
* 3. Continuously encodes screen frames and writes them to stdout
* 4. Handles graceful shutdown and resource cleanup
*
* The encoded stream uses:
* - AVC High Profile for better quality
* - Low latency settings for real-time streaming
* - Frame repetition after 100ms of inactivity to keep the stream alive
* - Zero-copy output via FileChannel for maximum performance
*
* @throws IllegalArgumentException if display dimensions are invalid or exceed codec capabilities
* @throws IOException if output pipe communication fails
*/
private fun streamAvcFrames() {
val displayInfo = DisplayUtils.getDisplayInfo()
val scaledWidth = (displayInfo.width * scale).toInt()
val scaledHeight = (displayInfo.height * scale).toInt()
Log.d(TAG, "Starting AVC stream: ${displayInfo.width}x${displayInfo.height} -> ${scaledWidth}x${scaledHeight}")
Log.d(TAG, "Configuration: bitrate=$bitrate, fps=$fps, I-frame interval=${I_FRAME_INTERVAL}s")
Log.d(TAG, "Scaled dimensions: width=$scaledWidth, height=$scaledHeight")
// Validate dimensions
if (scaledWidth <= 0 || scaledHeight <= 0) {
throw IllegalArgumentException("Invalid dimensions: ${scaledWidth}x${scaledHeight}")
}
if (scaledWidth % 2 != 0 || scaledHeight % 2 != 0) {
Log.w(TAG, "Warning: Dimensions not divisible by 2, may cause issues: ${scaledWidth}x${scaledHeight}")
}
// Check codec capabilities before attempting to configure
val codec = MediaCodec.createEncoderByType(MediaFormat.MIMETYPE_VIDEO_AVC)
val codecInfo = codec.codecInfo
val capabilities = codecInfo.getCapabilitiesForType(MediaFormat.MIMETYPE_VIDEO_AVC)
val videoCapabilities = capabilities.videoCapabilities
Log.d(TAG, "Codec capabilities:")
Log.d(TAG, " Supported widths: ${videoCapabilities.supportedWidths}")
Log.d(TAG, " Supported heights: ${videoCapabilities.supportedHeights}")
Log.d(TAG, " Width alignment: ${videoCapabilities.widthAlignment}")
Log.d(TAG, " Height alignment: ${videoCapabilities.heightAlignment}")
// Check if dimensions are supported
if (!videoCapabilities.isSizeSupported(scaledWidth, scaledHeight)) {
val maxWidth = videoCapabilities.supportedWidths.upper
val maxHeight = videoCapabilities.supportedHeights.upper
Log.e(TAG, "Dimensions ${scaledWidth}x${scaledHeight} not supported by codec")
Log.e(TAG, "Maximum supported: ${maxWidth}x${maxHeight}")
codec.release()
throw IllegalArgumentException(
"Video dimensions ${scaledWidth}x${scaledHeight} exceed codec capabilities. " +
"Maximum supported: ${maxWidth}x${maxHeight}. " +
"Try using --scale parameter to reduce resolution (e.g., --scale 0.5)"
)
}
// Configure MediaCodec for H.264 encoding (Google's configuration)
val format = MediaFormat.createVideoFormat(
MediaFormat.MIMETYPE_VIDEO_AVC,
scaledWidth,
scaledHeight
).apply {
setInteger(MediaFormat.KEY_BIT_RATE, bitrate)
setInteger(MediaFormat.KEY_FRAME_RATE, fps)
setInteger(MediaFormat.KEY_CAPTURE_RATE, fps) // Set capture rate to match frame rate
setFloat(MediaFormat.KEY_OPERATING_RATE, fps.toFloat()) // Set operating rate
setInteger(MediaFormat.KEY_I_FRAME_INTERVAL, I_FRAME_INTERVAL)
setInteger(MediaFormat.KEY_COLOR_FORMAT, MediaCodecInfo.CodecCapabilities.COLOR_FormatSurface)
// Use High profile for better VUI support
setInteger(MediaFormat.KEY_PROFILE, MediaCodecInfo.CodecProfileLevel.AVCProfileHigh)
// Low latency settings
setInteger(MediaFormat.KEY_LATENCY, 0) // Request lowest latency
setInteger(MediaFormat.KEY_PRIORITY, 0) // Realtime priority
// Repeat previous frame after 100ms to keep stream alive when screen is static
setLong(MediaFormat.KEY_REPEAT_PREVIOUS_FRAME_AFTER, 100_000L) // 100ms in microseconds
}
Log.d(TAG, "MediaFormat created: $format")
Log.d(TAG, "Codec created, attempting to configure...")
try {
codec.configure(format, null, null, MediaCodec.CONFIGURE_FLAG_ENCODE)
Log.d(TAG, "Codec configured successfully")
// Log the actual output format to see what the codec set
val outputFormat = codec.outputFormat
Log.d(TAG, "Codec output format: $outputFormat")
val actualFrameRate = outputFormat.getInteger(MediaFormat.KEY_FRAME_RATE, -1)
Log.d(TAG, "Actual frame rate in output: $actualFrameRate")
} catch (e: Exception) {
Log.e(TAG, "Failed to configure codec with format: $format", e)
codec.release()
throw e
}
// Get input surface from codec
val inputSurface = codec.createInputSurface()
// Create virtual display to render to codec's input surface
val virtualDisplay = DisplayUtils.createVirtualDisplay(
"avc.screen.capture",
scaledWidth,
scaledHeight,
displayInfo.dpi,
inputSurface
)
if (virtualDisplay == null) {
System.err.println("Error: Failed to create virtual display")
codec.release()
exitProcess(1)
}
// Start codec
codec.start()
Log.d(TAG, "AVC encoder started")
val bufferInfo = MediaCodec.BufferInfo()
val timeout = 100_000L // 100ms timeout for responsive shutdown (matches REPEAT_FRAME_DELAY)
// Get FileChannel for stdout to write directly from ByteBuffer (zero-copy)
val stdoutChannel = FileOutputStream(FileDescriptor.out).channel
var frameCount = 0
var lastPts = 0L
var firstPts = 0L
try {
// Encoding loop - matches Google's libscreen-sharing-agent.so behavior
while (!Thread.currentThread().isInterrupted) {
// Check if shutdown requested
if (shutdownLatch.count == 0L) {
break
}
// Dequeue encoded output buffer
val outputBufferIndex = codec.dequeueOutputBuffer(bufferInfo, timeout)
when {
outputBufferIndex >= 0 -> {
val outputBuffer = codec.getOutputBuffer(outputBufferIndex)
if (outputBuffer != null && bufferInfo.size > 0) {
// Write encoded H.264 data directly from ByteBuffer to stdout
// This is ZERO-COPY - ByteBuffer stays in native memory
// Blocking write provides backpressure (same as Google's SocketWriter)
outputBuffer.position(bufferInfo.offset)
outputBuffer.limit(bufferInfo.offset + bufferInfo.size)
// FileChannel.write() from DirectByteBuffer = zero-copy via DMA
try {
while (outputBuffer.hasRemaining()) {
stdoutChannel.write(outputBuffer)
}
} catch (e: IOException) {
// Pipe broken - client disconnected
Log.d(TAG, "Output pipe broken, cleaning up")
// Clean up resources before exiting
try {
stdoutChannel.close()
codec.stop()
codec.release()
virtualDisplay.release()
} catch (ex: Exception) {
Log.e(TAG, "Error during cleanup", ex)
}
exitProcess(0)
}
// Log frame info
val frameType = when {
(bufferInfo.flags and MediaCodec.BUFFER_FLAG_CODEC_CONFIG) != 0 -> "config"
(bufferInfo.flags and MediaCodec.BUFFER_FLAG_KEY_FRAME) != 0 -> "keyframe"
else -> "frame"
}
// Track presentation timestamps to calculate actual frame rate
if (frameType == "config") {
// Parse first few bytes to check for VUI
outputBuffer.position(bufferInfo.offset)
val firstBytes = ByteArray(minOf(20, bufferInfo.size))
outputBuffer.get(firstBytes)
} else {
if (frameCount == 0) {
firstPts = bufferInfo.presentationTimeUs
}
if (frameCount > 0 && frameCount % 60 == 0) {
val deltaPts = bufferInfo.presentationTimeUs - lastPts
val totalTime = (bufferInfo.presentationTimeUs - firstPts) / 1_000_000.0
val avgFps = frameCount / totalTime
// Log.d(TAG, "Frame $frameCount: pts=${bufferInfo.presentationTimeUs}µs, delta=${deltaPts}µs, avg_fps=%.2f".format(avgFps))
}
lastPts = bufferInfo.presentationTimeUs
frameCount++
}
// Log.v(TAG, "AVC $frameType: ${bufferInfo.size} bytes")
}
// Release buffer back to codec (enables backpressure when slow)
codec.releaseOutputBuffer(outputBufferIndex, false)
}
outputBufferIndex == MediaCodec.INFO_OUTPUT_FORMAT_CHANGED -> {
val newFormat = codec.outputFormat
Log.d(TAG, "Output format changed: $newFormat")
}
outputBufferIndex == MediaCodec.INFO_TRY_AGAIN_LATER -> {
// No buffer available, continue (normal)
}
else -> {
Log.w(TAG, "Unexpected output buffer index: $outputBufferIndex")
}
}
}
} finally {
Log.d(TAG, "Stopping AVC encoder")
stdoutChannel.close()
codec.stop()
codec.release()
virtualDisplay.release()
}
}
}The docstrings cover:
|
gmegidish
changed the title
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 1 file
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="app/src/main/java/com/mobilenext/devicekit/AvcServer.kt">
<violation number="1" location="app/src/main/java/com/mobilenext/devicekit/AvcServer.kt:237">
P3: Comment references non-existent constant `REPEAT_FRAME_DELAY`. Consider updating to reference `KEY_REPEAT_PREVIOUS_FRAME_AFTER` which is the actual setting being matched.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
|
|
||
| val bufferInfo = MediaCodec.BufferInfo() | ||
| val timeout = 10000L // 10ms timeout for lower latency | ||
| val timeout = 100_000L // 100ms timeout for responsive shutdown (matches REPEAT_FRAME_DELAY) |
There was a problem hiding this comment.
P3: Comment references non-existent constant REPEAT_FRAME_DELAY. Consider updating to reference KEY_REPEAT_PREVIOUS_FRAME_AFTER which is the actual setting being matched.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At app/src/main/java/com/mobilenext/devicekit/AvcServer.kt, line 237:
<comment>Comment references non-existent constant `REPEAT_FRAME_DELAY`. Consider updating to reference `KEY_REPEAT_PREVIOUS_FRAME_AFTER` which is the actual setting being matched.</comment>
<file context>
@@ -206,7 +234,7 @@ class AvcServer(private val bitrate: Int, private val scale: Float, private val
val bufferInfo = MediaCodec.BufferInfo()
- val timeout = 10000L // 10ms timeout for lower latency
+ val timeout = 100_000L // 100ms timeout for responsive shutdown (matches REPEAT_FRAME_DELAY)
// Get FileChannel for stdout to write directly from ByteBuffer (zero-copy)
</file context>
Suggested change
| val timeout = 100_000L // 100ms timeout for responsive shutdown (matches REPEAT_FRAME_DELAY) | |
| val timeout = 100_000L // 100ms timeout for responsive shutdown (matches KEY_REPEAT_PREVIOUS_FRAME_AFTER) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary by CodeRabbit
Bug Fixes
Refactor
✏️ Tip: You can customize this high-level summary in your review settings.