> 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@xxxxxxxxxx> > > --- > > 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 _______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel