ID3v2 tag parser refactoring 76/279476/5
authorminje.ahn <minje.ahn@samsung.com>
Wed, 10 Aug 2022 05:02:59 +0000 (14:02 +0900)
committerminje.ahn <minje.ahn@samsung.com>
Thu, 11 Aug 2022 06:05:58 +0000 (15:05 +0900)
ID3v2.3, v2.4 tag parser refactoring to reduce code complexity.

Change-Id: I185c715cd45af9c6b725f2543595195a84c9ecde
Signed-off-by: minje.ahn <minje.ahn@samsung.com>
packaging/libmm-fileinfo.spec
utils/mm_file_util_tag.c

index 6ce2c5a..91b1745 100644 (file)
@@ -1,6 +1,6 @@
 Name:      libmm-fileinfo
 Summary:    Media Fileinfo
-Version:    1.0.13
+Version:    1.0.14
 Release:    0
 Group:      System/Libraries
 License:    Apache-2.0
index e6bb27b..499561a 100644 (file)
@@ -2852,7 +2852,7 @@ static AvID3TagList __get_tag_info_v222(const unsigned char *tag)
        return AV_ID3TAG_MAX;
 }
 
-static AvID3TagList __get_tag_info_v223(const char *tag)
+static AvID3TagList __get_tag_info_v223(const unsigned char *tag)
 {
        unsigned int n = 0;
        int i = 0;
@@ -2897,7 +2897,7 @@ bool mm_file_id3tag_parse_v222(AvFileContentInfo *pInfo, unsigned char *buffer,
                        break;
 
                tag_id = __get_tag_info_v222(&buffer[curPos]);
-               frameLen = buffer[3 + curPos] << 16 | buffer[4 + curPos] << 8 | buffer[5 + curPos];
+               frameLen = buffer[curPos + 3] << 16 | buffer[curPos + 4] << 8 | buffer[curPos + 5];
                curPos += MP3_TAGv2_22_TXT_HEADER_LEN;
                if (curPos + frameLen > pInfo->tagV2Info.tagLen)
                        break;
@@ -2946,6 +2946,7 @@ bool mm_file_id3tag_parse_v222(AvFileContentInfo *pInfo, unsigned char *buffer,
 
                        if (buffer[curPos + 4] > 0x20 && (buffer[curPos + 3] == 0x00 || buffer[curPos + 3] == 0x01)) {
                                encType = __id3tag_get_text_encoding_v222(&buffer[curPos], 4);
+                               /*skip language data! */
                                curPos += 4;
                                frameLen -= 4;
                                pInfo->tagInfo[tag_id].value = mmfile_convert_to_utf8((const char *)&buffer[curPos], frameLen, charset_array[encType]);
@@ -2979,41 +2980,40 @@ NEXT:
 }
 
 static void __get_v223_encoding_info(const unsigned char *buffer,
-                                                                       unsigned long position,
-                                                                       unsigned long length,
-                                                                       unsigned int *offset,
+                                                                       int length,
+                                                                       int *offset,
                                                                        unsigned int *type)
 {
-       unsigned int _offset = 0;
+       int _offset = 0;
 
-       if (!buffer || !offset || !type || (position < length))
+       if (!buffer || !offset || !type)
                return;
 
-       if (IS_ENCODEDBY_UTF16(buffer + (position - length))) {
+       if (IS_ENCODEDBY_UTF16(buffer)) {
                *offset = 2;
                *type = AV_ID3V2_UTF16;
                return;
        }
-       if (IS_ENCODEDBY_UTF16_R(buffer + (position - length))) {
+       if (IS_ENCODEDBY_UTF16_R(buffer)) {
                *offset = 2;
                *type = AV_ID3V2_UTF16_BE;
                return;
        }
-       if (IS_ENCODEDBY_UTF16(buffer + (position - length + 1))) {
+       if (IS_ENCODEDBY_UTF16(buffer + 1)) {
                *offset = 3;
                *type = AV_ID3V2_UTF16;
                return;
        }
-       if (IS_ENCODEDBY_UTF16_R(buffer + (position - length + 1))) {
+       if (IS_ENCODEDBY_UTF16_R(buffer + 1)) {
                *offset = 3;
                *type = AV_ID3V2_UTF16_BE;
                return;
        }
 
-       if (buffer[position - length] == 0x00) {
+       if (buffer[0] == 0x00) {
                *offset = 1;
        } else {
-               while ((buffer[position - length + _offset] < 0x20) && (_offset < length))
+               while ((buffer[_offset] < 0x20) && (_offset < length))
                        _offset++;
                *offset = _offset;
        }
@@ -3022,38 +3022,27 @@ static void __get_v223_encoding_info(const unsigned char *buffer,
 
 bool mm_file_id3tag_parse_v223(AvFileContentInfo *pInfo, unsigned char *buffer, bool extract_artwork)
 {
-       unsigned long taglen = 0;
-       unsigned long needToloopv2taglen;
-       unsigned long oneFrameLen = 0;
-       unsigned long curPos = 0;
-       char CompTmp[5];
-       unsigned char *pExtContent = NULL;
-       unsigned long purelyFramelen = 0;
-       unsigned int encodingOffSet = 0;
-       int realCpyFrameNum = 0,  tmp = 0;
-       unsigned int textEncodingType = 0;
+       int extendedHeaderLen = 0;
+       int frameLen = 0;
+       int curPos = MP3_TAGv2_HEADER_LEN;
+       int encOffset = 0;
+       unsigned int encType = 0;
        char **charset_array = NULL;
        AvID3TagList tag_id = AV_ID3TAG_MAX;
        char *lang_info = NULL;
 
-       make_characterset_array(&charset_array);
+       if (pInfo->tagV2Info.tagLen <= 0)
+               return false;
 
+       make_characterset_array(&charset_array);
        init_content_info(pInfo);
 
-       taglen = pInfo->tagV2Info.tagLen;
-       needToloopv2taglen = taglen - MP3_TAGv2_HEADER_LEN;
-       curPos = MP3_TAGv2_HEADER_LEN;
-
        debug_msg(RELEASE, "ID3tag v223--------------------------------------------------------------");
-
-       /* check Extended Header */
        if (buffer[5] & 0x40) {
                /* if extended header exists, skip it*/
-               int extendedHeaderLen = (unsigned long)buffer[10] << 21 | (unsigned long)buffer[11] << 14 | (unsigned long)buffer[12] << 7  | (unsigned long)buffer[13];
-
+               extendedHeaderLen = buffer[10] << 21 | buffer[11] << 14 | buffer[12] << 7 | buffer[13];
                debug_msg(RELEASE, "--------------- extendedHeaderLen = %d", extendedHeaderLen);
-
-               if (extendedHeaderLen > (int)(taglen - curPos)) {
+               if (extendedHeaderLen > pInfo->tagV2Info.tagLen - MP3_TAGv2_HEADER_LEN) {
                        debug_error(DEBUG, "extended header too long.");
                } else {
                        curPos += extendedHeaderLen;
@@ -3061,137 +3050,116 @@ bool mm_file_id3tag_parse_v223(AvFileContentInfo *pInfo, unsigned char *buffer,
                }
        }
 
-       while (needToloopv2taglen > MP3_TAGv2_23_TXT_HEADER_LEN) {
+       while (curPos + MP3_TAGv2_23_TXT_HEADER_LEN < pInfo->tagV2Info.tagLen) {
                if (!g_ascii_isalnum(buffer[curPos]) || !g_ascii_isalnum(buffer[curPos + 1]) ||
                        !g_ascii_isalnum(buffer[curPos + 2]) || !g_ascii_isalnum(buffer[curPos + 3]))
                        break;
 
-               memcpy(CompTmp, &buffer[curPos], 4);
-
-               CompTmp[4] = 0;
-               oneFrameLen = MP3_TAGv2_23_TXT_HEADER_LEN;
-               oneFrameLen += (unsigned long)buffer[4 + curPos] << 24 | (unsigned long)buffer[5 + curPos] << 16
-                                       | (unsigned long)buffer[6 + curPos] << 8 | (unsigned long)buffer[7 + curPos];
-
-               debug_msg(RELEASE, "----------------------------------------------------------------------------------------------------");
-
-               if (oneFrameLen > taglen - curPos)
+               tag_id = __get_tag_info_v223(&buffer[curPos]);
+               frameLen = buffer[curPos + 4] << 24 | buffer[curPos + 5] << 16 | buffer[curPos + 6] << 8 | buffer[curPos + 7];
+               curPos += MP3_TAGv2_23_TXT_HEADER_LEN;
+               if (curPos + frameLen > pInfo->tagV2Info.tagLen)
                        break;
 
-               purelyFramelen = oneFrameLen - MP3_TAGv2_23_TXT_HEADER_LEN;
-               curPos += oneFrameLen;
+               if (frameLen <= 0)
+                       continue;
 
-               tag_id = __get_tag_info_v223(CompTmp);
-               if (tag_id == AV_ID3TAG_MAX || pInfo->tagInfo[tag_id].value || purelyFramelen == 0)
+               if (tag_id == AV_ID3TAG_MAX || pInfo->tagInfo[tag_id].value)
                        goto NEXT;
 
-               __get_v223_encoding_info(buffer, curPos, purelyFramelen, &encodingOffSet, &textEncodingType);
-               if (purelyFramelen <= encodingOffSet) {
-                       debug_warning(DEBUG, "warning: invalid frame length %lu %u", purelyFramelen, encodingOffSet);
-                       continue;
+               __get_v223_encoding_info(buffer + curPos, frameLen, &encOffset, &encType);
+               if (frameLen <= encOffset) {
+                       debug_warning(DEBUG, "warning: invalid frame length %d %d", frameLen, encOffset);
+                       goto NEXT;
                }
 
-               mmfile_free(pExtContent);
-               realCpyFrameNum = purelyFramelen - encodingOffSet;
-               pExtContent = g_malloc0(realCpyFrameNum + 3);
+               curPos += encOffset;
+               frameLen -= encOffset;
+               encOffset = 0;
 
-               if (textEncodingType != AV_ID3V2_UTF16 && textEncodingType != AV_ID3V2_UTF16_BE) {
-                       if (CompTmp[0] == 'T' || (strcmp(CompTmp, "APIC") == 0)) {
+               if (encType != AV_ID3V2_UTF16 && encType != AV_ID3V2_UTF16_BE) {
+                       if (tag_id != AV_ID3TAG_COMMENT && tag_id != AV_ID3TAG_UNSYNCLYRICS && tag_id != AV_ID3TAG_SYNCLYRICS) {
                                debug_msg(RELEASE, "get the new text encoding type");
-                               textEncodingType = buffer[curPos - purelyFramelen + encodingOffSet - 1];
+                               encType = buffer[curPos - 1];
                        }
                }
 
-               if (textEncodingType > AV_ID3V2_MAX) {
-                       debug_msg(DEBUG, "WRONG ENCOIDNG TYPE [%d], FRAME[%s]", textEncodingType, (char *)CompTmp);
-                       continue;
+               if (encType > AV_ID3V2_MAX) {
+                       debug_msg(DEBUG, "WRONG ENCOIDNG TYPE [%u], TAG ID[%d]", encType, tag_id);
+                       goto NEXT;
                }
 
-               memcpy(pExtContent, &buffer[curPos - purelyFramelen + encodingOffSet], purelyFramelen - encodingOffSet);
                switch (tag_id) {
                case AV_ID3TAG_COMMENT:
-                       if (realCpyFrameNum <= 3) {
-                               debug_msg(RELEASE, "Description info too small to parse realCpyFrameNum(%d)", realCpyFrameNum);
+                       if (frameLen <= 3) {
+                               debug_msg(RELEASE, "Description info too small to parse frameLen(%d)", frameLen);
                                break;
                        }
-                       realCpyFrameNum -= 3;
-                       tmp = 3;
+                       frameLen -= 3;
+                       encOffset = 3;
 
-                       /*pExtContent[tmp+1] value should't have encoding value */
-                       if (pExtContent[tmp] != 0x00 && pExtContent[tmp] != 0xFF && pExtContent[tmp] != 0xFE) {
-                               debug_msg(RELEASE, "failed to get Comment: tmp(%d), purelyFramelen - encodingOffSet(%lu)", tmp, purelyFramelen - encodingOffSet);
+                       if (buffer[curPos] != 0x00 && buffer[curPos] != 0xFF && buffer[curPos] != 0xFE) {
+                               debug_msg(RELEASE, "failed to get Comment: frameLen(%d)", frameLen);
                        }
 
-                       textEncodingType = __id3tag_get_text_encoding_v223(pExtContent, &realCpyFrameNum, textEncodingType, &tmp);
-                       debug_msg(RELEASE, "tmp(%d) textEncodingType(%d), realCpyFrameNum(%d)", tmp, textEncodingType, realCpyFrameNum);
-                       pInfo->tagInfo[tag_id].value = mmfile_convert_to_utf8((const char *)&pExtContent[tmp], realCpyFrameNum, charset_array[textEncodingType]);
+                       encType = __id3tag_get_text_encoding_v223(&buffer[curPos], &frameLen, encType, &encOffset);
+                       debug_msg(RELEASE, "encOffset(%d) encType(%u), frameLen(%d)", encOffset, encType, frameLen);
+                       curPos += encOffset;
+                       pInfo->tagInfo[tag_id].value = mmfile_convert_to_utf8((const char *)&buffer[curPos], frameLen, charset_array[encType]);
                        break;
 
                case AV_ID3TAG_SYNCLYRICS:
-                       if (realCpyFrameNum <= 5) {
-                               debug_msg(RELEASE, "Synchronised lyrics too small to parse realCpyFrameNum(%d)", realCpyFrameNum);
+                       if (frameLen <= 5) {
+                               debug_msg(RELEASE, "Synchronised lyrics too small to parse frameLen(%d)", frameLen);
                                break;
                        }
-                       realCpyFrameNum -= 5;
-                       tmp = 5;
+                       frameLen -= 5;
+                       encOffset = 5;
 
-                       /*pExtContent[tmp+1] value should't have encoding value */
-                       if (pExtContent[tmp] != 0x00 && pExtContent[tmp] != 0xFF && pExtContent[tmp] != 0xFE) {
-                               debug_msg(RELEASE, "failed to get Synchronised lyrics Info tmp(%d), purelyFramelen - encodingOffSet(%lu)", tmp, purelyFramelen - encodingOffSet);
+                       if (buffer[curPos] != 0x00 && buffer[curPos] != 0xFF && buffer[curPos] != 0xFE) {
+                               debug_msg(RELEASE, "failed to get Synchronised lyrics Info curPos(%d), frameLen(%d)", curPos, frameLen);
                        }
-                       textEncodingType = __id3tag_get_text_encoding_v223(pExtContent, &realCpyFrameNum, textEncodingType, &tmp);
-                       debug_msg(RELEASE, "tmp(%d) textEncodingType(%d), realCpyFrameNum(%d)", tmp, textEncodingType, realCpyFrameNum);
-                       __id3tag_parse_SYLT(pInfo, pExtContent, realCpyFrameNum, charset_array[textEncodingType], textEncodingType, tmp);
+                       encType = __id3tag_get_text_encoding_v223(&buffer[curPos], &frameLen, encType, &encOffset);
+                       debug_msg(RELEASE, "encOffset(%d) encType(%u), frameLen(%d)", encOffset, encType, frameLen);
+                       curPos += encOffset;
+                       __id3tag_parse_SYLT(pInfo, buffer, frameLen, charset_array[encType], encType, curPos);
                        break;
 
                case AV_ID3TAG_UNSYNCLYRICS:
-                       lang_info = strndup((char *)pExtContent, 3);
+                       lang_info = strndup((const char *)&buffer[curPos], 3);
 
-                       if (realCpyFrameNum <= 3) {
-                               debug_msg(RELEASE, "Unsynchronised lyrics too small to parse realCpyFrameNum(%d)", realCpyFrameNum);
+                       if (frameLen <= 3) {
+                               debug_msg(RELEASE, "Unsynchronised lyrics too small to parse frameLen(%d)", frameLen);
+                               mmfile_free(lang_info);
                                break;
                        }
-                       realCpyFrameNum -= 3;
-                       tmp = 3;
-
-                       /*find start of lyrics */
-                       while (1) {
-                               if (pExtContent[tmp] == 0x00) {
-                                       if (pExtContent[tmp + 1] == 0x00) {
-                                               realCpyFrameNum -= 2;
-                                               tmp += 2;
-                                       }
-                                       break;
-                               } else {
-                                       realCpyFrameNum--;
-                                       tmp++;
-                               }
-                       }
+                       frameLen -= 3;
+                       encOffset = 3;
 
-                       /*pExtContent[tmp+1] value should't have encoding value */
-                       debug_msg(RELEASE, "tpExtContent[%d] %x", tmp, pExtContent[tmp]);
-                       if (pExtContent[tmp] != 0x00 && pExtContent[tmp] != 0xFF && pExtContent[tmp] != 0xFE) {
-                               debug_msg(RELEASE, "failed to get Unsynchronised lyrics Info tmp(%d), purelyFramelen - encodingOffSet(%lu)", tmp, purelyFramelen - encodingOffSet);
+                       if (buffer[curPos] != 0x00 && buffer[curPos] != 0xFF && buffer[curPos] != 0xFE) {
+                               debug_msg(RELEASE, "failed to get Unsynchronised lyrics Info curPos(%d), frameLen(%d)", curPos, frameLen);
                        }
-                       textEncodingType = __id3tag_get_text_encoding_v223(pExtContent, &realCpyFrameNum, textEncodingType, &tmp);
+                       encType = __id3tag_get_text_encoding_v223(&buffer[curPos], &frameLen, encType, &encOffset);
 
                        char *char_set = NULL;
 
-                       debug_msg(RELEASE, "tmp(%d) textEncodingType(%d), realCpyFrameNum(%d)", tmp, textEncodingType, realCpyFrameNum);
+                       debug_msg(RELEASE, "encOffset(%d) encType(%u), frameLen(%d)", encOffset, encType, frameLen);
 
-                       if (textEncodingType == AV_ID3V2_ISO_8859) {
+                       if (encType == AV_ID3V2_ISO_8859) {
                                if (lang_info != NULL && !g_ascii_strcasecmp(lang_info, "KOR")) {
                                        char_set = strdup("EUC-KR");
                                } else {
-                                       char_set = mmfile_get_charset((const char *)&pExtContent[tmp]);
+                                       char_set = mmfile_get_charset((const char *)&buffer[curPos]);
                                }
                                mmfile_free(lang_info);
                        }
 
-                       if (char_set == NULL) {
-                               pInfo->tagInfo[tag_id].value = mmfile_convert_to_utf8((const char *)&pExtContent[tmp], realCpyFrameNum, charset_array[textEncodingType]);
+                       curPos += encOffset;
+
+                       if (!char_set) {
+                               pInfo->tagInfo[tag_id].value = mmfile_convert_to_utf8((const char *)&buffer[curPos], frameLen, charset_array[encType]);
                        } else {
-                               pInfo->tagInfo[tag_id].value = mmfile_convert_to_utf8((const char *)&pExtContent[tmp], realCpyFrameNum, char_set);
+                               pInfo->tagInfo[tag_id].value = mmfile_convert_to_utf8((const char *)&buffer[curPos], frameLen, char_set);
                                mmfile_free(char_set);
                        }
                        mmfile_free(lang_info);
@@ -3199,77 +3167,66 @@ bool mm_file_id3tag_parse_v223(AvFileContentInfo *pInfo, unsigned char *buffer,
 
                case AV_ID3TAG_PICTURE:
                        if (extract_artwork)
-                               _mm_file_id3tag_parse_APIC(pInfo, (unsigned char *)pExtContent, realCpyFrameNum, charset_array[textEncodingType]);
+                               _mm_file_id3tag_parse_APIC(pInfo, &buffer[curPos], frameLen, charset_array[encType]);
                        break;
 
                default:
-                       pInfo->tagInfo[tag_id].value = mmfile_convert_to_utf8((const char *)pExtContent, realCpyFrameNum, charset_array[textEncodingType]);
+                       pInfo->tagInfo[tag_id].value = mmfile_convert_to_utf8((const char *)&buffer[curPos], frameLen, charset_array[encType]);
                        break;
                }
 
                if (pInfo->tagInfo[tag_id].value)
                        debug_msg(RELEASE, "[%d] returned = (%s)", tag_id, pInfo->tagInfo[tag_id].value);
 NEXT:
-               mmfile_free(pExtContent);
-               memset(CompTmp, 0, 4);
-
-               if (curPos >= taglen)
-                       break;
-
-               needToloopv2taglen -= oneFrameLen;
-
-               oneFrameLen = 0;
-               encodingOffSet = 0;
-               realCpyFrameNum = 0;
-               textEncodingType = 0;
-               purelyFramelen = 0;
+               curPos += frameLen;
+               encOffset = 0;
+               encType = 0;
        }
 
        release_characterset_array(charset_array);
 
-       return (taglen > 0);
+       return true;
 }
 
 static void __get_v224_encoding_info(const unsigned char *buffer,
-                                                                       unsigned long position,
-                                                                       unsigned long length,
-                                                                       unsigned int *offset,
+                                                                       int length,
+                                                                       int *offset,
                                                                        unsigned int *type)
 {
-       unsigned int _offset = 0;
+       int _offset = 0;
 
-       if (!buffer || !offset || !type || (position < length))
+       if (!buffer || !offset || !type)
                return;
 
        /*in case of UTF 16 encoding */
        /*buffer+(position-length) data should '0x01' but in order to expansion, we don't accurately check the value. */
-       if (IS_ENCODEDBY_UTF16(buffer + (position - length))) {
+       if (IS_ENCODEDBY_UTF16(buffer)) {
                *offset = 2;
                *type = AV_ID3V2_UTF16;
                return;
        }
 
-       if (IS_ENCODEDBY_UTF16_R(buffer + (position - length))) {
+       if (IS_ENCODEDBY_UTF16_R(buffer)) {
                *offset = 2;
                *type = AV_ID3V2_UTF16_BE;
                return;
        }
 
-       if (IS_ENCODEDBY_UTF16(buffer + (position - length + 1))) {
+       if (IS_ENCODEDBY_UTF16(buffer + 1)) {
                *offset = 3;
                *type = AV_ID3V2_UTF16;
                return;
        }
-       if (IS_ENCODEDBY_UTF16_R(buffer + (position - length + 1))) {
+       if (IS_ENCODEDBY_UTF16_R(buffer + 1)) {
                *offset = 3;
                *type = AV_ID3V2_UTF16_BE;
                return;
        }
 
        /*in case of UTF-16 BE encoding */
-       if (buffer[position - length] == 0x02) {
+       if (buffer[0] == 0x02) {
                _offset = 1;
-               while ((buffer[position - length + _offset] == '\0') && (_offset < length))
+               while ((buffer[_offset] == '\0') && (_offset < length))
                        _offset++;/*null skip! */
                *offset = _offset;
                *type = AV_ID3V2_UTF16_BE;
@@ -3277,9 +3234,9 @@ static void __get_v224_encoding_info(const unsigned char *buffer,
        }
 
        /*in case of UTF8 encoding */
-       if (buffer[position - length] == 0x03) {
+       if (buffer[0] == 0x03) {
                _offset = 1;
-               while ((buffer[position - length + _offset] == '\0') && (_offset < length))
+               while ((buffer[_offset] == '\0') && (_offset < length))
                        _offset++;/*null skip! */
                *offset = _offset;
                *type = AV_ID3V2_UTF8;
@@ -3288,7 +3245,7 @@ static void __get_v224_encoding_info(const unsigned char *buffer,
        /*in case of ISO-8859-1 encoding */
        /*buffer+(position-length) data should 0x00 but in order to expansion, we don't accurately check the value. */
        _offset = 1;
-       while ((buffer[position - length + _offset] < 0x20) && (_offset < length))
+       while ((buffer[_offset] < 0x20) && (_offset < length))
                _offset++;/*less than 0x20 value skip! */
        *offset = _offset;
        *type = AV_ID3V2_ISO_8859;
@@ -3296,166 +3253,123 @@ static void __get_v224_encoding_info(const unsigned char *buffer,
 
 bool mm_file_id3tag_parse_v224(AvFileContentInfo *pInfo, unsigned char *buffer, bool extract_artwork)
 {
-       unsigned long taglen = 0;
-       unsigned long needToloopv2taglen;
-       unsigned long oneFrameLen = 0;
-       unsigned long curPos = 0;
-       char CompTmp[5];
-       unsigned char *pExtContent = NULL;
-       unsigned long purelyFramelen = 0;
-       unsigned int encodingOffSet = 0;
-       int realCpyFrameNum = 0,  tmp = 0;
-       unsigned int textEncodingType = 0;
+       int extendedHeaderLen = 0;
+       int frameLen = 0;
+       int curPos = MP3_TAGv2_HEADER_LEN;
+       int encOffset = 0;
+       unsigned int encType = 0;
        char **charset_array = NULL;
        AvID3TagList tag_id = AV_ID3TAG_MAX;
 
-       make_characterset_array(&charset_array);
+       if (pInfo->tagV2Info.tagLen <= 0)
+               return false;
 
+       make_characterset_array(&charset_array);
        init_content_info(pInfo);
 
-       taglen = pInfo->tagV2Info.tagLen;
-       needToloopv2taglen = taglen - MP3_TAGv2_HEADER_LEN;
-       curPos = MP3_TAGv2_HEADER_LEN;
-
        debug_msg(RELEASE, "ID3tag v224--------------------------------------------------------------");
-
-       /* check Extended Header */
        if (buffer[5] & 0x40) {
                /* if extended header exists, skip it*/
-               int extendedHeaderLen = (unsigned long)buffer[10] << 21 | (unsigned long)buffer[11] << 14 | (unsigned long)buffer[12] << 7  | (unsigned long)buffer[13];
-
+               extendedHeaderLen = buffer[10] << 21 | buffer[11] << 14 | buffer[12] << 7 | buffer[13];
                debug_msg(RELEASE, "--------------- extendedHeaderLen = %d", extendedHeaderLen);
-
-               if (extendedHeaderLen > (int)(taglen - curPos)) {
+               if (extendedHeaderLen > pInfo->tagV2Info.tagLen - MP3_TAGv2_HEADER_LEN) {
                        debug_error(DEBUG, "extended header too long.");
                } else {
                        curPos += extendedHeaderLen;
                }
        }
 
-       while (needToloopv2taglen > MP3_TAGv2_23_TXT_HEADER_LEN) {
+       while (curPos + MP3_TAGv2_23_TXT_HEADER_LEN < pInfo->tagV2Info.tagLen) {
                if (!g_ascii_isalnum(buffer[curPos]) || !g_ascii_isalnum(buffer[curPos + 1]) ||
                        !g_ascii_isalnum(buffer[curPos + 2]) || !g_ascii_isalnum(buffer[curPos + 3]))
                        break;
 
-               memcpy(CompTmp, &buffer[curPos], 4);
-
-               CompTmp[4] = 0;
-               oneFrameLen = MP3_TAGv2_23_TXT_HEADER_LEN;
-               oneFrameLen += (unsigned long)buffer[4 + curPos] << 21 | (unsigned long)buffer[5 + curPos] << 14
-                                       | (unsigned long)buffer[6 + curPos] << 7 | (unsigned long)buffer[7 + curPos];
-               if (oneFrameLen > taglen - curPos)
+               tag_id = __get_tag_info_v223(&buffer[curPos]);
+               frameLen = buffer[curPos + 4] << 21 | buffer[curPos + 5] << 14 | buffer[curPos + 6] << 7 | buffer[curPos + 7];
+               curPos += MP3_TAGv2_23_TXT_HEADER_LEN;
+               if (curPos + frameLen > pInfo->tagV2Info.tagLen)
                        break;
 
-               purelyFramelen = oneFrameLen - MP3_TAGv2_23_TXT_HEADER_LEN;
-               curPos += oneFrameLen;
-
-               debug_msg(RELEASE, "-----------------------------------------------------------------------------------");
+               if (frameLen <= 0)
+                       continue;
 
-               tag_id = __get_tag_info_v223(CompTmp);
-               if (tag_id == AV_ID3TAG_MAX || pInfo->tagInfo[tag_id].value || purelyFramelen == 0)
+               if (tag_id == AV_ID3TAG_MAX || pInfo->tagInfo[tag_id].value)
                        goto NEXT;
 
-               __get_v224_encoding_info(buffer, curPos, purelyFramelen, &encodingOffSet, &textEncodingType);
-               if (purelyFramelen <= encodingOffSet) {
-                       debug_warning(DEBUG, "warning: invalid frame length %lu %u", purelyFramelen, encodingOffSet);
-                       continue;
+               __get_v224_encoding_info(buffer + curPos, frameLen, &encOffset, &encType);
+               if (frameLen <= encOffset) {
+                       debug_warning(DEBUG, "warning: invalid frame length %d %d", frameLen, encOffset);
+                       goto NEXT;
                }
 
-               mmfile_free(pExtContent);
-               realCpyFrameNum = purelyFramelen - encodingOffSet;
-               pExtContent = g_malloc0(realCpyFrameNum + 3);
+               curPos += encOffset;
+               frameLen -= encOffset;
+               encOffset = 0;
 
-               if (textEncodingType != AV_ID3V2_UTF16 && textEncodingType != AV_ID3V2_UTF16_BE) {
-                       if (CompTmp[0] == 'T' || (strcmp(CompTmp, "APIC") == 0)) {
+               if (encType != AV_ID3V2_UTF16 && encType != AV_ID3V2_UTF16_BE) {
+                       if (tag_id != AV_ID3TAG_COMMENT && tag_id != AV_ID3TAG_UNSYNCLYRICS && tag_id != AV_ID3TAG_SYNCLYRICS) {
                                debug_msg(RELEASE, "get the new text encoding type");
-                               textEncodingType = buffer[curPos - purelyFramelen + encodingOffSet - 1];
+                               encType = buffer[curPos - 1];
                        }
                }
 
-               if (textEncodingType > AV_ID3V2_MAX) {
-                       debug_msg(DEBUG, "WRONG ENCOIDNG TYPE [%d], FRAME[%s]", textEncodingType, (char *)CompTmp);
-                       continue;
+               if (encType > AV_ID3V2_MAX) {
+                       debug_msg(DEBUG, "WRONG ENCOIDNG TYPE [%u], TAG ID[%d]", encType, tag_id);
+                       goto NEXT;
                }
 
-               memcpy(pExtContent, &buffer[curPos - purelyFramelen + encodingOffSet], purelyFramelen - encodingOffSet);
-
                switch (tag_id) {
                case AV_ID3TAG_COMMENT:
-                       if (realCpyFrameNum <= 3) {
-                               debug_msg(RELEASE, "Description info too small to parse realCpyFrameNum(%d)", realCpyFrameNum);
+                       /* fall through */
+               case AV_ID3TAG_UNSYNCLYRICS:
+                       if (frameLen <= 3) {
+                               debug_msg(RELEASE, "Description info too small to parse frameLen(%d)", frameLen);
                                break;
                        }
+                       frameLen -= 3;
+                       encOffset = 3;
 
-                       realCpyFrameNum -= 3;
-                       tmp = 3;
-
-                       textEncodingType = __id3tag_get_text_encoding_v224(pExtContent, &realCpyFrameNum, textEncodingType, &tmp);
-                       debug_msg(RELEASE, "tmp(%d) textEncodingType(%d), realCpyFrameNum(%d)", tmp, textEncodingType, realCpyFrameNum);
-
-                       pInfo->tagInfo[tag_id].value = mmfile_convert_to_utf8((const char *)&pExtContent[tmp], realCpyFrameNum, charset_array[textEncodingType]);
+                       encType = __id3tag_get_text_encoding_v224(&buffer[curPos], &frameLen, encType, &encOffset);
+                       debug_msg(RELEASE, "encOffset(%d) encType(%u), frameLen(%d)", encOffset, encType, frameLen);
+                       curPos += encOffset;
+                       pInfo->tagInfo[tag_id].value = mmfile_convert_to_utf8((const char *)&buffer[curPos], frameLen, charset_array[encType]);
                        break;
 
                case AV_ID3TAG_SYNCLYRICS:
-                       if (realCpyFrameNum <= 5) {
-                               debug_msg(RELEASE, "SyncLyrics info too small to parse realCpyFrameNum(%d)", realCpyFrameNum);
+                       if (frameLen <= 5) {
+                               debug_msg(RELEASE, "Synchronised lyrics too small to parse frameLen(%d)", frameLen);
                                break;
                        }
-                       realCpyFrameNum -= 5;
-                       tmp = 5;
-
-                       textEncodingType = __id3tag_get_text_encoding_v224(pExtContent, &realCpyFrameNum, textEncodingType, &tmp);
-                       debug_msg(RELEASE, "tmp(%d) textEncodingType(%d), realCpyFrameNum(%d)", tmp, textEncodingType, realCpyFrameNum);
+                       frameLen -= 5;
+                       encOffset = 5;
 
-                       __id3tag_parse_SYLT(pInfo, pExtContent, realCpyFrameNum, charset_array[textEncodingType], textEncodingType, tmp);
-                       break;
-
-               case AV_ID3TAG_UNSYNCLYRICS:
-                       if (realCpyFrameNum <= 3) {
-                               debug_msg(RELEASE, "Description info too small to parse realCpyFrameNum(%d)", realCpyFrameNum);
-                               break;
-                       }
-                       realCpyFrameNum -= 3;
-                       tmp = 3;
-
-                       textEncodingType = __id3tag_get_text_encoding_v224(pExtContent, &realCpyFrameNum, textEncodingType, &tmp);
-                       debug_msg(RELEASE, "tmp(%d) textEncodingType(%d), realCpyFrameNum(%d)", tmp, textEncodingType, realCpyFrameNum);
-
-                       pInfo->tagInfo[tag_id].value = mmfile_convert_to_utf8((const char *)&pExtContent[tmp], realCpyFrameNum, charset_array[textEncodingType]);
+                       encType = __id3tag_get_text_encoding_v224(&buffer[curPos], &frameLen, encType, &encOffset);
+                       debug_msg(RELEASE, "encOffset(%d) encType(%u), frameLen(%d)", encOffset, encType, frameLen);
+                       curPos += encOffset;
+                       __id3tag_parse_SYLT(pInfo, buffer, frameLen, charset_array[encType], encType, curPos);
                        break;
 
                case AV_ID3TAG_PICTURE:
                        if (extract_artwork)
-                               _mm_file_id3tag_parse_APIC(pInfo, (unsigned char *)pExtContent, realCpyFrameNum, charset_array[textEncodingType]);
+                               _mm_file_id3tag_parse_APIC(pInfo, &buffer[curPos], frameLen, charset_array[encType]);
                        break;
 
                default:
-                       pInfo->tagInfo[tag_id].value = mmfile_convert_to_utf8((const char *)pExtContent, realCpyFrameNum, charset_array[textEncodingType]);
+                       pInfo->tagInfo[tag_id].value = mmfile_convert_to_utf8((const char *)&buffer[curPos], frameLen, charset_array[encType]);
                        break;
                }
 
                if (pInfo->tagInfo[tag_id].value)
                        debug_msg(RELEASE, "[%d] returned = (%s)", tag_id, pInfo->tagInfo[tag_id].value);
-
 NEXT:
-               mmfile_free(pExtContent);
-               memset(CompTmp, 0, 4);
-
-               if (curPos >= taglen)
-                       break;
-
-               needToloopv2taglen -= oneFrameLen;
-
-               oneFrameLen = 0;
-               encodingOffSet = 0;
-               realCpyFrameNum = 0;
-               textEncodingType = 0;
-               purelyFramelen = 0;
+               curPos += frameLen;
+               encOffset = 0;
+               encType = 0;
        }
 
        release_characterset_array(charset_array);
 
-       return (taglen > 0);
+       return true;
 }