From 3fe16b410dc418796e22c404f9b8d173e0154032 Mon Sep 17 00:00:00 2001 From: Stefan Kieszkowski Date: Wed, 29 Nov 2023 22:29:05 +0000 Subject: [PATCH 1/5] Change parent class stat change position --- src/gstreamer/gstkvssink.cpp | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/gstreamer/gstkvssink.cpp b/src/gstreamer/gstkvssink.cpp index 7f74748f..5a8b9a4f 100644 --- a/src/gstreamer/gstkvssink.cpp +++ b/src/gstreamer/gstkvssink.cpp @@ -1609,6 +1609,13 @@ gst_kvs_sink_change_state(GstElement *element, GstStateChange transition) { case GST_STATE_CHANGE_READY_TO_PAUSED: gst_collect_pads_start (kvssink->collect); break; + default: + break; + } + + ret = GST_ELEMENT_CLASS (parent_class)->change_state(element, transition); + + switch (transition) { case GST_STATE_CHANGE_PAUSED_TO_READY: LOG_INFO("Stopping kvssink for " << kvssink->stream_name); gst_collect_pads_stop (kvssink->collect); @@ -1631,8 +1638,6 @@ gst_kvs_sink_change_state(GstElement *element, GstStateChange transition) { break; } - ret = GST_ELEMENT_CLASS (parent_class)->change_state(element, transition); - CleanUp: if (ret != GST_STATE_CHANGE_SUCCESS) { From abea82f9e78f71604bcb959f6a0cd1bdd36228cb Mon Sep 17 00:00:00 2001 From: Stefan Kieszkowski <85728496+stefankiesz@users.noreply.github.com> Date: Wed, 29 Nov 2023 14:55:20 -0800 Subject: [PATCH 2/5] Add failure check Co-authored-by: Niyati Maheshwari --- src/gstreamer/gstkvssink.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/gstreamer/gstkvssink.cpp b/src/gstreamer/gstkvssink.cpp index 5a8b9a4f..c58ccb77 100644 --- a/src/gstreamer/gstkvssink.cpp +++ b/src/gstreamer/gstkvssink.cpp @@ -1614,6 +1614,9 @@ gst_kvs_sink_change_state(GstElement *element, GstStateChange transition) { } ret = GST_ELEMENT_CLASS (parent_class)->change_state(element, transition); + if (ret == GST_STATE_CHANGE_FAILURE) { + goto CleanUp; + } switch (transition) { case GST_STATE_CHANGE_PAUSED_TO_READY: From 49776feb4f76b8ae18c3e26d758fbebe6f0516af Mon Sep 17 00:00:00 2001 From: Stefan Kieszkowski <85728496+stefankiesz@users.noreply.github.com> Date: Thu, 30 Nov 2023 11:32:23 -0800 Subject: [PATCH 3/5] Testing on mac os 13 --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index cfe8d7b8..aa93292f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -11,7 +11,7 @@ on: jobs: mac-os-build-clang: - runs-on: macos-12 + runs-on: macos-13 env: AWS_KVS_LOG_LEVEL: 2 CC: /usr/bin/clang From a87b2831e6177d5ba250310fa70a406d21977a4b Mon Sep 17 00:00:00 2001 From: Stefan Kieszkowski <85728496+stefankiesz@users.noreply.github.com> Date: Tue, 5 Dec 2023 15:08:48 -0800 Subject: [PATCH 4/5] Revert ci to mac os 12 --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index aa93292f..cfe8d7b8 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -11,7 +11,7 @@ on: jobs: mac-os-build-clang: - runs-on: macos-13 + runs-on: macos-12 env: AWS_KVS_LOG_LEVEL: 2 CC: /usr/bin/clang From 745db125c4422652907f8ddcf33069ea61429671 Mon Sep 17 00:00:00 2001 From: Stefan Kieszkowski <85728496+stefankiesz@users.noreply.github.com> Date: Wed, 6 Dec 2023 12:04:24 -0800 Subject: [PATCH 5/5] Add comments explaining state transition guidelines --- src/gstreamer/gstkvssink.cpp | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/src/gstreamer/gstkvssink.cpp b/src/gstreamer/gstkvssink.cpp index c58ccb77..c1d3b77a 100644 --- a/src/gstreamer/gstkvssink.cpp +++ b/src/gstreamer/gstkvssink.cpp @@ -1572,12 +1572,25 @@ init_track_data(GstKvsSink *kvssink) { static GstStateChangeReturn gst_kvs_sink_change_state(GstElement *element, GstStateChange transition) { + /* + The below state transition cases are separated into two switch blocks: + one for upward (NULL->READY->PAUSED->PLAYING) transitions and one for + downward (PLAYING->PAUSED->READY->NULL) transitions. It is necessary to transition + an element's parent class state after any of the element's upward transitions but + before any downward transitions. As per GStreamer documentation, + "this is necessary in order to safely handle concurrent access by multiple threads." + + https://gstreamer.freedesktop.org/documentation/plugin-development/basics/states. + html?gi-language=c#:~:text=Note%20that%20upwards,destroying%20allocated%20resources. + */ + GstStateChangeReturn ret = GST_STATE_CHANGE_SUCCESS; GstKvsSink *kvssink = GST_KVS_SINK (element); auto data = kvssink->data; string err_msg = ""; ostringstream oss; + // Upward transitions switch (transition) { case GST_STATE_CHANGE_NULL_TO_READY: if(kvssink->log_config_path != NULL) { @@ -1613,11 +1626,13 @@ gst_kvs_sink_change_state(GstElement *element, GstStateChange transition) { break; } + // Parent class transition ret = GST_ELEMENT_CLASS (parent_class)->change_state(element, transition); if (ret == GST_STATE_CHANGE_FAILURE) { goto CleanUp; } + // Downward transitions switch (transition) { case GST_STATE_CHANGE_PAUSED_TO_READY: LOG_INFO("Stopping kvssink for " << kvssink->stream_name); @@ -1673,4 +1688,4 @@ GST_PLUGIN_DEFINE ( "Proprietary", "GStreamer", "http://gstreamer.net/" -) \ No newline at end of file +)