Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ENG-4199 Request Key Frame #11

Open
wants to merge 21 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 16 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 0 additions & 4 deletions app/meson.build
Original file line number Diff line number Diff line change
Expand Up @@ -322,10 +322,6 @@ if get_option('buildtype') == 'debug'
'src/util/str.c',
'src/util/strbuf.c',
]],
['test_strutil', [
'tests/test_strutil.c',
'src/util/str_util.c',
]],
['test_vector', [
'tests/test_vector.c',
]],
Expand Down
4 changes: 4 additions & 0 deletions app/src/control_msg.c
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,7 @@ sc_control_msg_serialize(const struct sc_control_msg *msg, unsigned char *buf) {
case SC_CONTROL_MSG_TYPE_EXPAND_SETTINGS_PANEL:
case SC_CONTROL_MSG_TYPE_COLLAPSE_PANELS:
case SC_CONTROL_MSG_TYPE_ROTATE_DEVICE:
case SC_CONTROL_MSG_TYPE_REQUEST_KEY_FRAME:
// no additional data
return 1;
default:
Expand Down Expand Up @@ -229,6 +230,9 @@ sc_control_msg_log(const struct sc_control_msg *msg) {
case SC_CONTROL_MSG_TYPE_ROTATE_DEVICE:
LOG_CMSG("rotate device");
break;
case SC_CONTROL_MSG_TYPE_REQUEST_KEY_FRAME:
LOG_CMSG("request key frame");
break;
default:
LOG_CMSG("unknown type: %u", (unsigned) msg->type);
break;
Expand Down
1 change: 1 addition & 0 deletions app/src/control_msg.h
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ enum sc_control_msg_type {
SC_CONTROL_MSG_TYPE_SET_CLIPBOARD,
SC_CONTROL_MSG_TYPE_SET_SCREEN_POWER_MODE,
SC_CONTROL_MSG_TYPE_ROTATE_DEVICE,
SC_CONTROL_MSG_TYPE_REQUEST_KEY_FRAME,
};

enum sc_screen_power_mode {
Expand Down
23 changes: 22 additions & 1 deletion app/src/forward.c
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@

#include <SDL2/SDL_events.h>

#include "control_msg.h"
#include "controller.h"
#include "events.h"
#include "packet.h"
#include "util/log.h"
Expand Down Expand Up @@ -99,6 +101,7 @@ bool forward_init(struct forward *forward, const char *socketname) {
forward->socket = socket(PF_LOCAL, SOCK_STREAM, 0);
if (forward->socket < 0) {
LOGE("Could not create forward socket");
sc_cond_destroy(&forward->queue_cond);
sc_mutex_destroy(&forward->mutex);
return false;
}
Expand All @@ -115,6 +118,7 @@ bool forward_init(struct forward *forward, const char *socketname) {
if(0 != err) {
LOGE("Could not bind to forward socket [%d][%s]", errno, strerror(errno));
close(forward->socket);
sc_cond_destroy(&forward->queue_cond);
sc_mutex_destroy(&forward->mutex);
return false;
}
Expand All @@ -123,6 +127,7 @@ bool forward_init(struct forward *forward, const char *socketname) {
if (!ok) {
LOGE("Could not create receiver_mutex");
close(forward->socket);
sc_cond_destroy(&forward->queue_cond);
sc_mutex_destroy(&forward->mutex);
return false;
}
Expand All @@ -140,6 +145,9 @@ static void notify_disconnected() {
void forward_destroy(struct forward *forward) {
close(forward->socket);
forward->socket = -1;
sc_mutex_destroy(&forward->receiver_mutex);
sc_cond_destroy(&forward->queue_cond);
sc_mutex_destroy(&forward->mutex);
}

static bool send_packet(struct forward *forward, const AVPacket *packet) {
Expand Down Expand Up @@ -309,6 +317,16 @@ run_forward_server(void *data) {
if (size>=4 && 0==strncmp("STOP", request, 4)) {
should_stop = true;
}
if (size>=3 && 0==strncmp("KEY", request, 3)) {
struct sc_control_msg msg = {
.type = SC_CONTROL_MSG_TYPE_REQUEST_KEY_FRAME,
};
if (forward->controller) {
if (!sc_controller_push_msg(forward->controller, &msg)) {
LOGW("Could not request key frame");
}
}
}
}
}
}
Expand All @@ -333,7 +351,8 @@ run_forward(void *data) {
sc_mutex_lock(&forward->mutex);

while (!forward->stopped && sc_queue_is_empty(&forward->queue)) {
sc_cond_timedwait(&forward->queue_cond, &forward->mutex, 1000);
sc_tick deadline = sc_tick_now() + SC_TICK_FROM_MS(1000);
sc_cond_timedwait(&forward->queue_cond, &forward->mutex, deadline);
}

if (forward->stopped) {
Expand Down Expand Up @@ -427,12 +446,14 @@ forward_push(struct forward *forward, const AVPacket *packet) {

if (forward->stopped) {
// reject any new packet (this will stop the stream)
sc_mutex_unlock(&forward->mutex);
return false;
}

struct forward_packet *rec = forward_packet_new(packet);
if (!rec) {
LOGE("Could not allocate forward packet");
sc_mutex_unlock(&forward->mutex);
return false;
}

Expand Down
4 changes: 4 additions & 0 deletions app/src/forward.h
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,10 @@ struct forward {
sc_cond queue_cond;
bool stopped;
struct forward_queue queue;

// NOTE(Frankleonrose): Assigned after controller is initialized, before
// packets start flowing.
struct sc_controller *controller;
};

bool forward_init(struct forward *forward, const char *socketname);
Expand Down
3 changes: 3 additions & 0 deletions app/src/scrcpy.c
Original file line number Diff line number Diff line change
Expand Up @@ -615,6 +615,9 @@ scrcpy(struct scrcpy_options *options) {
}
}

if (fwd) {
fwd->controller = controller;
}
}

// There is a controller if and only if control is enabled
Expand Down
16 changes: 16 additions & 0 deletions app/tests/test_control_msg_serialize.c
Original file line number Diff line number Diff line change
Expand Up @@ -320,6 +320,21 @@ static void test_serialize_rotate_device(void) {
assert(!memcmp(buf, expected, sizeof(expected)));
}

static void test_serialize_request_key_frame(void) {
struct sc_control_msg msg = {
.type = SC_CONTROL_MSG_TYPE_REQUEST_KEY_FRAME,
};

unsigned char buf[SC_CONTROL_MSG_MAX_SIZE];
size_t size = sc_control_msg_serialize(&msg, buf);
assert(size == 1);

const unsigned char expected[] = {
SC_CONTROL_MSG_TYPE_REQUEST_KEY_FRAME,
};
assert(!memcmp(buf, expected, sizeof(expected)));
}

int main(int argc, char *argv[]) {
(void) argc;
(void) argv;
Expand All @@ -338,5 +353,6 @@ int main(int argc, char *argv[]) {
test_serialize_set_clipboard_long();
test_serialize_set_screen_power_mode();
test_serialize_rotate_device();
test_serialize_request_key_frame();
return 0;
}
6 changes: 3 additions & 3 deletions mobot.mk
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,8 @@ build-app: $(AVLIBS)
LDFLAGS="-Wl,-lm -Wl,-lpthread $(MAC_LDFLAGS)" \
meson build-app --buildtype release --strip -Db_lto=true \
-Dlocal_libav=$(AVDIR) \
-Dcompile_server=false \
-Dcompile_server=true \
-Dusb=false \
|| (ret=$$?; rm -rf $@ && exit $$ret)

scrcpy: build-app
Expand All @@ -65,11 +66,10 @@ scrcpy: build-app
test:
rm -rf build_test
CFLAGS="-DMOBOT_VERSION='\"$(GIT_DESCRIBE)/$(ARCH)\"'" \
meson build_test --buildtype debug --strip -Db_sanitize=address -Db_lto=true -Dlocal_libav=build-libav -Dcompile_server=false
meson build_test --buildtype debug --strip -Db_sanitize=address -Db_lto=true -Dlocal_libav=build-libav -Dcompile_server=false -Dusb=false
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we also set -Dcompile_server=true here? Or is that unnecessary for tests?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's unnecessary for tests.

ninja -Cbuild_test
build_test/app/test_cli
build_test/app/test_control_msg_serialize
build_test/app/test_strutil
build_test/app/test_buffer_util
build_test/app/test_device_msg_deserialize
build_test/app/test_cbuf
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ public final class ControlMessage {
public static final int TYPE_SET_CLIPBOARD = 9;
public static final int TYPE_SET_SCREEN_POWER_MODE = 10;
public static final int TYPE_ROTATE_DEVICE = 11;
public static final int TYPE_REQUEST_KEY_FRAME = 12;

public static final long SEQUENCE_INVALID = 0;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ public ControlMessage next() {
case ControlMessage.TYPE_EXPAND_SETTINGS_PANEL:
case ControlMessage.TYPE_COLLAPSE_PANELS:
case ControlMessage.TYPE_ROTATE_DEVICE:
case ControlMessage.TYPE_REQUEST_KEY_FRAME:
msg = ControlMessage.createEmpty(type);
break;
default:
Expand Down
3 changes: 3 additions & 0 deletions server/src/main/java/com/genymobile/scrcpy/Controller.java
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,9 @@ private void handleEvent() throws IOException {
}
}
break;
case ControlMessage.TYPE_REQUEST_KEY_FRAME:
device.requestKeyFrame();
break;
case ControlMessage.TYPE_ROTATE_DEVICE:
Device.rotateDevice();
break;
Expand Down
15 changes: 15 additions & 0 deletions server/src/main/java/com/genymobile/scrcpy/Device.java
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,10 @@ public final class Device {
private static final ServiceManager SERVICE_MANAGER = new ServiceManager();
private static final Settings SETTINGS = new Settings(SERVICE_MANAGER);

public interface KeyFrameListener {
void onKeyFrameRequested();
}

public interface RotationListener {
void onRotationChanged(int rotation);
}
Expand All @@ -48,6 +52,7 @@ public interface ClipboardListener {
private final int lockVideoOrientation;

private ScreenInfo screenInfo;
private KeyFrameListener keyFrameListener;
private RotationListener rotationListener;
private ClipboardListener clipboardListener;
private final AtomicBoolean isSettingClipboard = new AtomicBoolean();
Expand Down Expand Up @@ -223,6 +228,10 @@ public static boolean isScreenOn() {
return SERVICE_MANAGER.getPowerManager().isScreenOn();
}

public synchronized void setKeyFrameListener(KeyFrameListener keyFrameListener) {
this.keyFrameListener = keyFrameListener;
}

public synchronized void setRotationListener(RotationListener rotationListener) {
this.rotationListener = rotationListener;
}
Expand Down Expand Up @@ -295,6 +304,12 @@ public static boolean powerOffScreen(int displayId) {
return pressReleaseKeycode(KeyEvent.KEYCODE_POWER, displayId, Device.INJECT_MODE_ASYNC);
}

public void requestKeyFrame() {
if (keyFrameListener!=null) {
keyFrameListener.onKeyFrameRequested();
}
}

/**
* Disable auto-rotation (if enabled), set the screen rotation and re-enable auto-rotation (if it was enabled).
*/
Expand Down
47 changes: 35 additions & 12 deletions server/src/main/java/com/genymobile/scrcpy/ScreenEncoder.java
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,9 @@
import android.media.MediaCodecList;
import android.media.MediaFormat;
import android.os.Build;
import android.os.Bundle;
import android.os.IBinder;
import android.os.SystemClock;
import android.view.Surface;

import java.io.FileDescriptor;
Expand All @@ -19,7 +21,7 @@
import java.util.List;
import java.util.concurrent.atomic.AtomicBoolean;

public class ScreenEncoder implements Device.RotationListener {
public class ScreenEncoder implements Device.RotationListener, Device.KeyFrameListener {

private static final int DEFAULT_I_FRAME_INTERVAL = 10; // seconds
private static final int REPEAT_FRAME_DELAY_US = 100_000; // repeat after 100ms
Expand All @@ -31,6 +33,7 @@ public class ScreenEncoder implements Device.RotationListener {
private static final long PACKET_FLAG_CONFIG = 1L << 63;
private static final long PACKET_FLAG_KEY_FRAME = 1L << 62;

private final AtomicBoolean keyFrameRequested = new AtomicBoolean();
private final AtomicBoolean rotationChanged = new AtomicBoolean();
private final ByteBuffer headerBuffer = ByteBuffer.allocate(12);

Expand All @@ -43,6 +46,7 @@ public class ScreenEncoder implements Device.RotationListener {
private long ptsOrigin;

private boolean firstFrameSent;
private long requestKeyFrameTime = 0;

public ScreenEncoder(boolean sendFrameMeta, int bitRate, int maxFps, List<CodecOption> codecOptions, String encoderName,
boolean downsizeOnError) {
Expand All @@ -54,6 +58,17 @@ public ScreenEncoder(boolean sendFrameMeta, int bitRate, int maxFps, List<CodecO
this.downsizeOnError = downsizeOnError;
}

@Override
public void onKeyFrameRequested() {
keyFrameRequested.set(true);
requestKeyFrameTime = SystemClock.uptimeMillis();
Ln.d("Received KEY FRAME request");
}

public boolean consumeKeyFrameRequest() {
return keyFrameRequested.getAndSet(false);
}

@Override
public void onRotationChanged(int rotation) {
rotationChanged.set(true);
Expand All @@ -76,8 +91,12 @@ public void streamScreen(Device device, FileDescriptor fd) throws IOException {

private void internalStreamScreen(Device device, FileDescriptor fd) throws IOException {
MediaFormat format = createFormat(bitRate, maxFps, codecOptions);
device.setKeyFrameListener(this);
device.setRotationListener(this);
boolean alive;
// Report time for initial key frame to be generated
requestKeyFrameTime = SystemClock.uptimeMillis();

try {
do {
MediaCodec codec = createCodec(encoderName);
Expand Down Expand Up @@ -128,6 +147,7 @@ private void internalStreamScreen(Device device, FileDescriptor fd) throws IOExc
}
} while (alive);
} finally {
device.setKeyFrameListener(null);
device.setRotationListener(null);
}
}
Expand All @@ -148,15 +168,20 @@ private boolean encode(MediaCodec codec, FileDescriptor fd) throws IOException {
boolean eof = false;
MediaCodec.BufferInfo bufferInfo = new MediaCodec.BufferInfo();

while (!consumeRotationChange() && !eof) {
int outputBufferId = codec.dequeueOutputBuffer(bufferInfo, -1);
while (!consumeRotationChange() && !consumeKeyFrameRequest() && !eof) {
// NOTE(frankleonrose): Timeout after 10ms to respond to key frame request.
int outputBufferId = codec.dequeueOutputBuffer(bufferInfo, 10000);
niamu marked this conversation as resolved.
Show resolved Hide resolved
eof = (bufferInfo.flags & MediaCodec.BUFFER_FLAG_END_OF_STREAM) != 0;
try {
if (consumeRotationChange()) {
niamu marked this conversation as resolved.
Show resolved Hide resolved
// must restart encoding with new size
break;
}
if (outputBufferId >= 0) {
if (outputBufferId >= 0) {
try {
boolean keyFrame = (bufferInfo.flags & MediaCodec.BUFFER_FLAG_KEY_FRAME) != 0;
if (keyFrame && requestKeyFrameTime>0) {
long now = SystemClock.uptimeMillis();
long ms = now - requestKeyFrameTime;
requestKeyFrameTime = 0;
Ln.d("Returning KEY FRAME after " + ms + "ms");
}

ByteBuffer codecBuffer = codec.getOutputBuffer(outputBufferId);

if (sendFrameMeta) {
Expand All @@ -168,9 +193,7 @@ private boolean encode(MediaCodec codec, FileDescriptor fd) throws IOException {
// If this is not a config packet, then it contains a frame
firstFrameSent = true;
}
}
} finally {
if (outputBufferId >= 0) {
} finally {
codec.releaseOutputBuffer(outputBufferId, false);
}
}
Expand Down