Add __is_valid_picture_index() and Apply color for dlog

to get or remove picuture, functions should check picture index.
similar picture index checking codes were duplicated for each funtions, so unified them into __is_valid_picture_index().

Change-Id: I06debb6c390aa85ab8d39e34b363d8829bee20e7

diff --git a/include/metadata_editor_private.h b/include/metadata_editor_private.h
index 8d7eb1d078046362cd4040a54db7a88754a2f469..fba0ffd01411cde41d014d31336ce7f83ff9c726 100755 (executable)
--- a/include/metadata_editor_private.h
+++ b/include/metadata_editor_private.h
@@ -29,40 +29,46 @@ extern "C" {
 #endif
 #define LOG_TAG "CAPI_MEDIA_METADATA_EDITOR"
 
+#define FONT_COLOR_RESET    "\033[0m"
+#define FONT_COLOR_RED      "\033[31m"
+#define FONT_COLOR_GREEN    "\033[32m"
+#define FONT_COLOR_YELLOW   "\033[33m"
+#define FONT_COLOR_CYAN     "\033[36m"
+
 #define metadata_editor_debug(fmt, arg...) do { \
-               LOGD("" fmt "", ##arg);     \
+               LOGD(FONT_COLOR_RESET"" fmt "" FONT_COLOR_RESET, ##arg);     \
        } while (0)
 
 #define metadata_editor_info(fmt, arg...) do { \
-               LOGI("" fmt "", ##arg);     \
+               LOGI(FONT_COLOR_GREEN"" fmt "" FONT_COLOR_RESET, ##arg);     \
        } while (0)
 
 #define metadata_editor_error(fmt, arg...) do { \
-               LOGE("" fmt "", ##arg);     \
+               LOGE(FONT_COLOR_RED"" fmt "" FONT_COLOR_RESET, ##arg);     \
        } while (0)
 
 #define metadata_editor_debug_fenter() do { \
-               LOGD("<Enter>");     \
+               LOGD(FONT_COLOR_YELLOW"<ENTER>" FONT_COLOR_RESET); \
        } while (0)
 
 #define metadata_editor_debug_fleave() do { \
-               LOGD("<Leave>");     \
+               LOGD(FONT_COLOR_YELLOW"<LEAVE>" FONT_COLOR_RESET); \
        } while (0)
 
 #define  metadata_editor_sec_debug(fmt, arg...) do { \
-               SECURE_LOGD("" fmt "", ##arg);     \
+               SECURE_LOGD(FONT_COLOR_CYAN"" fmt "" FONT_COLOR_RESET, ##arg);     \
        } while (0)
 
 #define metadata_editor_retvm_if(expr, val, fmt, arg...) do { \
                        if (expr) { \
-                               LOGE("" fmt "", ##arg); \
+                               LOGE(FONT_COLOR_RED"" fmt "" FONT_COLOR_RESET, ##arg);  \
                                return (val); \
                        } \
                } while (0)
 
 #define metadata_editor_retm_if(expr, fmt, arg...) do { \
                        if (expr) { \
-                               LOGE("" fmt "", ##arg); \
+                               LOGE(FONT_COLOR_RED"" fmt "" FONT_COLOR_RESET, ##arg); \
                                return; \
                        } \
                } while (0)

diff --git a/src/metadata_editor.cpp b/src/metadata_editor.cpp
index c7bda608d677783e4d61ac486e7fc29b9ed8d046..6dbe835acbcea3796e6236555e99707e09b5f732 100755 (executable)
--- a/src/metadata_editor.cpp
+++ b/src/metadata_editor.cpp
@@ -1142,25 +1142,33 @@ static char * __get_mime_type_from_cover_art(const MP4::CoverArt& cover_art)
        return NULL;
 }
 
-static int __get_APIC(ID3v2::Tag *tag, int index, void **picture, int *size, char **mime_type)
+static int __is_valid_picture_index(bool is_empty, unsigned int list_size, int index)
 {
-       metadata_editor_retvm_if(!tag, METADATA_EDITOR_ERROR_INVALID_PARAMETER, "Tag does not exist");
-
-       auto lst = tag->frameListMap()["APIC"];
+       metadata_editor_debug("list size [%d] picture index[%d]", list_size, index);
 
-       // Check if there are pictures in the tag
-       if (lst.isEmpty()) {
+       if (is_empty) {
                metadata_editor_error("No pictures in file");
                return METADATA_EDITOR_ERROR_INVALID_PARAMETER;
        }
 
-       // Check if index is correct or not
-       if ((index < 0) || (lst.size() <= (uint)index)) {
+       if ((index < 0) || (list_size <= (uint)index)) {
                metadata_editor_error("Index of picture is out of range");
                return METADATA_EDITOR_ERROR_INVALID_PARAMETER;
        }
 
-       metadata_editor_info("There are %u pictures in file. Start of picture number %d extraction", lst.size(), index);
+       return METADATA_EDITOR_ERROR_NONE;
+}
+
+static int __get_APIC(ID3v2::Tag *tag, int index, void **picture, int *size, char **mime_type)
+{
+       int ret = METADATA_EDITOR_ERROR_NONE;
+
+       metadata_editor_retvm_if(!tag, METADATA_EDITOR_ERROR_INVALID_PARAMETER, "Tag does not exist");
+
+       auto lst = tag->frameListMap()["APIC"];
+
+       ret = __is_valid_picture_index(lst.isEmpty(), lst.size(), index);
+       metadata_editor_retvm_if(ret != METADATA_EDITOR_ERROR_NONE, ret, "Invalid picture index");
 
        auto pictureFrame = static_cast<ID3v2::AttachedPictureFrame*>(lst[index]);
 
@@ -1184,6 +1192,8 @@ static int __get_mp3_picture(metadata_editor_s *metadata, int index, void **pict
 
 static int __get_mp4_picture(metadata_editor_s *metadata, int index, void **picture, int *size, char **mime_type)
 {
+       int ret = METADATA_EDITOR_ERROR_NONE;
+
        auto tag = dynamic_cast<MP4::Tag*>(metadata->file->tag());
        metadata_editor_retvm_if(!tag, METADATA_EDITOR_ERROR_INVALID_PARAMETER, "Tag does not exist");
 
@@ -1194,11 +1204,8 @@ static int __get_mp4_picture(metadata_editor_s *metadata, int index, void **pict
 
        auto lst = tag->item("covr").toCoverArtList();
 
-       // Check if the index is in range of CoverArtList Item
-       if ((index < 0) || ((uint)index >= lst.size())) {
-               metadata_editor_error("Index of picture is out of range");
-               return METADATA_EDITOR_ERROR_INVALID_PARAMETER;
-       }
+       ret = __is_valid_picture_index(lst.isEmpty(), lst.size(), index);
+       metadata_editor_retvm_if(ret != METADATA_EDITOR_ERROR_NONE, ret, "Invalid picture index");
 
        auto pictureFrame = static_cast<MP4::CoverArt>(lst[index]);
 
@@ -1215,21 +1222,15 @@ static int __get_mp4_picture(metadata_editor_s *metadata, int index, void **pict
 #if 0
 static int __get_flac_picture(metadata_editor_s *metadata, int index, void **picture, int *size, char **mime_type)
 {
+       int ret = METADATA_EDITOR_ERROR_NONE;
+
        auto _file = dynamic_cast<FLAC::File*>(metadata->file);
        metadata_editor_retvm_if(!_file, METADATA_EDITOR_ERROR_OPERATION_FAILED, "fail to dynamic_cast");
 
        auto lst = _file->pictureList();
 
-       if (lst.isEmpty()) {
-               metadata_editor_error("No pictures in FLAC file");
-               return METADATA_EDITOR_ERROR_INVALID_PARAMETER;
-       }
-
-       // Check if the index is in range of CoverArtList Item
-       if ((index < 0) || ((uint)index >= lst.size())) {
-               metadata_editor_error("Index of picture is out of range");
-               return METADATA_EDITOR_ERROR_INVALID_PARAMETER;
-       }
+       ret = __is_valid_picture_index(lst.isEmpty(), lst.size(), index);
+       metadata_editor_retvm_if(ret != METADATA_EDITOR_ERROR_NONE, ret, "Invalid picture index");
 
        auto pictureFrame = static_cast<FLAC::Picture*>(lst[index]);
 
@@ -1403,15 +1404,14 @@ extern "C" int metadata_editor_append_picture(metadata_editor_h metadata, const
 
 static int __remove_APIC(ID3v2::Tag *tag, int index)
 {
+       int ret = METADATA_EDITOR_ERROR_NONE;
+
        metadata_editor_retvm_if(!tag, METADATA_EDITOR_ERROR_INVALID_PARAMETER, "Tag does not exist");
 
        auto lst = tag->frameListMap()["APIC"];
-       metadata_editor_retvm_if(lst.isEmpty(), METADATA_EDITOR_ERROR_INVALID_PARAMETER, "No pictures in file");
 
-       if ((index < 0) || ((uint)index >= lst.size())) {
-               metadata_editor_error("Index of picture is out of range");
-               return METADATA_EDITOR_ERROR_INVALID_PARAMETER;
-       }
+       ret = __is_valid_picture_index(lst.isEmpty(), lst.size(), index);
+       metadata_editor_retvm_if(ret != METADATA_EDITOR_ERROR_NONE, ret, "Invalid picture index");
 
        tag->removeFrame(lst[index]);
 
@@ -1424,6 +1424,8 @@ static int __remove_mp3_picture(metadata_editor_s *metadata, int index)
 
 static int __remove_mp4_picture(metadata_editor_s *metadata, int index)
 {
+       int ret = METADATA_EDITOR_ERROR_NONE;
+
        auto tag = dynamic_cast<MP4::Tag*>(metadata->file->tag());
        metadata_editor_retvm_if(!tag, METADATA_EDITOR_ERROR_OPERATION_FAILED, "Tag does not exist");
 
@@ -1434,13 +1436,9 @@ static int __remove_mp4_picture(metadata_editor_s *metadata, int index)
 
        auto lst = tag->item("covr").toCoverArtList();
 
-       // Check if the index is in range of CoverArtList Item
-       if ((index < 0) || ((uint)index >= lst.size())) {
-               metadata_editor_error("Index of picture is out of range");
-               return METADATA_EDITOR_ERROR_INVALID_PARAMETER;
-       }
+       ret = __is_valid_picture_index(lst.isEmpty(), lst.size(), index);
+       metadata_editor_retvm_if(ret != METADATA_EDITOR_ERROR_NONE, ret, "Invalid picture index");
 
-       metadata_editor_info("The picture number %d will be deleted", index);
        auto picIt = lst.begin();
        std::advance(picIt, index);
        lst.erase(picIt);
@@ -1453,23 +1451,16 @@ static int __remove_mp4_picture(metadata_editor_s *metadata, int index)
 #if 0
 static int __remove_flac_picture(metadata_editor_s *metadata, int index)
 {
+       int ret = METADATA_EDITOR_ERROR_NONE;
+
        auto _file = dynamic_cast<FLAC::File*>(metadata->file);
        metadata_editor_retvm_if(!_file, METADATA_EDITOR_ERROR_OPERATION_FAILED, "fail to dynamic_cast");
 
        auto lst = _file->pictureList();
 
-       if (lst.isEmpty()) {
-               metadata_editor_error("No pictures in file. Nothing to delete");
-               return METADATA_EDITOR_ERROR_INVALID_PARAMETER;
-       }
-
-       // Check if the index is in range of CoverArtList Item
-       if ((index < 0) || ((uint)index >= lst.size())) {
-               metadata_editor_error("Index of picture is out of range");
-               return METADATA_EDITOR_ERROR_INVALID_PARAMETER;
-       }
+       ret = __is_valid_picture_index(lst.isEmpty(), lst.size(), index);
+       metadata_editor_retvm_if(ret != METADATA_EDITOR_ERROR_NONE, ret, "Invalid picture index");
 
-       metadata_editor_info("The picture number %d will be deleted", index);
        _file->removePicture(lst[index], true);
 
        return METADATA_EDITOR_ERROR_NONE;