revise media actions(changedCB)

diff --git a/media/MediaAction.h b/media/MediaAction.h
index e658eef..23c3fbd 100644 (file)
--- a/media/MediaAction.h
+++ b/media/MediaAction.h
@@ -27,9 +27,9 @@ public:
                : PluginAction(name)
        {
        }
-       virtual ~MediaAction() = default;
+       ~MediaAction() = default;
 
-       virtual int set(std::string val)
+       virtual int set(const std::string &val)
        {
                return MODES_ERROR_NOT_SUPPORTED;
        }

diff --git a/media/MediaFactory.cpp b/media/MediaFactory.cpp
index 3e99441..e37192b 100644 (file)
--- a/media/MediaFactory.cpp
+++ b/media/MediaFactory.cpp
@@ -14,6 +14,7 @@
  * limitations under the License.
  */
 #include "MediaFactory.h"
+
 #include "plugin-log.h"
 #include "MediaPlayer.h"
 #include "SoundVolume.h"
@@ -22,37 +23,37 @@ MODES_NAMESPACE_USE;
 
 MediaFactory::MediaFactory()
 {
-       actionMap[MediaPlayer::NAME] = PLAYER;
-       actionMap[SoundVolume::NAME[MEDIA_VOLUME]] = MEDIA_VOLUME;
        actionMap[SoundVolume::NAME[SYSTEM_VOLUME]] = SYSTEM_VOLUME;
-       actionMap[SoundVolume::NAME[RINGTONE_VOLUME]] = RINGTONE_VOLUME;
        actionMap[SoundVolume::NAME[NOTIFICATION_VOLUME]] = NOTIFICATION_VOLUME;
+       actionMap[SoundVolume::NAME[RINGTONE_VOLUME]] = RINGTONE_VOLUME;
+       actionMap[SoundVolume::NAME[MEDIA_VOLUME]] = MEDIA_VOLUME;
+       actionMap[MediaPlayer::NAME] = PLAYER;
 }
 
 MediaAction* MediaFactory::createAction(const std::string &key)
 {
        auto search = actionMap.find(key);
-       if (search == actionMap.end()) {
+       if (actionMap.end() == search) {
                ERR("No MediaAction(%s)", key.c_str());
                return nullptr;
        }
 
        MediaAction *action;
        switch (search->second) {
-       case PLAYER:
-               action = new MediaPlayer();
-               break;
-       case MEDIA_VOLUME:
-               action = new SoundVolume(MEDIA_VOLUME, SOUND_TYPE_MEDIA);
-               break;
        case SYSTEM_VOLUME:
                action = new SoundVolume(SYSTEM_VOLUME, SOUND_TYPE_SYSTEM);
                break;
+       case NOTIFICATION_VOLUME:
+               action = new SoundVolume(NOTIFICATION_VOLUME, SOUND_TYPE_NOTIFICATION);
+               break;
        case RINGTONE_VOLUME:
                action = new SoundVolume(RINGTONE_VOLUME, SOUND_TYPE_RINGTONE);
                break;
-       case NOTIFICATION_VOLUME:
-               action = new SoundVolume(NOTIFICATION_VOLUME, SOUND_TYPE_NOTIFICATION);
+       case MEDIA_VOLUME:
+               action = new SoundVolume(MEDIA_VOLUME, SOUND_TYPE_MEDIA);
+               break;
+       case PLAYER:
+               action = new MediaPlayer();
                break;
        default:
                action = nullptr;

diff --git a/media/MediaFactory.h b/media/MediaFactory.h
index 7358a2b..1ef5e4c 100644 (file)
--- a/media/MediaFactory.h
+++ b/media/MediaFactory.h
@@ -30,10 +30,10 @@ public:
        void destroyAction(MediaAction *action);
 private:
        enum actionKey{
-               MEDIA_VOLUME,
                SYSTEM_VOLUME,
-               RINGTONE_VOLUME,
                NOTIFICATION_VOLUME,
+               RINGTONE_VOLUME,
+               MEDIA_VOLUME,
                PLAYER
        };
 

diff --git a/media/MediaPlayer.cpp b/media/MediaPlayer.cpp
index 94f3053..112e213 100644 (file)
--- a/media/MediaPlayer.cpp
+++ b/media/MediaPlayer.cpp
@@ -14,50 +14,41 @@
  * limitations under the License.
  */
 #include "MediaPlayer.h"
+
 #include <string>
-#include <modes_errors.h>
 #include <player.h>
+#include <modes_errors.h>
 #include "plugin-log.h"
 
 MODES_NAMESPACE_USE;
 
 const std::string MediaPlayer::NAME = "player";
-player_h MediaPlayer::player = NULL;
-sound_stream_info_h MediaPlayer::stream_info = NULL;
 
 MediaPlayer::MediaPlayer()
-       : MediaAction(NAME)
-{
-}
-
-int MediaPlayer::createPlayer()
+       : MediaAction(NAME), player(NULL), streamInfo(NULL), cb(nullptr), cbData(nullptr)
 {
-       int soundRet = sound_manager_create_stream_information(SOUND_STREAM_TYPE_MEDIA, NULL, NULL, &stream_info);
-       if (SOUND_MANAGER_ERROR_NONE != soundRet) {
-               ERR("sound_manager_create_stream_information() Fail(%s)", get_error_message(soundRet));
-               return MODES_ERROR_SYSTEM;
+       int ret = player_create(&player);
+       if (PLAYER_ERROR_NONE != ret) {
+               ERR("player_create() Fail(%s)", get_error_message(ret));
+               return;
        }
 
-       int playerRet = player_create(&player);
-       if (PLAYER_ERROR_NONE != playerRet) {
-               ERR("player_create() Fail(%s)", get_error_message(playerRet));
-               return MODES_ERROR_SYSTEM;
+       ret = sound_manager_create_stream_information(SOUND_STREAM_TYPE_MEDIA, NULL, NULL, &streamInfo);
+       if (SOUND_MANAGER_ERROR_NONE != ret) {
+               ERR("sound_manager_create_stream_information() Fail(%s)", get_error_message(ret));
+               return;
        }
 
-       playerRet = player_set_sound_stream_info(player, stream_info);
-       if (PLAYER_ERROR_NONE != playerRet) {
-               ERR("player_set_sound_stream_info() Fail(%s)", get_error_message(playerRet));
-               return MODES_ERROR_SYSTEM;
+       ret = player_set_sound_stream_info(player, streamInfo);
+       if (PLAYER_ERROR_NONE != ret) {
+               ERR("player_set_sound_stream_info() Fail(%s)", get_error_message(ret));
+               return;
        }
-       return MODES_ERROR_NONE;
 }
 
-void MediaPlayer::destroyPlayer(void *data)
+MediaPlayer::~MediaPlayer()
 {
-       if (stream_info) {
-               sound_manager_destroy_stream_information(stream_info);
-               stream_info = NULL;
-       }
+       sound_manager_destroy_stream_information(streamInfo);
 
        if (player) {
                player_state_e state;
@@ -66,18 +57,11 @@ void MediaPlayer::destroyPlayer(void *data)
                        player_stop(player);
                player_unprepare(player);
                player_destroy(player);
-               player = NULL;
        }
 }
 
-int MediaPlayer::set(std::string val)
+int MediaPlayer::set(const std::string &val)
 {
-       int modesRet = createPlayer();
-       if (MODES_ERROR_SYSTEM == modesRet) {
-               ERR("createPlayer() Fail(%d)", modesRet);
-               return MODES_ERROR_SYSTEM;
-       }
-
        int ret = player_set_uri(player, val.c_str());
        if (PLAYER_ERROR_NONE != ret) {
                ERR("player_set_uri() Fail(%s)", get_error_message(ret));
@@ -90,22 +74,66 @@ int MediaPlayer::set(std::string val)
                return MODES_ERROR_SYSTEM;
        }
 
-       ret = player_set_completed_cb(player, destroyPlayer, NULL);
+       ret = player_start(player);
        if (PLAYER_ERROR_NONE != ret) {
-               ERR("player_set_completed_cb() Fail(%s)", get_error_message(ret));
+               ERR("player_start() Fail(%s)", get_error_message(ret));
                return MODES_ERROR_SYSTEM;
        }
 
-       ret = player_start(player);
+       return MODES_ERROR_NONE;
+}
+
+void MediaPlayer::undo()
+{
+       player_stop(player);
+}
+
+std::string MediaPlayer::serialize()
+{
+       //Not Support
+       return std::string();
+}
+
+int MediaPlayer::parse(const std::string &archive)
+{
+       WARN("MediaPlayer is volatile");
+       return MODES_ERROR_NOT_SUPPORTED;
+}
+
+int MediaPlayer::setChangedCallback(valueChangedCB callback, void *userData)
+{
+       RETV_IF(nullptr == callback, MODES_ERROR_INVALID_PARAMETER);
+
+       int ret = player_set_completed_cb(player, completeCB, this);
        if (PLAYER_ERROR_NONE != ret) {
-               ERR("player_start() Fail(%s)", get_error_message(ret));
+               ERR("player_set_completed_cb() Fail(%s)", get_error_message(ret));
                return MODES_ERROR_SYSTEM;
        }
 
+       cb = callback;
+       cbData = userData;
+
        return MODES_ERROR_NONE;
 }
 
-void MediaPlayer::undo()
+void MediaPlayer::unSetChangedCallback(valueChangedCB callback, void *userData)
 {
-       destroyPlayer(NULL);
+       RET_IF(nullptr == callback);
+
+       int ret = player_unset_completed_cb(player);
+       if (PLAYER_ERROR_NONE != ret)
+               ERR("player_unset_completed_cb() Fail(%s)", get_error_message(ret));
+
+       cb = nullptr;
+       cbData = nullptr;
+}
+
+void MediaPlayer::completeCB(void *data)
+{
+       MediaPlayer *action = (MediaPlayer*)data;
+
+       RET_IF(nullptr == data);
+
+       if (action->cb)
+               action->cb(action->cbData);
 }

diff --git a/media/MediaPlayer.h b/media/MediaPlayer.h
index f0feeac..2385a01 100644 (file)
--- a/media/MediaPlayer.h
+++ b/media/MediaPlayer.h
@@ -24,18 +24,24 @@ MODES_NAMESPACE_BEGIN
 
 class MediaPlayer : public MediaAction {
 public:
-       static const std::string NAME;
        MediaPlayer();
-       ~MediaPlayer() = default;
+       ~MediaPlayer();
 
-       int set(std::string val) override;
+       int set(const std::string &val) override;
        void undo() override;
+       std::string serialize() override;
+       int parse(const std::string &archive) override;
+       int setChangedCallback(valueChangedCB callback, void *userData) override;
+       void unSetChangedCallback(valueChangedCB callback, void *userData) override;
+
+       static const std::string NAME;
 private:
-       int createPlayer();
-       static void destroyPlayer(void *data);
+       static void completeCB(void *data);
 
-       static player_h player;
-       static sound_stream_info_h stream_info;
+       player_h player;
+       sound_stream_info_h streamInfo;
+       valueChangedCB cb;
+       void *cbData;
 };
 
 MODES_NAMESPACE_END

diff --git a/media/MediaPlugin.cpp b/media/MediaPlugin.cpp
index 8bef8a1..0cfe1ce 100644 (file)
--- a/media/MediaPlugin.cpp
+++ b/media/MediaPlugin.cpp
@@ -67,7 +67,6 @@ int MediaPlugin::set(const std::string &key, const std::string &val, PluginActio
                mediaFactory.destroyAction(action);
                return ret;
        }
-       DBG("Action(%s) set(%s)", key.c_str(), val.c_str());
 
        if (piAction)
                *piAction = action;
@@ -87,7 +86,6 @@ int MediaPlugin::set(const std::string &key, int val, PluginAction **piAction)
                mediaFactory.destroyAction(action);
                return ret;
        }
-       DBG("Action(%s) set(%d)", key.c_str(), val);
 
        if (piAction)
                *piAction = action;
@@ -116,8 +114,6 @@ void MediaPlugin::undo(PluginAction *piAction)
        MediaAction *action = static_cast<MediaAction*>(piAction);
        RET_IF(nullptr == piAction);
 
-       DBG("Action(%s) undo", action->getName().c_str());
-
        action->undo();
        mediaFactory.destroyAction(action);
 }

diff --git a/media/SoundVolume.cpp b/media/SoundVolume.cpp
index 05a788b..03aa22f 100644 (file)
--- a/media/SoundVolume.cpp
+++ b/media/SoundVolume.cpp
@@ -14,6 +14,7 @@
  * limitations under the License.
  */
 #include "SoundVolume.h"
+
 #include <string>
 #include <sstream>
 #include <modes_errors.h>
@@ -24,29 +25,27 @@ MODES_NAMESPACE_USE;
 
 // It should align with actionKey in MediaFactory
 const std::string SoundVolume::NAME[4] = {
-       "mediaVolume",
        "systemVolume",
-       "ringtoneVolume",
        "notificationVolume"
+       "ringtoneVolume",
+       "mediaVolume",
 };
 
 SoundVolume::SoundVolume(int key, sound_type_e soundType)
-       : MediaAction(NAME[key]), oldVal(-1), volumeCallbackID(0), cb(NULL), cbData(NULL), requestVal(-1), type(soundType)
+       : MediaAction(NAME[key]), type(soundType),
+       requestVal(-1), oldVal(-1), callbackID(0), cb(nullptr), cbData(nullptr)
 {
 }
 
-void SoundVolume::volumeChangedCB(sound_type_e type, unsigned int volume, void *user_data)
-{
-       SoundVolume *action = (SoundVolume*)user_data;
-       RET_IF(NULL == user_data);
-
-       if (action->requestVal != (int)volume)
-               action->cb(action->cbData);
-}
-
 int SoundVolume::set(int val)
 {
-       int ret;
+       requestVal = val;
+
+       int ret = sound_manager_get_volume(type, &oldVal);
+       if (SOUND_MANAGER_ERROR_NONE != ret) {
+               ERR("sound_manager_get_volume() Fail(%s)", get_error_message(ret));
+               return MODES_ERROR_SYSTEM;
+       }
 
        if (-1 == val) {
                ret = sound_manager_get_max_volume(type, &val);
@@ -54,25 +53,18 @@ int SoundVolume::set(int val)
                        ERR("sound_manager_get_max_volume() Fail(%s)", get_error_message(ret));
                        return MODES_ERROR_SYSTEM;
                }
-               DBG("sound_manager_get_max_volume() type(%d), return(%d)", type, val);
-       }
-
-       requestVal = val;
-       ret = sound_manager_get_volume(type, &oldVal);
-       if (SOUND_MANAGER_ERROR_NONE != ret) {
-               ERR("sound_manager_get_volume() Fail(%s)", get_error_message(ret));
-               return MODES_ERROR_SYSTEM;
+               DBG("sound_manager_get_max_volume(%s) is (%d)", getName().c_str(), val);
        }
 
-       if (val == oldVal) {
-               INFO("Already media volume is [%d]", val);
-               return MODES_ERROR_NONE;
-       }
 
-       ret = sound_manager_set_volume(type, val);
-       if (SOUND_MANAGER_ERROR_NONE != ret) {
-               ERR("sound_manager_set_volume() Fail(%s)", get_error_message(ret));
-               return MODES_ERROR_SYSTEM;
+       if (oldVal == val) {
+               INFO("%s already set[%d]", getName().c_str(), val);
+       } else {
+               ret = sound_manager_set_volume(type, val);
+               if (SOUND_MANAGER_ERROR_NONE != ret) {
+                       ERR("sound_manager_set_volume(%s) Fail(%s)", getName().c_str(), get_error_message(ret));
+                       return MODES_ERROR_SYSTEM;
+               }
        }
 
        return MODES_ERROR_NONE;
@@ -81,9 +73,8 @@ int SoundVolume::set(int val)
 void SoundVolume::undo()
 {
        int ret = sound_manager_set_volume(type, oldVal);
-       if (SOUND_MANAGER_ERROR_NONE != ret) {
+       if (SOUND_MANAGER_ERROR_NONE != ret)
                ERR("sound_manager_set_volume() Fail(%s)", get_error_message(ret));
-       }
 }
 
 std::string SoundVolume::serialize()
@@ -104,22 +95,39 @@ int SoundVolume::parse(const std::string &archive)
 
 int SoundVolume::setChangedCallback(valueChangedCB callback, void *userData)
 {
-       int ret = sound_manager_add_volume_changed_cb(volumeChangedCB, this, &volumeCallbackID);
+       RETV_IF(nullptr == callback, MODES_ERROR_INVALID_PARAMETER);
+
+       int ret = sound_manager_add_volume_changed_cb(volumeChangedCB, this, &callbackID);
        if (SOUND_MANAGER_ERROR_NONE != ret) {
                ERR("sound_manager_add_volume_changed_cb() Fail(%s)", get_error_message(ret));
                return MODES_ERROR_SYSTEM;
        }
+
        cb = callback;
        cbData = userData;
+
        return MODES_ERROR_NONE;
 }
 
 void SoundVolume::unSetChangedCallback(valueChangedCB callback, void *userData)
 {
-       int ret = sound_manager_remove_volume_changed_cb(volumeCallbackID);
-       if (SOUND_MANAGER_ERROR_NONE != ret) {
+       RET_IF(nullptr == callback);
+
+       int ret = sound_manager_remove_volume_changed_cb(callbackID);
+       if (SOUND_MANAGER_ERROR_NONE != ret)
                ERR("sound_manager_add_volume_changed_cb() Fail(%s)", get_error_message(ret));
-       }
-       cb = NULL;
-       cbData = NULL;
+
+       cb = nullptr;
+       cbData = nullptr;
+}
+
+void SoundVolume::volumeChangedCB(sound_type_e type, unsigned int volume, void *userData)
+{
+       SoundVolume *action = (SoundVolume*)userData;
+
+       RET_IF(nullptr == userData);
+       RET_IF(type != action->type);
+
+       if (action->cb && action->requestVal != (int)volume)
+               action->cb(action->cbData);
 }

diff --git a/media/SoundVolume.h b/media/SoundVolume.h
index 35a72a0..9f04e74 100644 (file)
--- a/media/SoundVolume.h
+++ b/media/SoundVolume.h
@@ -24,7 +24,6 @@ MODES_NAMESPACE_BEGIN
 
 class SoundVolume : public MediaAction {
 public:
-       static const std::string NAME[4];
        SoundVolume(int key, sound_type_e soundType);
        ~SoundVolume() = default;
 
@@ -35,14 +34,16 @@ public:
        int setChangedCallback(valueChangedCB callback, void *userData) override;
        void unSetChangedCallback(valueChangedCB callback, void *userData) override;
 
+       static const std::string NAME[4];
 private:
-       static void volumeChangedCB(sound_type_e type, unsigned int volume, void *user_data);
+       static void volumeChangedCB(sound_type_e type, unsigned int volume, void *userData);
+
+       sound_type_e type; //not same with the NAME[] order
+       int requestVal;
        int oldVal;
-       int volumeCallbackID;
+       int callbackID;
        valueChangedCB cb;
        void *cbData;
-       int requestVal;
-       sound_type_e type;
 };
 
 MODES_NAMESPACE_END

diff --git a/unittests/mdsp_test_media.cpp b/unittests/mdsp_test_media.cpp
index 95c92f1..ebc53b4 100644 (file)
--- a/unittests/mdsp_test_media.cpp
+++ b/unittests/mdsp_test_media.cpp
@@ -38,7 +38,7 @@ protected:
                g_main_loop_unref(loop);
                loop = NULL;
                objectDelete(plugin);
-               plugin = NULL;
+               plugin = nullptr;
        }
 
        static gboolean mediaPlaySetUndoTimeout(gpointer data)
@@ -112,7 +112,7 @@ protected:
 };
 
 int MediaPluginTest::result = 0;
-Plugin *MediaPluginTest::plugin = NULL;
+Plugin *MediaPluginTest::plugin = nullptr;
 GMainLoop *MediaPluginTest::loop = NULL;
 
 TEST_F(MediaPluginTest, setUndoMediaPlay)