[Libcdio-devel] [PATCH v2] cdtext: Count empty fields as tracks
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
Re: [Libcdio-devel] [PATCH v2] cdtext: Count empty fields as tracks
I have committed this patch to the branch count-empty-tracks-as-field-v2; see https://git.savannah.gnu.org/cgit/libcdio.git/log/?h=count-empty-tracks-as-field-v2 I invite review of this. If after a week or so there are no objections, I will merge this into the master branch. On Thu, Sep 5, 2024 at 10:13 AM John Ernberg wrote: > 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
[Libcdio-devel] RFC - move libcdio from savannah.gnu.org to github?
I'd like your opinion about moving the libcdio git repository from savannah.gnu.org to github (or some other git repository) The mechanisms at savannah.gnu.org have not been improving or developing while github, gitlab, etc., seem to be constantly improving. Your thoughts?
