server/shadow: shadow encoder related enhancement/fix.

1. Export fps related API so that subsystem implementation no longer need to know about details in encoder structure.
2. Discard frameList dictionary.
The 'value' in this dictionary is never used and not properly free'ed when client is disconnected.
The dictionary was used to calculate 'inflight' frame count. Once an ACK is received from client, an item in the dictionary is removed.
We then calculate 'inflight' frame by the count of the items in the dictionary.
However, some rdp clients (win7 mstsc) skips frame ACK if it is inactive, ACK of some frame would actually never arrive.
We actually don't need the dictionary. We only need to record the latest acknowledged frame id, and the difference between last sent frame id is the inflight frame count.
3. Minor fix in default fps calculation. encoder->frameAck is wrongly used as integer while it's actually bool flag.
This commit is contained in:
zihao.jiang 2015-04-10 02:33:54 +08:00
parent 9fab504862
commit e00655c3c2
6 changed files with 55 additions and 62 deletions

View File

@ -279,6 +279,9 @@ FREERDP_API BOOL shadow_client_post_msg(rdpShadowClient* client, void* context,
FREERDP_API int shadow_client_boardcast_msg(rdpShadowServer* server, void* context, UINT32 type, SHADOW_MSG_OUT* msg, void* lParam);
FREERDP_API int shadow_client_boardcast_quit(rdpShadowServer* server, int nExitCode);
FREERDP_API int shadow_encoder_preferred_fps(rdpShadowEncoder* encoder);
FREERDP_API UINT32 shadow_encoder_inflight_frames(rdpShadowEncoder* encoder);
#ifdef __cplusplus
}
#endif

View File

@ -29,7 +29,6 @@
#include "../shadow_client.h"
#include "../shadow_surface.h"
#include "../shadow_capture.h"
#include "../shadow_encoder.h"
#include "../shadow_subsystem.h"
#include "../shadow_mcevent.h"
@ -377,7 +376,7 @@ void (^mac_capture_stream_handler)(CGDisplayStreamFrameStatus, uint64_t, IOSurfa
if (client)
{
subsystem->captureFrameRate = client->encoder->fps;
subsystem->captureFrameRate = shadow_encoder_preferred_fps(client->encoder);
}
}

View File

@ -42,7 +42,6 @@
#include "../shadow_screen.h"
#include "../shadow_client.h"
#include "../shadow_encoder.h"
#include "../shadow_capture.h"
#include "../shadow_surface.h"
#include "../shadow_subsystem.h"
@ -735,7 +734,7 @@ int x11_shadow_screen_grab(x11ShadowSubsystem* subsystem)
if (client)
{
subsystem->captureFrameRate = client->encoder->fps;
subsystem->captureFrameRate = shadow_encoder_preferred_fps(client->encoder);
}
}

View File

