The following CD-TEXT would fail when parsing due to an off-by-one in the songwriter data type because the album-level field is empty:
80 0 0 0 41 20 4d 65 6d 6f 69 72 20 6f 66 20 5f 23 80 0 1 c 46 72 65 65 20 57 69 6c 6c 0 47 6f 5b 8d 80 1 2 2 6e 65 20 42 75 74 20 4e 6f 74 20 46 b0 df 80 1 3 e 6f 72 67 6f 74 74 65 6e 0 49 6e 73 9 18 80 2 4 3 61 6e 69 74 79 20 41 20 4d 6f 6e 69 49 27 80 2 5 f 6b 65 72 20 4f 66 20 4d 65 0 54 6f 13 d2 80 3 6 2 20 50 65 72 73 69 73 74 20 6f 72 20 3b 87 80 3 7 e 41 64 68 65 72 65 0 41 6e 20 45 6c 63 87 80 4 8 5 65 67 79 20 66 6f 72 20 61 20 4d 61 bc e6 80 4 9 f 6e 20 41 6c 69 76 65 0 50 73 79 63 93 89 80 5 a 4 68 6f 74 69 63 6c 79 73 6d 0 41 6e d6 a 80 6 b 2 20 49 6e 74 72 61 6d 75 72 61 6c 20 aa b6 80 6 c e 4d 61 64 6e 65 73 73 0 51 75 65 73 b8 0 80 7 d 4 74 69 6f 6e 73 20 6f 66 20 61 20 48 60 11 80 7 e f 6f 6c 69 73 74 69 63 20 44 69 76 69 bb b8 80 7 f f 6e 65 0 57 69 74 68 20 56 69 72 74 60 f5 80 8 10 9 75 65 20 49 20 41 6d 20 46 72 65 65 21 6 80 8 11 f 0 42 61 74 74 6c 65 73 20 41 72 65 a5 b9 80 9 12 b 20 57 6f 6e 20 57 69 74 68 69 6e 0 b0 f3 80 a 13 0 41 20 4d 65 6d 69 6f 72 20 6f 66 20 52 2f 80 a 14 c 46 72 65 65 20 57 69 6c 6c 0 0 0 78 30 81 0 15 0 4b 72 6f 73 69 73 0 4b 72 6f 73 69 91 f1 81 1 16 5 73 0 9 0 9 0 9 0 9 0 9 0 80 2d 81 7 17 0 9 0 9 0 9 0 9 0 0 0 0 0 bd f4 82 0 18 0 0 4b 72 6f 73 69 73 0 9 0 9 0 47 9c ^^^ 82 4 19 0 9 0 9 0 9 0 9 0 9 0 9 0 1f 7f 82 a 1a 0 9 0 0 0 0 0 0 0 0 0 0 0 1e f8 83 0 1b 0 0 4b 72 6f 73 69 73 0 9 0 9 0 62 1b 83 4 1c 0 9 0 9 0 9 0 9 0 9 0 9 0 31 9f 83 a 1d 0 9 0 0 0 0 0 0 0 0 0 0 0 c6 da 84 0 1e 0 0 4b 72 6f 73 69 73 0 9 0 9 0 8c 40 84 4 1f 0 9 0 9 0 9 0 9 0 9 0 9 0 d4 a3 84 a 20 0 9 0 0 0 0 0 0 0 0 0 0 0 98 d2 8e 0 21 0 0 51 4d 37 32 38 31 39 30 35 32 30 2c cb 8e 1 22 b 34 0 51 4d 37 32 38 31 39 30 35 32 31 28 8e 2 23 a 30 35 0 51 4d 37 32 38 31 39 30 35 1a 5c 8e 3 24 9 32 30 36 0 51 4d 37 32 38 31 39 30 53 af 8e 4 25 8 35 32 30 37 0 51 4d 37 32 38 31 39 6c 0 8e 5 26 7 30 35 32 30 38 0 51 4d 37 32 38 31 c6 9b 8e 6 27 6 39 30 35 32 30 39 0 51 4d 37 32 38 39 e0 8e 7 28 5 31 39 30 35 32 31 30 0 51 4d 37 32 48 67 8e 8 29 4 38 31 39 30 35 32 31 31 0 51 4d 37 bf 19 8e 9 2a 3 32 38 31 39 30 35 32 31 32 0 51 4d 17 fa 8e a 2b 2 37 32 38 31 39 30 35 32 31 33 0 0 6f 43 8f 0 2c 0 0 1 a 0 15 3 3 3 3 0 0 0 5f 7b 8f 1 2d 0 0 0 0 0 0 0 b 3 2e 0 0 0 68 6c 8f 2 2e 0 0 0 0 0 9 0 0 0 0 0 0 0 e7 87 With the album-level field empty cur_track was never incremented, causing an off-by-one. Since the CD-TEXT additionally makes use of the Tab Indicator the entire CD-TEXT is thrown out as invalid due to a tab found in track 1 (that should be track 2). Fix this by always incrementing cur_track on a termination, even if the buffer contains nothing while still avoiding adding any blank fields to the parsed CD-TEXT structure. Include the above CD-TEXT in the test data. --- v2: - Avoid regression with previous CD-TEXT tests by not adding the buffer if it's empty. (Rocky) - Include the CD-TEXT in the test suite. - Fix a confusion regarding the field IDs in the commit message. According to the CD-TEXT specification says that if a field of type 80 to 85 or 8e is present for one track, it must be present for all of them. The regression in the tests was that MESSAGE and GENRE got added where they weren't before. In terms of MESSAGE that was probably more correct, but starting to add them in such cases is maybe an API break. If you'd prefer that there still needs to be some filtering in GENRE, I wonder if in that case it should be limited to track 0 since it's not part of the list of fields that need to be present for all tracks if used. --- lib/driver/cdtext.c | 69 +++++++++++++++++---------------- test/cdtext-krosis.right | 75 ++++++++++++++++++++++++++++++++++++ test/check_cdtext.sh | 6 +++ test/data/cdtext-krosis.cdt | Bin 0 -> 846 bytes 4 files changed, 117 insertions(+), 33 deletions(-) create mode 100644 test/cdtext-krosis.right create mode 100644 test/data/cdtext-krosis.cdt diff --git a/lib/driver/cdtext.c b/lib/driver/cdtext.c index a80d5974..d92c9434 100644 --- a/lib/driver/cdtext.c +++ b/lib/driver/cdtext.c @@ -799,7 +799,7 @@ cdtext_data_init(cdtext_t *p_cdtext, uint8_t *wdata, size_t i_data) buffer[i_buf++] = pack.text[j]; if(pack.db_chars) buffer[i_buf++] = pack.text[j+1]; - } else if(i_buf > 0) { + } else { /* if end of string */ /* check if the buffer contains only the Tab Indicator */ @@ -815,38 +815,41 @@ cdtext_data_init(cdtext_t *p_cdtext, uint8_t *wdata, size_t i_data) buffer[i_buf++] = 0; } - switch (pack.type) { - case CDTEXT_PACK_TITLE: - cdtext_set(p_cdtext, CDTEXT_FIELD_TITLE, buffer, cur_track, charset); - break; - case CDTEXT_PACK_PERFORMER: - cdtext_set(p_cdtext, CDTEXT_FIELD_PERFORMER, buffer, cur_track, charset); - break; - case CDTEXT_PACK_SONGWRITER: - cdtext_set(p_cdtext, CDTEXT_FIELD_SONGWRITER, buffer, cur_track, charset); - break; - case CDTEXT_PACK_COMPOSER: - cdtext_set(p_cdtext, CDTEXT_FIELD_COMPOSER, buffer, cur_track, charset); - break; - case CDTEXT_PACK_ARRANGER: - cdtext_set(p_cdtext, CDTEXT_FIELD_ARRANGER, buffer, cur_track, charset); - break; - case CDTEXT_PACK_MESSAGE: - cdtext_set(p_cdtext, CDTEXT_FIELD_MESSAGE, buffer, cur_track, charset); - break; - case CDTEXT_PACK_DISCID: - if (cur_track == 0) - cdtext_set(p_cdtext, CDTEXT_FIELD_DISCID, buffer, cur_track, NULL); - break; - case CDTEXT_PACK_GENRE: - cdtext_set(p_cdtext, CDTEXT_FIELD_GENRE, buffer, cur_track, "ASCII"); - break; - case CDTEXT_PACK_UPC: - if (cur_track == 0) - cdtext_set(p_cdtext, CDTEXT_FIELD_UPC_EAN, buffer, cur_track, "ASCII"); - else - cdtext_set(p_cdtext, CDTEXT_FIELD_ISRC, buffer, cur_track, "ISO-8859-1"); - break; + if ( ! CDTEXT_COMPARE_CHAR(buffer, '\0', pack.db_chars)) { + /* implies cur_track is in valid range */ + switch (pack.type) { + case CDTEXT_PACK_TITLE: + cdtext_set(p_cdtext, CDTEXT_FIELD_TITLE, buffer, cur_track, charset); + break; + case CDTEXT_PACK_PERFORMER: + cdtext_set(p_cdtext, CDTEXT_FIELD_PERFORMER, buffer, cur_track, charset); + break; + case CDTEXT_PACK_SONGWRITER: + cdtext_set(p_cdtext, CDTEXT_FIELD_SONGWRITER, buffer, cur_track, charset); + break; + case CDTEXT_PACK_COMPOSER: + cdtext_set(p_cdtext, CDTEXT_FIELD_COMPOSER, buffer, cur_track, charset); + break; + case CDTEXT_PACK_ARRANGER: + cdtext_set(p_cdtext, CDTEXT_FIELD_ARRANGER, buffer, cur_track, charset); + break; + case CDTEXT_PACK_MESSAGE: + cdtext_set(p_cdtext, CDTEXT_FIELD_MESSAGE, buffer, cur_track, charset); + break; + case CDTEXT_PACK_DISCID: + if (cur_track == 0) + cdtext_set(p_cdtext, CDTEXT_FIELD_DISCID, buffer, cur_track, NULL); + break; + case CDTEXT_PACK_GENRE: + cdtext_set(p_cdtext, CDTEXT_FIELD_GENRE, buffer, cur_track, "ASCII"); + break; + case CDTEXT_PACK_UPC: + if (cur_track == 0) + cdtext_set(p_cdtext, CDTEXT_FIELD_UPC_EAN, buffer, cur_track, "ASCII"); + else + cdtext_set(p_cdtext, CDTEXT_FIELD_ISRC, buffer, cur_track, "ISO-8859-1"); + break; + } } i_buf = 0; ++cur_track; diff --git a/test/cdtext-krosis.right b/test/cdtext-krosis.right new file mode 100644 index 00000000..3e15ea6b --- /dev/null +++ b/test/cdtext-krosis.right @@ -0,0 +1,75 @@ + +Language 0 'English': +CD-TEXT for Disc: + TITLE: A Memoir of Free Will + PERFORMER: Krosis +CD-TEXT for Track 1: + TITLE: Gone But Not Forgotten + PERFORMER: Krosis + SONGWRITER: Krosis + COMPOSER: Krosis + ARRANGER: Krosis + ISRC: QM7281905204 +CD-TEXT for Track 2: + TITLE: Insanity A Moniker Of Me + PERFORMER: Krosis + SONGWRITER: Krosis + COMPOSER: Krosis + ARRANGER: Krosis + ISRC: QM7281905205 +CD-TEXT for Track 3: + TITLE: To Persist or Adhere + PERFORMER: Krosis + SONGWRITER: Krosis + COMPOSER: Krosis + ARRANGER: Krosis + ISRC: QM7281905206 +CD-TEXT for Track 4: + TITLE: An Elegy for a Man Alive + PERFORMER: Krosis + SONGWRITER: Krosis + COMPOSER: Krosis + ARRANGER: Krosis + ISRC: QM7281905207 +CD-TEXT for Track 5: + TITLE: Psychoticlysm + PERFORMER: Krosis + SONGWRITER: Krosis + COMPOSER: Krosis + ARRANGER: Krosis + ISRC: QM7281905208 +CD-TEXT for Track 6: + TITLE: An Intramural Madness + PERFORMER: Krosis + SONGWRITER: Krosis + COMPOSER: Krosis + ARRANGER: Krosis + ISRC: QM7281905209 +CD-TEXT for Track 7: + TITLE: Questions of a Holistic Divine + PERFORMER: Krosis + SONGWRITER: Krosis + COMPOSER: Krosis + ARRANGER: Krosis + ISRC: QM7281905210 +CD-TEXT for Track 8: + TITLE: With Virtue I Am Free + PERFORMER: Krosis + SONGWRITER: Krosis + COMPOSER: Krosis + ARRANGER: Krosis + ISRC: QM7281905211 +CD-TEXT for Track 9: + TITLE: Battles Are Won Within + PERFORMER: Krosis + SONGWRITER: Krosis + COMPOSER: Krosis + ARRANGER: Krosis + ISRC: QM7281905212 +CD-TEXT for Track 10: + TITLE: A Memior of Free Will + PERFORMER: Krosis + SONGWRITER: Krosis + COMPOSER: Krosis + ARRANGER: Krosis + ISRC: QM7281905213 diff --git a/test/check_cdtext.sh b/test/check_cdtext.sh index dfbfc9a5..7ae0634d 100755 --- a/test/check_cdtext.sh +++ b/test/check_cdtext.sh @@ -68,4 +68,10 @@ test_cdtext_raw "$opts" ${srcdir}/${fname}.dump ${srcdir}/${fname}.right RC=$? check_result $RC "CD-Text libburnia roundtrip" "${CDTEXT_RAW} $opts" +fname=cdtext-krosis +opts="${srcdir}/data/${fname}.cdt" +test_cdtext_raw "$opts" ${srcdir}/${fname}.dump ${srcdir}/${fname}.right +RC=$? +check_result $RC "CD-Text Krosis blanks + tab indicator" "${CDTEXT_RAW} $opts" + exit 0 diff --git a/test/data/cdtext-krosis.cdt b/test/data/cdtext-krosis.cdt new file mode 100644 index 0000000000000000000000000000000000000000..eeb99672ce0a08bbb8b370fddbcf20b1be67b90d GIT binary patch literal 846 zcmY+BO-vI(7>2+7X}7z@Rz&1aVGtEm&~6ohc(JHJW2&G=4w}GP20C%Olk853jRyy4 zLQHgt@nWKh2jj(yoD4>dH!j|Y@!&;IdNyieJUL6D(3!(G^X$Ce&$}D|QYeG-l(>ja zqlpd<Qq7QyF&Zbf4a4;M11`z3gVA7tp^KEEq2;e!QexDdp^RY%(vGJ$a#>X@hp-|N z*{DO(om>vZZ($c*nilEdDy>`NN?1mt*!76V5OvY%MXqSE)Km_;7*Y;8Z)X>|8VTW< zBAOQ8EtIj|ednqkcTmbEvlvFbV)oHJ9x~J%Wh84Cy}S^9Hh4HHqqM_ZD_?Le8@+tZ z!!@ytHRWK>gRuhkR)A|eR7R-d1!G!hWU`iPu{gCwGbD@7ky-L~g=_J+gJGPo9J)qa zHu;A~cIpL;(kPWjfh+A{9;uDPpv9PtJ(P0si#J@~6-DFJ5stAOabWouH|ol+5;2A- zclkIsc5ibP;C5mT>fwS*g&F1W@aLSgC**-%*}P{?YpC4BkAnBV=b;fcWx>ngr}Ju4 z<xN1^?#>&{B5tkLzYEY(rMlU&pzf_wna>uCeO0Q}FC}QLQr$mOQroIjpPrWtRHb^n z=0iKcSf(%8Zw@4SlZoz+zO+AjavL(ce7VC&^lqW%8@_TtPbN-P(3MxddN3m7eVfPz zKXgb7GWKtv)hB+qGrV48po|8-`C3<Ky{?&9M<X+S<gi+9#WVx-p~=^eC>!0GNr4*v u`NmOsvtjcD06n_|-2jpS^@>=+%85HmQqP8^QRSF8<?JOnh)|jT$KrpTmdKp| literal 0 HcmV?d00001 -- 2.45.1