On Thu, 2016-12-08 at 23:27 +0100, Victor Toso wrote:
> From: Victor Toso <me@xxxxxxxxxxxxxx>
>
> We should replace the video_codecs GArray only after the parsing of
> input is done, otherwise we might lose previous configuration.
>
> Tests were updated to match this situation. Input that fails to
> replace video_codecs are considerer bad.
>
> Signed-off-by: Victor Toso <victortoso@xxxxxxxxxx>
> ---
> server/reds.c | 17 +++++++++++++----
> server/tests/test-codecs-parsing.c | 32
> ++++++++++++++++++++++++++++----
> 2 files changed, 41 insertions(+), 8 deletions(-)
>
> diff --git a/server/reds.c b/server/reds.c
> index 6064a6d..3b30928 100644
> --- a/server/reds.c
> +++ b/server/reds.c
> @@ -3538,6 +3538,7 @@ static const char* parse_video_codecs(const
> char *codecs, char **encoder,
> static void reds_set_video_codecs(RedsState *reds, const char
> *codecs)
> {
> char *encoder_name, *codec_name;
> + GArray *video_codecs;
>
> g_return_if_fail(codecs != NULL);
>
> @@ -3545,9 +3546,7 @@ static void reds_set_video_codecs(RedsState
> *reds, const char *codecs)
> codecs = default_video_codecs;
> }
>
> - /* The video_codecs array is immutable */
> - g_array_unref(reds->config->video_codecs);
> - reds->config->video_codecs = g_array_new(FALSE, FALSE,
> sizeof(RedVideoCodec));
> + video_codecs = g_array_new(FALSE, FALSE,
> sizeof(RedVideoCodec));
> const char *c = codecs;
> while ( (c = parse_video_codecs(c, &encoder_name, &codec_name))
> ) {
> uint32_t encoder_index, codec_index;
> @@ -3568,13 +3567,23 @@ static void reds_set_video_codecs(RedsState
> *reds, const char *codecs)
> new_codec.create = video_encoder_procs[encoder_index];
> new_codec.type = video_codec_names[codec_index].id;
> new_codec.cap = video_codec_caps[codec_index];
> - g_array_append_val(reds->config->video_codecs,
> new_codec);
> + g_array_append_val(video_codecs, new_codec);
> }
>
> free(encoder_name);
> free(codec_name);
> codecs = c;
> }
> +
> + if (video_codecs->len == 0) {
> + spice_warning("Failed to set video codecs, input string:
> '%s'", codecs);
> + g_array_unref(video_codecs);
> + return;
> + }
> +
> + /* The video_codecs array is immutable */
> + g_array_unref(reds->config->video_codecs);
> + reds->config->video_codecs = video_codecs;
> }
>
> SPICE_GNUC_VISIBLE int spice_server_init(SpiceServer *reds,
> SpiceCoreInterface *core)
> diff --git a/server/tests/test-codecs-parsing.c b/server/tests/test-
> codecs-parsing.c
> index c1b3276..5af2e5d 100644
> --- a/server/tests/test-codecs-parsing.c
> +++ b/server/tests/test-codecs-parsing.c
> @@ -28,9 +28,6 @@ static void codecs_good(void)
> {
> guint i;
> const gchar *codecs[] = {
> - "",
> - ";",
> - ";;;;",
Why do you remove these ?
> "spice:mjpeg",
> "spice:mjpeg;;;",
> "spice:mjpeg;;spice:mjpeg;;;",
> @@ -62,45 +59,70 @@ static void codecs_bad(void)
> const gchar *codecs;
> const GLogLevelFlags log_level;
> const gchar *error_message;
> + gboolean default_err_message;
> } test_cases[] = {
> {
> + "",
> + G_LOG_LEVEL_WARNING,
> + "*Failed to set video codecs*",
> + FALSE,
> + },{
> + ";",
> + G_LOG_LEVEL_WARNING,
> + "*Failed to set video codecs*",
> + FALSE,
> + },{
> + ";;;;",
> + G_LOG_LEVEL_WARNING,
> + "*Failed to set video codecs*",
> + FALSE,
> + },{
> NULL,
> G_LOG_LEVEL_CRITICAL,
> - "*assertion 'codecs != NULL' failed"
> + "*assertion 'codecs != NULL' failed",
> + FALSE,
> },{
> ";:;",
> G_LOG_LEVEL_WARNING,
> "*spice: invalid encoder:codec value*",
> + TRUE,
> },{
> "::::",
> G_LOG_LEVEL_WARNING,
> "*spice: invalid encoder:codec value*",
> + TRUE,
> },{
> "missingcolon",
> G_LOG_LEVEL_WARNING,
> "*spice: invalid encoder:codec value*",
> + TRUE,
> },{
> ":missing_encoder",
> G_LOG_LEVEL_WARNING,
> "*spice: invalid encoder:codec value*",
> + TRUE,
> },{
> "missing_value:;",
> G_LOG_LEVEL_WARNING,
> "*spice: invalid encoder:codec value*",
> + TRUE,
> },{
> "unknown_encoder:mjpeg",
> G_LOG_LEVEL_WARNING,
> "*spice: unknown video encoder unknown_encoder",
> + TRUE,
> },{
> "spice:unknown_codec",
> G_LOG_LEVEL_WARNING,
> "*spice: unknown video codec unknown_codec",
> + TRUE,
> },
> #if !defined(HAVE_GSTREAMER_1_0) && !defined(HAVE_GSTREAMER_0_10)
> {
> "gstreamer:mjpeg",
> G_LOG_LEVEL_WARNING,
> "*spice: unsupported video encoder gstreamer",
> + TRUE,
> }
> #endif
> };
> @@ -111,6 +133,8 @@ static void codecs_bad(void)
>
> for (i = 0; i < G_N_ELEMENTS(test_cases); ++i) {
> g_test_expect_message(G_LOG_DOMAIN,
> test_cases[i].log_level, test_cases[i].error_message);
> + if (test_cases[i].default_err_message)
> + g_test_expect_message(G_LOG_DOMAIN,
> G_LOG_LEVEL_WARNING, "*Failed to set video codecs*");
> g_assert_cmpint(spice_server_set_video_codecs(server,
> test_cases[i].codecs), ==, 0);
> g_test_assert_expected_messages();
> }
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel