Revise DPM policy checking codes 88/234388/6
authorSangchul Lee <sc11.lee@samsung.com>
Tue, 26 May 2020 04:16:18 +0000 (13:16 +0900)
committerSangchul Lee <sc11.lee@samsung.com>
Tue, 26 May 2020 08:20:40 +0000 (17:20 +0900)
Modified to keep going even if the DPM API fails.
Missing g_object_unref() is added.
Some logs are modified.

[Version] 0.1.43
[Issue Type] Improvement

Change-Id: I2d472262195fca4e7cc80883b5d40279cce0d244
Signed-off-by: Sangchul Lee <sc11.lee@samsung.com>
include/media_streamer_node_policy.h
packaging/capi-media-streamer.spec
src/media_streamer_node.c
src/media_streamer_node_policy.c

index 6ada20d..cf4e6ae 100644 (file)
@@ -31,7 +31,7 @@ typedef enum {
 
 int ms_node_policy_init(media_streamer_node_s *node);
 int ms_node_policy_deinit(media_streamer_node_s *node);
-int ms_node_policy_check(media_streamer_node_s *node);
+int ms_node_policy_check(media_streamer_node_s *node, gboolean *allowed);
 
 #ifdef __cplusplus
 }
index 6dc4809..8ee803a 100644 (file)
@@ -1,6 +1,6 @@
 Name:       capi-media-streamer
 Summary:    A Media Streamer API
-Version:    0.1.42
+Version:    0.1.43
 Release:    0
 Group:      Multimedia/API
 License:    Apache-2.0
index 674e2cd..a897daf 100644 (file)
@@ -955,7 +955,7 @@ gboolean ms_src_node_prepare_iter(const GValue *item, GValue *ret, gpointer user
        media_streamer_s *ms_streamer = (media_streamer_s *) user_data;
        GstElement *src_element = NULL;
        GstElement *found_element = NULL;
-       media_streamer_node_s *found_node = NULL;
+       media_streamer_node_s *node = NULL;
        GstPad *src_pad = NULL;
 
        ms_debug_fenter();
@@ -968,12 +968,14 @@ gboolean ms_src_node_prepare_iter(const GValue *item, GValue *ret, gpointer user
        g_object_ref(src_element);
        g_value_set_boolean(ret, FALSE);
 
-       found_node = (media_streamer_node_s *) g_hash_table_lookup(ms_streamer->nodes_table, GST_ELEMENT_NAME(src_element));
-       if (!found_node) {
+       node = (media_streamer_node_s *) g_hash_table_lookup(ms_streamer->nodes_table,
+               GST_ELEMENT_NAME(src_element));
+       if (!node) {
                /* If we fail to find corresponding node inside streamer
                        then apparently this element doesn't require resources. */
-               ms_debug("Failed to find corresponding node [%s] inside streamer", GST_ELEMENT_NAME(src_element));
+               ms_debug("Could not find corresponding node [%s] inside streamer, skip it", GST_ELEMENT_NAME(src_element));
                g_value_set_boolean(ret, TRUE);
+               g_object_unref(src_element);
                return TRUE;
        }
 
@@ -1700,8 +1702,8 @@ gboolean ms_node_resources_acquire_iter(const GValue *item, GValue *ret, gpointe
                GST_ELEMENT_NAME(element));
        if (!node) {
                /* If we fail to find corresponding node inside streamer
-                       then apparently this element doesn't require resources */
-               ms_info("Failed to find corresponding node [%s] inside streamer", GST_ELEMENT_NAME(element));
+                       then apparently this element doesn't require resources. */
+               ms_debug("Could not find corresponding node [%s] inside streamer, skip it", GST_ELEMENT_NAME(element));
                g_value_set_boolean(ret, TRUE);
                g_object_unref(element);
                return TRUE;
@@ -1744,7 +1746,7 @@ gboolean ms_node_resources_release_iter(const GValue *item, GValue *ret, gpointe
        if (!node) {
                /* If we fail to find corresponding node inside streamer
                        then apparently this element doesn't require resources. */
-               ms_debug("Failed to find corresponding node [%s] inside streamer", GST_ELEMENT_NAME(element));
+               ms_debug("Could not find corresponding node [%s] inside streamer, skip it", GST_ELEMENT_NAME(element));
                g_value_set_boolean(ret, TRUE);
                g_object_unref(element);
                return TRUE;
@@ -1770,6 +1772,7 @@ gboolean ms_node_policy_check_iter(const GValue *item, GValue *ret, gpointer use
        media_streamer_s *ms_streamer = (media_streamer_s *) user_data;
        media_streamer_node_s *node = NULL;
        GstElement *element = NULL;
+       gboolean allowed = FALSE;
 
        ms_debug_fenter();
 
@@ -1778,8 +1781,6 @@ gboolean ms_node_policy_check_iter(const GValue *item, GValue *ret, gpointer use
        ms_retvm_if(ms_streamer->nodes_table == NULL, FALSE, "ms_streamer->nodes_table is NULL");
        ms_retvm_if(ret == NULL, FALSE, "ret is NULL");
 
-       g_value_set_boolean(ret, FALSE);
-
        element = GST_ELEMENT(g_value_get_object(item));
        g_object_ref(element);
 
@@ -1787,24 +1788,26 @@ gboolean ms_node_policy_check_iter(const GValue *item, GValue *ret, gpointer use
                GST_ELEMENT_NAME(element));
        if (!node) {
                /* If we fail to find corresponding node inside streamer
-                       then apparently this element doesn't require resources */
-               ms_info("Failed to find corresponding node [%s] inside streamer", GST_ELEMENT_NAME(element));
+                       then apparently this element doesn't require resources. */
+               ms_debug("Could not find corresponding node [%s] inside streamer, skip it", GST_ELEMENT_NAME(element));
                g_value_set_boolean(ret, TRUE);
                g_object_unref(element);
                return TRUE;
        }
 
-       if (MEDIA_STREAMER_ERROR_NONE != ms_node_policy_check(node)) {
+       if (ms_node_policy_check(node, &allowed)) {
                ms_error("Failed to check policy for node [%s]", node->name);
+               /* Note that it should be TRUE(allowed) if the DPM API failed. */
+               g_value_set_boolean(ret, TRUE);
                g_object_unref(element);
                return FALSE;
        }
 
-       g_value_set_boolean(ret, TRUE);
+       g_value_set_boolean(ret, allowed);
 
        g_object_unref(element);
 
        ms_debug_fleave();
 
-       return TRUE;
+       return allowed;
 }
index 721e656..8bb8650 100644 (file)
@@ -42,7 +42,7 @@ static void __ms_node_get_dpm_check_needed(media_streamer_node_s *node, media_st
 
        subtype = node->subtype;
 
-       ms_debug("Checking policy for node type %d, subtype %d", node->type, subtype);
+       ms_debug("Checking DPM policy for node type %d, subtype %d", node->type, subtype);
 
        /* Check for node types */
        switch (subtype) {
@@ -54,6 +54,7 @@ static void __ms_node_get_dpm_check_needed(media_streamer_node_s *node, media_st
                *policy = POLICY_TYPE_MIC;
                break;
        default:
+               *policy = POLICY_TYPE_NONE;
                break;
        }
 
@@ -96,7 +97,7 @@ static void __ms_node_policy_changed_cb(const char *name, const char *value, voi
                interrupted_cb(MEDIA_STREAMER_INTERRUPTED_BY_SECURITY,
                        streamer->interrupted_cb.user_data);
        } else {
-               ms_info("Interuption will not be handled because interrupted_cb is NULL");
+               ms_info("Interruption will not be handled because interrupted_cb is NULL");
        }
        g_mutex_unlock(&streamer->mutex_lock);
 
@@ -162,22 +163,21 @@ int ms_node_policy_deinit(media_streamer_node_s *node)
 
        ms_retvm_if(node == NULL, MEDIA_STREAMER_ERROR_INVALID_PARAMETER, "node is NULL");
 
-       /* Check if node require policy manager */
        __ms_node_get_dpm_check_needed(node, &policy);
        if (POLICY_TYPE_NONE == policy) {
-               ms_info("No policy is needed for %p node type [%d] subtype [%d]",
+               ms_info("No need to check DPM restriction state for node [%p] type [%d] subtype [%d]",
                        node, node->type, node->subtype);
                return MEDIA_STREAMER_ERROR_NONE;
        }
 
-       if (node->dpm_handle) {
-               if (node->policy_changed_cb_id > 0)
-                       dpm_remove_policy_changed_cb(node->dpm_handle, node->policy_changed_cb_id);
-               else
-                       ms_info("invalid dpm cb id");
+       ms_retvm_if(node->dpm_handle == NULL, MEDIA_STREAMER_ERROR_INVALID_OPERATION, "DPM handle is NULL");
 
-               ms_debug("DPM released");
-       }
+       if (node->policy_changed_cb_id > 0)
+               dpm_remove_policy_changed_cb(node->dpm_handle, node->policy_changed_cb_id);
+       else
+               ms_info("invalid dpm cb id");
+
+       ms_debug("DPM released");
 
        dpm_manager_destroy(node->dpm_handle);
        node->dpm_handle = NULL;
@@ -187,29 +187,27 @@ int ms_node_policy_deinit(media_streamer_node_s *node)
        return ret;
 }
 
-int ms_node_policy_check(media_streamer_node_s *node)
+int ms_node_policy_check(media_streamer_node_s *node, gboolean *allowed)
 {
        media_streamer_policy_type_e policy = POLICY_TYPE_NONE;
-       int ret = MEDIA_STREAMER_ERROR_NONE;
        int dpm_state = DPM_ALLOWED;
        int dpm_ret = DPM_ERROR_NONE;
 
        ms_debug_fenter();
 
        ms_retvm_if(node == NULL, MEDIA_STREAMER_ERROR_INVALID_PARAMETER, "node is NULL");
+       ms_retvm_if(allowed == NULL, MEDIA_STREAMER_ERROR_INVALID_PARAMETER, "allowed is NULL");
+
+       *allowed = TRUE;
 
-       /* Check if node require policy manager */
        __ms_node_get_dpm_check_needed(node, &policy);
        if (POLICY_TYPE_NONE == policy) {
-               ms_info("No policy is needed for %p node type [%d] subtype [%d]",
+               ms_info("No need to check DPM restriction state for node [%p] type [%d] subtype [%d]",
                        node, node->type, node->subtype);
                return MEDIA_STREAMER_ERROR_NONE;
        }
 
-       if (node->dpm_handle == NULL) {
-               ms_error("DPM manager handle is NULL");
-               return MEDIA_STREAMER_ERROR_INVALID_OPERATION;
-       }
+       ms_retvm_if(node->dpm_handle == NULL, MEDIA_STREAMER_ERROR_INVALID_OPERATION, "DPM handle is NULL");
 
        switch (policy) {
        case POLICY_TYPE_CAMERA:
@@ -222,17 +220,18 @@ int ms_node_policy_check(media_streamer_node_s *node)
                break;
        }
 
-       if (dpm_ret == DPM_ERROR_NONE) {
-               ms_info("DPM state - %d", dpm_state);
-               if (dpm_state == DPM_DISALLOWED) {
-                       ms_error("Policy disallowed by DPM");
-                       return MEDIA_STREAMER_ERROR_INVALID_OPERATION;
-               }
-       } else {
-               ms_info("get DPM state failed, continue too work");
+       if (dpm_ret != DPM_ERROR_NONE) {
+               ms_error("Failed to get DPM restriction state");
+               return MEDIA_STREAMER_ERROR_INVALID_OPERATION;
+       }
+
+       ms_info("DPM state - %d", dpm_state);
+       if (dpm_state == DPM_DISALLOWED) {
+               ms_error("Policy disallowed by DPM");
+               *allowed = FALSE;
        }
 
        ms_debug_fleave();
 
-       return ret;
+       return MEDIA_STREAMER_ERROR_NONE;
 }