[Spice-devel] [PATCH spice-server v2 4/6] Add an helper to test VideoEncoder
Frediano Ziglio
fziglio at redhat.com
Wed Nov 9 09:52:10 UTC 2016
> On Fri, Oct 21, 2016 at 01:40:38PM +0100, Frediano Ziglio wrote:
> > Add an utility to make possible to check various features of
> > VideoEncoder.
> > 2 GStreamer plugins are used in a chain like this:
> > (1) input pipeline -> (2) video encoder -> (3) output pipeline
> > While converting output from (1) is compared with output of (3)
> > making sure the streaming is working correctly.
>
> Maybe a short high level description could be added to the source of the
> test program too.
>
Yes, I'll put some additional comments too.
Or maybe in the usage description.
> For what it's worth, that's a lot of code, sometimes quite close to code
> already present in spice-server or spice-gtk. Hopefully the testcase by
> itself will not have too many bugs ;)
>
Well, in case of tests some bug are acceptable :-)
The first compiling version was not far from this one.
> > You can set various options:
> > - part of the input pipeline description to allow specifying different
> > video from GStreamer test ones to a video file;
> > - the encoder to use;
> > - different image properties to use for (2) input:
> > - different bit depth;
> > - top/down or down/up;
> > - initial bitrate.
> >
> > The idea is to use this helper in combination with a shell script
> > and some video sources to make able to test various settings.
> > Also can be used to extend the current encoder list.
> >
> > As an example you can use a command like
> >
> > $ ./gst-test -e gstreamer:vp8 -i \
> > 'filesrc location=bbb_sunflower_1080p_30fps_normal.mp4 \
> > ! decodebin ! videoconvert'
> >
> > to check vp8 encoding.
> >
> > Currently it does not emulate bandwidth changes as stream reports
> > from the client are not coded.
> >
> > Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
> > ---
> > server/tests/Makefile.am | 9 +
> > server/tests/gst-test.c | 936
> > +++++++++++++++++++++++++++++++++++++++++++++++
> > 2 files changed, 945 insertions(+)
> > create mode 100644 server/tests/gst-test.c
> >
> > Changes since v1:
> > - added option to split image into multiple chunks;
> > - added option to specify minimum PSNR;
> > - fix check for video finished;
> > - do not queue too much data to output;
> > - fix clipping;
> > - remove RFC.
> >
> > diff --git a/server/tests/Makefile.am b/server/tests/Makefile.am
> > index 8580a9a..c799779 100644
> > --- a/server/tests/Makefile.am
> > +++ b/server/tests/Makefile.am
> > @@ -57,6 +57,7 @@ noinst_PROGRAMS = \
> > test_vdagent \
> > test_display_width_stride \
> > spice-server-replay \
> > + gst-test \
> > $(check_PROGRAMS) \
> > $(NULL)
> >
> > @@ -105,3 +106,11 @@ libstat_test4_a_SOURCES = stat-test.c
> > libstat_test4_a_CPPFLAGS = $(AM_CPPFLAGS) -DTEST_COMPRESS_STAT=1
> > -DTEST_RED_WORKER_STAT=1 -DTEST_NAME=stat_test4
> >
> > test_qxl_parsing_LDADD = ../libserver.la $(LDADD)
> > +
> > +gst_test_SOURCES = gst-test.c \
> > + $(NULL)
> > +gst_test_CPPFLAGS = \
> > + $(AM_CPPFLAGS) \
> > + $(GSTREAMER_0_10_CFLAGS) \
> > + $(GSTREAMER_1_0_CFLAGS) \
> > + $(NULL)
> > diff --git a/server/tests/gst-test.c b/server/tests/gst-test.c
> > new file mode 100644
> > index 0000000..0a68d7d
> > --- /dev/null
> > +++ b/server/tests/gst-test.c
> > @@ -0,0 +1,936 @@
> > +/* -*- Mode: C; c-basic-offset: 4; indent-tabs-mode: nil -*- */
> > +/*
> > + Copyright (C) 2016 Red Hat, Inc.
> > +
> > + This library is free software; you can redistribute it and/or
> > + modify it under the terms of the GNU Lesser General Public
> > + License as published by the Free Software Foundation; either
> > + version 2.1 of the License, or (at your option) any later version.
> > +
> > + This library is distributed in the hope that it will be useful,
> > + but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> > + Lesser General Public License for more details.
> > +
> > + You should have received a copy of the GNU Lesser General Public
> > + License along with this library; if not, see
> > <http://www.gnu.org/licenses/>.
> > +*/
> > +/* Utility to check video encoder code
> > + */
> [snip]
> > +
> > +// handle output frames from input pipeline
> > +static void
> > +input_frames(GstSample *sample, void *param)
> > +{
> > + spice_assert(video_encoder && sample);
> > +
> > + if (SPICE_UNLIKELY(!clipping_type_computed)) {
> > + compute_clipping_rect(sample);
> > + }
> > +
> > + VideoBuffer *p_outbuf = NULL;
> > + // TODO correct ?? emulate another timer ??
> > + uint32_t frame_mm_time = reds_get_mm_time();
> > +
> > + // convert frame to SpiceBitmap/DRM prime
> > + TestFrame *frame = gst_to_spice_frame(sample);
> > +
> > + // send frame to our video encoder (must be from a single thread)
> > + int res = video_encoder->encode_frame(video_encoder, frame_mm_time,
> > frame->bitmap,
> > + &clipping_rect, top_down, frame,
> > + &p_outbuf);
> > + switch (res) {
> > + case VIDEO_ENCODER_FRAME_ENCODE_DONE:
> > + // save frame into queue for comparison later
> > + frame_ref(frame);
> > + pthread_mutex_lock(&frame_queue_mtx);
> > + g_queue_push_tail(&frame_queue, frame);
>
> Have you looked at using GAsyncQueue rather than handling the locking
> manually?
>
Was a GAsyncQueue. But I added the locking to support limiting the queue.
For some reason the queues was growing a bit too much (till my machine
was going out of memory!). I'm not debugging gstreamer itself so I coded
a manual limit. The locking was required to avoid races using the condition.
> > + while (g_queue_get_length(&frame_queue) >= 16) {
> > + pthread_cond_wait(&frame_queue_cond, &frame_queue_mtx);
> > + }
> > + pthread_mutex_unlock(&frame_queue_mtx);
> > + spice_assert(p_outbuf);
> > + pipeline_send_raw_data(output_pipeline, p_outbuf);
> > + break;
> > + case VIDEO_ENCODER_FRAME_UNSUPPORTED:
> > + // ?? what to do ??
> > + // should not happen, format passed should be supported
> > + // could happen for serious problems and encoder gave up
> > + spice_assert(0);
> > + break;
> > + case VIDEO_ENCODER_FRAME_DROP:
> > + break;
> > + default:
> > + // invalid value returned
> > + spice_assert(0);
> > + }
> > +
> > + // TODO call client_stream_report to simulate this report from the
> > client
> > +
> > + frame_unref(frame);
> > +}
> > +
> > +// handle output frames from output pipeline
> > +static void
> > +output_frames(GstSample *sample, void *param)
> > +{
> > + TestFrame *curr_frame = gst_to_spice_frame(sample);
> > +
> > + // get first frame queued
> > + pthread_mutex_lock(&frame_queue_mtx);
> > + TestFrame *expected_frame = g_queue_pop_head(&frame_queue);
> > + pthread_cond_signal(&frame_queue_cond);
> > + pthread_mutex_unlock(&frame_queue_mtx);
> > + if (!expected_frame) {
> > + g_printerr("Frame not present in the queue but arrived in
> > output!\n");
> > + exit(1);
> > + }
> > +
> > + // TODO try to understand if this is correct
> > + if (!top_down) {
> > + curr_frame->bitmap->flags ^= SPICE_BITMAP_FLAGS_TOP_DOWN;
> > + }
> > +#ifdef DUMP_BITMAP
> > + dump_bitmap(expected_frame->bitmap);
> > + dump_bitmap(curr_frame->bitmap);
> > +#endif
> > +
> > + // compute difference
> > + double psnr = compute_psnr(expected_frame->bitmap, clipping_rect.left,
> > clipping_rect.top,
> > + curr_frame->bitmap, 0, 0,
> > + clipping_rect.right - clipping_rect.left,
> > + clipping_rect.bottom - clipping_rect.top);
> > +
> > + // check is more or less the same
> > + if (psnr < minimum_psnr) {
> > + g_printerr("Frame PSNR too low, got %g minimum %g\n", psnr,
> > minimum_psnr);
> > + exit(1);
> > + }
> > +
> > + frame_unref(expected_frame);
> > + frame_unref(curr_frame);
> > +}
> > +
> > +static const EncoderInfo encoder_infos[] = {
> > + { "mjpeg", mjpeg_encoder_new, SPICE_VIDEO_CODEC_TYPE_MJPEG,
> > + "caps=image/jpeg ! jpegdec" },
> > + { "gstreamer:mjpeg", gstreamer_encoder_new,
> > SPICE_VIDEO_CODEC_TYPE_MJPEG,
> > + "caps=image/jpeg ! jpegdec" },
> > + { "gstreamer:vp8", gstreamer_encoder_new,
> > SPICE_VIDEO_CODEC_TYPE_VP8,
> > + "caps=video/x-vp8 ! vp8dec" },
> > + { "gstreamer:h264", gstreamer_encoder_new,
> > SPICE_VIDEO_CODEC_TYPE_H264,
> > + "! h264parse ! avdec_h264" },
> > + { NULL, NULL }
>
> Fwiw, the way it's done in spice-gtk with separate caps and decoder
> plugin name is easier to read imo (because the naming is easier)
>
I can split it.
> > +};
> > +
> > +int main(int argc, char *argv[])
> > +{
> > + gchar *input_pipeline_desc = NULL;
> > + gchar *image_format = "32BIT";
> > + gchar *encoder_name = "mjpeg";
> > + gboolean use_hw_encoder = FALSE; // TODO use
> > + gchar *clipping = "(0,0)x(100%,100%)";
> > +
> > + // - input pipeline
> > + // - top/down
> > + // - format for video encoder input (bits, rgb/bgr)
> > + // - encoder (mjpeg/vp8/h264)
> > + // - use h/w acceleration (if available)
> > + // - clipping (part of the source)
> > + // - TODO bandwidth changes?
> > + // - TODO fps ??
> > + GOptionEntry entries[] = {
> > + { "input-pipeline", 'i', 0, G_OPTION_ARG_STRING,
> > &input_pipeline_desc,
> > + "GStreamer input pipeline", "PIPELINE" },
> > + { "top-down", 0, 0, G_OPTION_ARG_NONE, &top_down,
> > + "Image encoded as top-down", NULL },
> > + { "format", 'f', 0, G_OPTION_ARG_STRING, &image_format,
> > + "Image format (16BIT/24BIT/32BIT/RGBA)", "FMT" },
> > + { "encoder", 'e', 0, G_OPTION_ARG_STRING, &encoder_name,
> > + "Encoder to use", "ENC" },
> > + { "use-hw-encoder", 0, 0, G_OPTION_ARG_NONE, &use_hw_encoder,
> > + "Use H/W encoders if possible", NULL },
> > + { "clipping", 0, 0, G_OPTION_ARG_STRING, &clipping,
> > + "Clipping region (x1,y1)-(x2,y2) or (x,y)x(w,h)", "STRING" },
>
> This description does not seem to match the format of the default value
> above ("(0,0)x(100%,100%)")
>
The (0,0)x(100%,100%) match (x,y)x(w,h), it's just that w and h are
specified with percentage instead of pixels.
Maybe I can describe the pixel usage...
> > + { "starting-bitrate", 0, 0, G_OPTION_ARG_INT64,
> > &starting_bit_rate,
> > + "Initial bitrate", "BITRATE" },
> > + { "min-psnr", 0, 0, G_OPTION_ARG_DOUBLE, &minimum_psnr,
> > + "Minimum PSNR accepted", "PSNR" },
> > + { "split-lines", 0, 0, G_OPTION_ARG_INT, &image_split_lines,
> > + "Split image into different chunks every LINES lines", "LINES"
> > },
> > + { NULL }
> > + };
> > +
> > + GOptionContext *context = NULL;
> > + GError *error = NULL;
> > + context = g_option_context_new("- helper for testing VideoEncoder");
> > + g_option_context_add_main_entries(context, entries, NULL);
> > + if (!g_option_context_parse(context, &argc, &argv, &error)) {
> > + g_printerr("Option parsing failed: %s\n", error->message);
> > + exit(1);
> > + }
> > +
> > + if (!input_pipeline_desc) {
> > + g_printerr("Input pipeline option missing\n");
> > + exit(1);
> > + }
> > +
> > + if (!encoder_name) {
> > + g_printerr("Encoder name option missing\n");
> > + exit(1);
> > + }
> > +
> > + const EncoderInfo *encoder = get_encoder_info(encoder_name);
> > + if (!encoder) {
> > + g_printerr("Encoder name unsupported: %s\n", encoder_name);
> > + exit(1);
> > + }
> > +
> > + bitmap_format = get_bitmap_format(image_format);
> > + if (bitmap_format == SPICE_BITMAP_FMT_INVALID) {
> > + g_printerr("Invalid image format: %s\n", image_format);
> > + exit(1);
> > + }
> > +
> > + parse_clipping(clipping);
> > +
> > + if (minimum_psnr < 0) {
> > + g_printerr("Invalid PSNR specified %f\n", minimum_psnr);
> > + exit(1);
> > + }
> > +
> > + if (image_split_lines < 1) {
> > + g_printerr("Invalid --split-lines option: %d\n",
> > image_split_lines);
> > + exit(1);
> > + }
> > +
> > + gst_init(&argc, &argv);
> > +
> > + // TODO give particular error if pipeline fails to be created
> > +
> > + create_output_pipeline(encoder, output_frames, NULL);
> > +
> > + create_video_encoder(encoder);
> > +
> > + create_input_pipeline(input_pipeline_desc, input_frames, NULL);
> > +
> > + // run all input streaming
> > + pipeline_wait_eos(input_pipeline);
> > +
> > + video_encoder->destroy(video_encoder);
> > +
> > + // send EOS to output and wait
> > + // this assure we processed all frames sent from input pipeline
> > + if (gst_app_src_end_of_stream(output_pipeline->appsrc) != GST_FLOW_OK)
> > {
> > + g_printerr("gst_app_src_end_of_stream failed\n");
> > + exit(1);
> > + }
> > + pipeline_wait_eos(output_pipeline);
> > +
> > + // check queue is now empty
> > + pthread_mutex_lock(&frame_queue_mtx);
> > + TestFrame *frame = g_queue_pop_head(&frame_queue);
> > + pthread_mutex_unlock(&frame_queue_mtx);
> > + if (frame) {
> > + g_printerr("Queue not empty at the end\n");
> > + exit(1);
> > + }
> > +
> > + return 0;
> > +}
> > +
>
> [snip]
>
> > +static TestPipeline*
> > +create_pipeline(const char *desc, SampleProc sample_proc, void *param)
> > +{
> > + TestPipeline *pipeline = spice_new0(TestPipeline, 1);
> > +
> > + pipeline->sample_proc = sample_proc;
> > + pipeline->sample_param = param;
> > +
> > + GError *err = NULL;
> > + pipeline->gst_pipeline = gst_parse_launch_full(desc, NULL,
> > GST_PARSE_FLAG_FATAL_ERRORS, &err);
> > + if (!pipeline->gst_pipeline) {
> > + g_printerr("GStreamer error: %s\n", err->message);
> > + return NULL;
> > + }
> > +
> > + pipeline->appsrc =
> > GST_APP_SRC(gst_bin_get_by_name(GST_BIN(pipeline->gst_pipeline), "src"));
> > + pipeline->appsink =
> > GST_APP_SINK(gst_bin_get_by_name(GST_BIN(pipeline->gst_pipeline),
> > "sink"));
> > + if (!pipeline->appsink) {
> > + g_printerr("Appsync not found in pipeline: %s\n", desc);
> > + return NULL;
> > + }
> > +
> > + GstAppSinkCallbacks appsink_cbs = { NULL, NULL, new_sample };
> > + gst_app_sink_set_callbacks(pipeline->appsink, &appsink_cbs, pipeline,
> > NULL);
> > +
> > + GstBus *bus = gst_element_get_bus(pipeline->gst_pipeline);
> > + gst_bus_set_sync_handler(bus, handle_pipeline_message, pipeline,
> > NULL);
>
> Do we have to use gst_bus_set_sync_handler? The documentation recommends
> to use gst_bus_watch or _poll functions.
>
I think I used spice-server/spice-gtk as a reference.
> > + gst_object_unref(bus);
> > +
> > + if (gst_element_set_state(pipeline->gst_pipeline, GST_STATE_PLAYING)
> > ==
> > + GST_STATE_CHANGE_FAILURE) {
> > + g_printerr("GStreamer error: Unable to set the pipeline to the
> > playing state.\n");
> > + exit(1);
> > + }
> > +
> > + return pipeline;
> > +}
> > +
> [snip]
> > +
> > +static SpiceBitmapFmt
> > +get_bitmap_format(const char *format)
> > +{
> > +#define FMT(fmt) if (strcmp(format, #fmt) == 0) { return SPICE_BITMAP_FMT_
> > ## fmt; }
> > + FMT(32BIT);
> > + FMT(24BIT);
> > + FMT(16BIT);
> > + FMT(RGBA);
> > +#undef FMT
>
> No macro here please, the expanded version is not much longer, and far
> more readable.
>
sed will be my friend :-)
> > + return SPICE_BITMAP_FMT_INVALID;
> > +}
>
> Christophe
>
Frediano
More information about the Spice-devel
mailing list