Skip to content

Commit

Permalink
Add runtime string checks and additional unit tests
Browse files Browse the repository at this point in the history
  • Loading branch information
@sirknightj
sirknightj committed Jan 27, 2025
1 parent d88e505 commit ecb5342
Show file tree
Hide file tree
Showing 3 changed files with 89 additions and 6 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/kvssink.yml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,4 +65,4 @@ jobs:
working-directory: ./build
run: |
export GST_PLUGIN_PATH=`pwd`
./tst/gstkvsplugintest
GST_DEBUG=4 ./tst/gstkvsplugintest
38 changes: 35 additions & 3 deletions src/gstreamer/gstkvssink.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<DeviceInfoProvider> device_info_provider(new KvsSinkDeviceInfoProvider(kvssink->storage_size,
kvssink->stop_stream_timeout,
kvssink->service_connection_timeout,
Expand Down Expand Up @@ -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))) {
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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<StreamDefinition> stream_definition(new StreamDefinition(kvssink->stream_name,
hours(kvssink->retention_period_hours),
p_stream_tags,
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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
Expand Down
55 changes: 53 additions & 2 deletions tst/gstreamer/gstkvstest.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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?)");

Expand All @@ -30,23 +31,28 @@ 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);
gst_object_unref(sinkpad);
}

gst_check_teardown_element (kinesisvideoproducersink);

cout << "cleanup_kinesisvideoproducersink() end" << endl;
}

GST_START_TEST(kvsproducersinktestplayandstop)
Expand Down Expand Up @@ -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 =
Expand Down Expand Up @@ -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);
Expand Down

0 comments on commit ecb5342

Please sign in to comment.