@ -357,17 +357,16 @@ BOOL shadow_client_activate(freerdp_peer* peer)
BOOL shadow_client_surface_frame_acknowledge(rdpShadowClient* client, UINT32 frameId)
{
SURFACE_FRAME* frame;
wListDictionary* frameList;
/*
* Record the last client acknowledged frame id to
* calculate how much frames are in progress.
* Some rdp clients (win7 mstsc) skips frame ACK if it is
* inactive, we should not expect ACK for each frame.
* So it is OK to calculate inflight frame count according to
* a latest acknowledged frame id.
*/
client->encoder->lastAckframeId = frameId;
frameList = client->encoder->frameList;
frame = (SURFACE_FRAME*) ListDictionary_GetItemValue(frameList, (void*) (size_t) frameId);
if (frame)
{
ListDictionary_Remove(frameList, (void*) (size_t) frameId);
free(frame);
}
return TRUE;
}
@ -428,7 +427,7 @@ int shadow_client_send_surface_bits(rdpShadowClient* client, rdpShadowSurface* s
}
if (encoder->frameAck)
frameId = (UINT32) shadow_encoder_create_frame_id(encoder);
frameId = shadow_encoder_create_frame_id(encoder);
if (settings->RemoteFxCodec)
{

View File

@ -24,15 +24,37 @@
#include "shadow_encoder.h"
int shadow_encoder_create_frame_id(rdpShadowEncoder* encoder)
int shadow_encoder_preferred_fps(rdpShadowEncoder* encoder)
{
/* Return preferred fps calculated according to the last
* sent frame id and last client-acknowledged frame id.
*/
return encoder->fps;
}
UINT32 shadow_encoder_inflight_frames(rdpShadowEncoder* encoder)
{
/* Return inflight frame count =
* <last sent frame id> - <last client-acknowledged frame id>
* Note: This function is exported so that subsystem could
* implement its own strategy to tune fps.
*/
return encoder->frameId - encoder->lastAckframeId;
}
UINT32 shadow_encoder_create_frame_id(rdpShadowEncoder* encoder)
{
UINT32 frameId;
int inFlightFrames;
SURFACE_FRAME* frame;
inFlightFrames = ListDictionary_Count(encoder->frameList);
inFlightFrames = shadow_encoder_inflight_frames(encoder);
if (inFlightFrames > encoder->frameAck)
/*
* Calculate preferred fps according to how much frames are
* in-progress. Note that it only works when subsytem implementation
* calls shadow_encoder_preferred_fps and takes the suggestion.
*/
if (inFlightFrames > 1)
{
encoder->fps = (100 / (inFlightFrames + 1) * encoder->maxFps) / 100;
}
@ -47,16 +69,9 @@ int shadow_encoder_create_frame_id(rdpShadowEncoder* encoder)
if (encoder->fps < 1)
encoder->fps = 1;
frame = (SURFACE_FRAME*) malloc(sizeof(SURFACE_FRAME));
frameId = ++encoder->frameId;
if (!frame)
return -1;
frameId = frame->frameId = ++encoder->frameId;
if (!ListDictionary_Add(encoder->frameList, (void*) (size_t) frame->frameId, frame))
return -1;
return (int) frame->frameId;
return frameId;
}
int shadow_encoder_init_grid(rdpShadowEncoder* encoder)
@ -130,16 +145,11 @@ int shadow_encoder_init_rfx(rdpShadowEncoder* encoder)
rfx_context_set_pixel_format(encoder->rfx, RDP_PIXEL_FORMAT_B8G8R8A8);
if (!encoder->frameList)
{
encoder->fps = 16;
encoder->maxFps = 32;
encoder->frameId = 0;
encoder->frameList = ListDictionary_New(TRUE);
if (!encoder->frameList)
return -1;
encoder->lastAckframeId = 0;
encoder->frameAck = settings->SurfaceFrameMarkerEnabled;
}
encoder->codecs |= FREERDP_CODEC_REMOTEFX;
@ -159,16 +169,11 @@ int shadow_encoder_init_nsc(rdpShadowEncoder* encoder)
nsc_context_set_pixel_format(encoder->nsc, RDP_PIXEL_FORMAT_B8G8R8A8);
if (!encoder->frameList)
{
encoder->fps = 16;
encoder->maxFps = 32;
encoder->frameId = 0;
encoder->frameList = ListDictionary_New(TRUE);
if (!encoder->frameList)
return -1;
encoder->lastAckframeId = 0;
encoder->frameAck = settings->SurfaceFrameMarkerEnabled;
}
encoder->nsc->ColorLossLevel = settings->NSCodecColorLossLevel;
encoder->nsc->ChromaSubsamplingLevel = settings->NSCodecAllowSubsampling ? 1 : 0;
@ -241,12 +246,6 @@ int shadow_encoder_uninit_rfx(rdpShadowEncoder* encoder)
encoder->rfx = NULL;
}
if (encoder->frameList)
{
ListDictionary_Free(encoder->frameList);
encoder->frameList = NULL;
}
encoder->codecs &= ~FREERDP_CODEC_REMOTEFX;
return 1;
@ -260,12 +259,6 @@ int shadow_encoder_uninit_nsc(rdpShadowEncoder* encoder)
encoder->nsc = NULL;
}
if (encoder->frameList)
{
ListDictionary_Free(encoder->frameList);
encoder->frameList = NULL;
}
encoder->codecs &= ~FREERDP_CODEC_NSCODEC;
return 1;

View File

@ -54,7 +54,7 @@ struct rdp_shadow_encoder
int maxFps;
BOOL frameAck;
UINT32 frameId;
wListDictionary* frameList;
UINT32 lastAckframeId;
};
#ifdef __cplusplus
@ -63,7 +63,7 @@ extern "C" {
int shadow_encoder_reset(rdpShadowEncoder* encoder);
int shadow_encoder_prepare(rdpShadowEncoder* encoder, UINT32 codecs);
int shadow_encoder_create_frame_id(rdpShadowEncoder* encoder);
UINT32 shadow_encoder_create_frame_id(rdpShadowEncoder* encoder);
rdpShadowEncoder* shadow_encoder_new(rdpShadowClient* client);
void shadow_encoder_free(rdpShadowEncoder* encoder);