From ecb534241033e46520409b1c80eb1a266878fa0f Mon Sep 17 00:00:00 2001 From: sirknightj Date: Sun, 26 Jan 2025 23:06:16 -0800 Subject: [PATCH] Add runtime string checks and additional unit tests --- .github/workflows/kvssink.yml | 2 +- src/gstreamer/gstkvssink.cpp | 38 ++++++++++++++++++++++-- tst/gstreamer/gstkvstest.cpp | 55 +++++++++++++++++++++++++++++++++-- 3 files changed, 89 insertions(+), 6 deletions(-) diff --git a/.github/workflows/kvssink.yml b/.github/workflows/kvssink.yml index bff03c33..10d104ed 100644 --- a/.github/workflows/kvssink.yml +++ b/.github/workflows/kvssink.yml @@ -65,4 +65,4 @@ jobs: working-directory: ./build run: | export GST_PLUGIN_PATH=`pwd` - ./tst/gstkvsplugintest + GST_DEBUG=4 ./tst/gstkvsplugintest diff --git a/src/gstreamer/gstkvssink.cpp b/src/gstreamer/gstkvssink.cpp index 83b9a4b1..00bb5d8c 100644 --- a/src/gstreamer/gstkvssink.cpp +++ b/src/gstreamer/gstkvssink.cpp @@ -137,6 +137,8 @@ GST_DEBUG_CATEGORY_STATIC (gst_kvs_sink_debug); #define MAX_GSTREAMER_MEDIA_TYPE_LEN 16 +#define INTERNAL_CHECK_PREFIX "Assertion failed:" + namespace KvsSinkSignals { guint err_signal_id; guint ack_signal_id; @@ -261,6 +263,11 @@ void closed(UINT64 custom_data, STREAM_HANDLE stream_handle, UPLOAD_HANDLE uploa void kinesis_video_producer_init(GstKvsSink *kvssink) { auto data = kvssink->data; + + if (kvssink->stream_name == NULL) { + throw runtime_error(string(INTERNAL_CHECK_PREFIX) + " kvssink->stream_name is unexpectedly NULL!"); + } + unique_ptr device_info_provider(new KvsSinkDeviceInfoProvider(kvssink->storage_size, kvssink->stop_stream_timeout, kvssink->service_connection_timeout, @@ -302,11 +309,11 @@ void kinesis_video_producer_init(GstKvsSink *kvssink) } } else { - access_key_str = string(kvssink->access_key); - secret_key_str = string(kvssink->secret_key); + access_key_str = kvssink->access_key ? string(kvssink->access_key) : ""; + secret_key_str = kvssink->secret_key ? string(kvssink->secret_key) : ""; } - // Handle session token seperately, since this is optional with long term credentials + // Handle session token separately, since this is optional with long term credentials if (0 == strcmp(kvssink->session_token, DEFAULT_SESSION_TOKEN)) { session_token_str = ""; if (nullptr != (session_token = getenv(SESSION_TOKEN_ENV_VAR))) { @@ -370,6 +377,11 @@ void kinesis_video_producer_init(GstKvsSink *kvssink) LOG_INFO("Getting URL from env for " << kvssink->stream_name); control_plane_uri_str = string(control_plane_uri); } + + if (kvssink->user_agent == NULL) { + throw runtime_error(string(INTERNAL_CHECK_PREFIX) + " kvssink->user_agent is unexpectedly NULL!"); + } + LOG_INFO("User agent string: " << kvssink->user_agent); data->kinesis_video_producer = KinesisVideoProducer::createSync(std::move(device_info_provider), std::move(client_callback_provider), @@ -423,6 +435,13 @@ void create_kinesis_video_stream(GstKvsSink *kvssink) { break; } + // StreamDefinition takes in C++ strings, check the gchar* for nullptr before constructing + if (kvssink->stream_name == NULL || kvssink->content_type == NULL || + kvssink->user_agent == NULL || kvssink->kms_key_id == NULL || + kvssink->codec_id == NULL || kvssink->track_name == NULL) { + throw runtime_error(string(INTERNAL_CHECK_PREFIX) + " A string was unexpectedly NULL!"); + } + unique_ptr stream_definition(new StreamDefinition(kvssink->stream_name, hours(kvssink->retention_period_hours), p_stream_tags, @@ -476,6 +495,15 @@ bool kinesis_video_stream_init(GstKvsSink *kvssink, string &err_msg) { create_kinesis_video_stream(kvssink); break; } catch (runtime_error &err) { + + // Don't retry if the error is an internal error + if (STRNCMP(INTERNAL_CHECK_PREFIX, err.what(), STRLEN(INTERNAL_CHECK_PREFIX)) == 0) { + ostringstream oss; + oss << "Failed to create stream. Error: " << err.what(); + err_msg = oss.str(); + return false; + } + if (--retry_count == 0) { ostringstream oss; oss << "Failed to create stream. Error: " << err.what(); @@ -1569,6 +1597,10 @@ init_track_data(GstKvsSink *kvssink) { g_free(video_content_type); g_free(audio_content_type); + + if (kvssink->content_type == NULL) { + throw runtime_error(string(INTERNAL_CHECK_PREFIX) + " kvssink->content_type is unexpectedly NULL!"); + } } static GstStateChangeReturn diff --git a/tst/gstreamer/gstkvstest.cpp b/tst/gstreamer/gstkvstest.cpp index f8653c28..3e7ecfde 100644 --- a/tst/gstreamer/gstkvstest.cpp +++ b/tst/gstreamer/gstkvstest.cpp @@ -17,8 +17,9 @@ static char const *sessionToken; static GstElement * setup_kinesisvideoproducersink(void) { + cout << "setup_kinesisvideoproducersink() start" << endl; + GstElement *kinesisvideoproducersink; - GST_DEBUG("Setup kinesisvideoproducersink"); kinesisvideoproducersink = gst_check_setup_element ("kvssink"); fail_unless(kinesisvideoproducersink != nullptr, "Failed to create kvssink element (is GST_PLUGIN_PATH set?)"); @@ -30,16 +31,19 @@ setup_kinesisvideoproducersink(void) // https://gitlab.freedesktop.org/gstreamer/gst-docs/-/issues/91 // Use gst_element_request_pad_simple in newer GStreamer versions - GstPad *sinkpad = gst_element_get_request_pad(kinesisvideoproducersink, "video_0"); + GstPad *sinkpad = gst_element_get_request_pad(kinesisvideoproducersink, "video_%u"); fail_unless(sinkpad != nullptr, "Failed to request video pad"); gst_object_unref(sinkpad); + cout << "setup_kinesisvideoproducersink() end" << endl; return kinesisvideoproducersink; } static void cleanup_kinesisvideoproducersink(GstElement * kinesisvideoproducersink) { + cout << "cleanup_kinesisvideoproducersink() start" << endl; + GstPad *sinkpad = gst_element_get_static_pad(kinesisvideoproducersink, "video_0"); if (sinkpad) { gst_element_release_request_pad(kinesisvideoproducersink, sinkpad); @@ -47,6 +51,8 @@ cleanup_kinesisvideoproducersink(GstElement * kinesisvideoproducersink) } gst_check_teardown_element (kinesisvideoproducersink); + + cout << "cleanup_kinesisvideoproducersink() end" << endl; } GST_START_TEST(kvsproducersinktestplayandstop) @@ -101,6 +107,49 @@ GST_START_TEST(kvsproducersinkteststop) } GST_END_TEST; +GST_START_TEST(check_stream_name_null_handled) + { + GstElement *kinesisvideoproducersink; + + // Setup + kinesisvideoproducersink = gst_check_setup_element ("kvssink"); + fail_unless(kinesisvideoproducersink != nullptr, "Failed to create kvssink element"); + + g_object_set(G_OBJECT (kinesisvideoproducersink), + "stream-name", nullptr, + NULL); + + // Test - Initialization of the client & stream occurs here + fail_unless_equals_int(GST_STATE_CHANGE_FAILURE, gst_element_set_state(kinesisvideoproducersink, GST_STATE_PLAYING)); + + // Teardown + fail_unless_equals_int(GST_STATE_CHANGE_SUCCESS, gst_element_set_state(kinesisvideoproducersink, GST_STATE_NULL)); + gst_check_teardown_element(kinesisvideoproducersink); + } +GST_END_TEST; + +GST_START_TEST(check_no_pads_content_type_set_correctly) + { + GstElement *kinesisvideoproducersink; + + // Setup + kinesisvideoproducersink = gst_check_setup_element ("kvssink"); + fail_unless(kinesisvideoproducersink != nullptr, "Failed to create kvssink element"); + + g_object_set(G_OBJECT (kinesisvideoproducersink), + "stream-name", "test-stream", + NULL); + + // Test - Initialization of the client & stream occurs here + // Expecting kvssink->content_type == nullptr since no pads attached + fail_unless_equals_int(GST_STATE_CHANGE_FAILURE, gst_element_set_state(kinesisvideoproducersink, GST_STATE_PLAYING)); + + // Teardown + fail_unless_equals_int(GST_STATE_CHANGE_SUCCESS, gst_element_set_state(kinesisvideoproducersink, GST_STATE_NULL)); + gst_check_teardown_element(kinesisvideoproducersink); + } +GST_END_TEST; + GST_START_TEST(check_properties_are_passed_correctly) { GstElement *pElement = @@ -266,6 +315,8 @@ Suite *gst_kinesisvideoproducer_suite(void) { return s; } + tcase_add_test(tc, check_stream_name_null_handled); + tcase_add_test(tc, check_no_pads_content_type_set_correctly); tcase_add_test(tc, kvsproducersinktestplayandstop); tcase_add_test(tc, kvsproducersinktestplaytopausetoplay); tcase_add_test(tc, kvsproducersinkteststop);