[Spice-devel] [PATCH spice-server 08/28] mjpeg_encoder: move the control over frame drops to mjpeg_encoder
Yonit Halperin
yhalperi at redhat.com
Mon Apr 15 11:28:38 PDT 2013
On 04/14/2013 09:21 AM, Alon Levy wrote:
> On Tue, Feb 26, 2013 at 01:03:54PM -0500, Yonit Halperin wrote:
>
> ACK with one comment.
>
>> ---
>> server/mjpeg_encoder.c | 20 +++++++++-----------
>> server/mjpeg_encoder.h | 21 ++++++++++++++-------
>> server/red_worker.c | 21 ++++++++++++++++-----
>> 3 files changed, 39 insertions(+), 23 deletions(-)
>>
>> diff --git a/server/mjpeg_encoder.c b/server/mjpeg_encoder.c
>> index 70b6338..c124286 100644
>> --- a/server/mjpeg_encoder.c
>> +++ b/server/mjpeg_encoder.c
>> @@ -641,9 +641,15 @@ int mjpeg_encoder_start_frame(MJpegEncoder *encoder, SpiceBitmapFmt format,
>> MJpegEncoderRateControl *rate_control = &encoder->rate_control;
>> struct timespec time;
>> uint64_t now;
>> + uint64_t interval;
>>
>> clock_gettime(CLOCK_MONOTONIC, &time);
>> now = ((uint64_t) time.tv_sec) * 1000000000 + time.tv_nsec;
>> + interval = (now - rate_control->bit_rate_info.last_frame_time);
>> +
>> + if (interval < (1000*1000*1000) / rate_control->fps) {
>> + return MJPEG_ENCODER_FRAME_DROP;
>> + }
>>
>> mjpeg_encoder_adjust_params_to_bit_rate(encoder);
>>
>> @@ -690,14 +696,14 @@ int mjpeg_encoder_start_frame(MJpegEncoder *encoder, SpiceBitmapFmt format,
>> break;
>> default:
>> spice_warning("unsupported format %d", format);
>> - return FALSE;
>> + return MJPEG_ENCODER_FRAME_UNSUPPORTED;
>> }
>>
>> if (encoder->pixel_converter != NULL) {
>> unsigned int stride = width * 3;
>> /* check for integer overflow */
>> if (stride < width) {
>> - return FALSE;
>> + return MJPEG_ENCODER_FRAME_UNSUPPORTED;
>> }
>> if (encoder->row_size < stride) {
>> encoder->row = spice_realloc(encoder->row, stride);
>> @@ -715,7 +721,7 @@ int mjpeg_encoder_start_frame(MJpegEncoder *encoder, SpiceBitmapFmt format,
>> jpeg_set_quality(&encoder->cinfo, quality, TRUE);
>> jpeg_start_compress(&encoder->cinfo, encoder->first_frame);
>>
>> - return TRUE;
>> + return MJPEG_ENCODER_FRAME_ENCODE_START;
>> }
>>
>> int mjpeg_encoder_encode_scanline(MJpegEncoder *encoder, uint8_t *src_pixels,
>> @@ -773,14 +779,6 @@ size_t mjpeg_encoder_end_frame(MJpegEncoder *encoder)
>> return encoder->rate_control.last_enc_size;
>> }
>>
>> -uint32_t mjpeg_encoder_get_fps(MJpegEncoder *encoder)
>> -{
>> - if (!encoder->rate_control_is_active) {
>> - spice_warning("bit rate control is not active");
>> - }
>> - return encoder->rate_control.fps;
>> -}
>> -
>> static void mjpeg_encoder_quality_eval_stop(MJpegEncoder *encoder)
>> {
>> MJpegEncoderRateControl *rate_control = &encoder->rate_control;
>> diff --git a/server/mjpeg_encoder.h b/server/mjpeg_encoder.h
>> index f9ae43c..0ee2e96 100644
>> --- a/server/mjpeg_encoder.h
>> +++ b/server/mjpeg_encoder.h
>> @@ -21,6 +21,12 @@
>>
>> #include "red_common.h"
>>
>> +enum {
>> + MJPEG_ENCODER_FRAME_UNSUPPORTED = -1,
>> + MJPEG_ENCODER_FRAME_DROP,
>> + MJPEG_ENCODER_FRAME_ENCODE_START,
>> +};
>> +
>> typedef struct MJpegEncoder MJpegEncoder;
>>
>> /*
>> @@ -44,8 +50,15 @@ void mjpeg_encoder_destroy(MJpegEncoder *encoder);
>> uint8_t mjpeg_encoder_get_bytes_per_pixel(MJpegEncoder *encoder);
>>
>> /*
>> - * *dest must be either NULL or allocated by malloc, since it might be freed
>> + * dest must be either NULL or allocated by malloc, since it might be freed
>> * during the encoding, if its size is too small.
>> + *
>> + * return:
>> + * MJPEG_ENCODER_FRAME_UNSUPPORTED : frame cannot be encoded
>> + * MJPEG_ENCODER_FRAME_DROP : frame should be dropped. This value can only be returned
>> + * if mjpeg rate control is active.
>> + * MJPEG_ENCODER_FRAME_ENCODE_START: frame encoding started. Continue with
>> + * mjpeg_encoder_encode_scanline.
>> */
>> int mjpeg_encoder_start_frame(MJpegEncoder *encoder, SpiceBitmapFmt format,
>> int width, int height,
>> @@ -60,12 +73,6 @@ size_t mjpeg_encoder_end_frame(MJpegEncoder *encoder);
>> */
>>
>> /*
>> - * The recommended output frame rate (per second) for the
>> - * current available bit rate.
>> - */
>> -uint32_t mjpeg_encoder_get_fps(MJpegEncoder *encoder);
>> -
>> -/*
>> * Data that should be periodically obtained from the client. The report contains:
>> * num_frames : the number of frames that reached the client during the time
>> * the report is referring to.
>> diff --git a/server/red_worker.c b/server/red_worker.c
>> index 568b8e5..a390629 100644
>> --- a/server/red_worker.c
>> +++ b/server/red_worker.c
>> @@ -8352,6 +8352,7 @@ static inline int red_marshall_stream_data(RedChannelClient *rcc,
>> RedWorker *worker = dcc->common.worker;
>> int n;
>> int width, height;
>> + int ret;
>>
>> if (!stream) {
>> spice_assert(drawable->sized_stream);
>> @@ -8389,13 +8390,23 @@ static inline int red_marshall_stream_data(RedChannelClient *rcc,
>> }
>>
>> outbuf_size = dcc->send_data.stream_outbuf_size;
>> - if (!mjpeg_encoder_start_frame(agent->mjpeg_encoder, image->u.bitmap.format,
>> - width, height,
>> - &dcc->send_data.stream_outbuf,
>> - &outbuf_size,
>> - drawable->red_drawable->mm_time)) {
>> + ret = mjpeg_encoder_start_frame(agent->mjpeg_encoder, image->u.bitmap.format,
>> + width, height,
>> + &dcc->send_data.stream_outbuf,
>> + &outbuf_size,
>> + drawable->red_drawable->mm_time);
>> + switch (ret) {
>> + case MJPEG_ENCODER_FRAME_DROP:
>> + spice_warning("mjpeg rate control is not supported yet");
>> + return TRUE;
>> + case MJPEG_ENCODER_FRAME_UNSUPPORTED:
>> return FALSE;
>> + case MJPEG_ENCODER_FRAME_ENCODE_START:
>> + break;
>> + default:
>> + spice_error("bad return value (%d) from mjpeg_encoder_start_frame", ret);
>
> Why don't we return here?
>
Just forgot. I Will add it.
>> }
>> +
>> if (!encode_frame(dcc, &drawable->red_drawable->u.copy.src_area,
>> &image->u.bitmap, stream)) {
>> return FALSE;
>> --
>> 1.8.1
>>
>> _______________________________________________
>> Spice-devel mailing list
>> Spice-devel at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/spice-devel
More information about the Spice-devel
mailing list