bugCompact Disc Input and Control Library - Bugs: bug #53929, cd-text with invalid characters...

 
 

bug #53929: cd-text with invalid characters failing to convert to utf8

Submitter:  Serge Pouliquen <sp31415>
Submitted:  Thu 17 May 2018 12:40:28 PM UTC
   
 
Category:  None Severity:  5 - Average
Item Group:  cd-info Status:  Fixed
Privacy:  Public Assigned to:  scdbackup
Open/Closed:  Closed
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Tue 07 May 2019 03:51:07 PM UTC, comment #12: 

Hi,

by commit
  http://git.savannah.gnu.org/cgit/libcdio.git/commit/?id=aedfd7c7c2ad0d1945b61c9179e81d955feca304
the reason for this bug report is supposed to be handled as good as
possible. Everything but charcter set SHIFT_JIS is now converted from
ISO-8859-1 to UTF-8.

This does not totally prevent conversion refusals because there are
undefined byte codes in ISO-8859-1. But we cannot do better by e.g.
using the extension CP1252, because this contains an incomplete circumflex
character which has to be followd by a suitable vowel.

Have a nice day :)

Thomas

Thomas Schmitt <scdbackup>
Group Member
Sat 27 Apr 2019 02:18:01 PM UTC, comment #11: 

Hi,

cdrskin_output.txt has:

> 73 : 8f 00 49 00 01 ...


Yep. It says the CD-TEXT is encoded in "ASCII".
Different than written by me previously, it is byte 4 (i.e. the fifth)
in the first 0x8f pack, not in each 0x8f pack.

I will now start a discussion at libcdio-devel, how tolerant libcdio
shall be towards this mistake.

You may try this change in lib/driver/cdtext.c:

--- lib/driver/cdtext.c 2018-06-14 17:26:07.742400554 +0200
+++ lib/driver/cdtext.bug53929.c        2019-04-27 15:08:22.336291660 +0200
@@ -717,7 +717,12 @@ cdtext_data_init(cdtext_t *p_cdtext, uin
             charset = (char *) "ISO-8859-1";
             break;
           case CDTEXT_CHARCODE_ASCII:
-            charset = (char *) "ASCII";
+            /* ASCII is a subset of ISO-8859-1. Some CDs announce it but then
+             * have 8-bit characters in their text. Trying ISO-8859-1 gives
+             * more hope for a readable result than telling iconv to be picky.
+             */
+            charset = (char *) "ISO-8859-1";
             break;
           case CDTEXT_CHARCODE_SHIFT_JIS:
             charset = (char *) "SHIFT_JIS";

Have a nice day :)

Thomas

Thomas Schmitt <scdbackup>
Group Member
Sat 27 Apr 2019 09:23:11 AM UTC, comment #10: 

Hi,

i forgot to mention that the cdrskin run produces a data file in
the current directory, named "cdtext.dat".
It will not overwrite an existing file of that name, though.

The file contains the CD-TEXT header and the packs as binary data.
You may delete it.

Have a nice day :)

Thomas

Thomas Schmitt <scdbackup>
Group Member
Sat 27 Apr 2019 07:15:23 AM UTC, comment #9: 

Hi,

(I send a copy of this text as mail to you. The display of text
 paragraphs and indentation is awful in the savannah bug tracker.)
 
i wrote:

> > Main suspect is that variable charset (i.e. the input encoding) is not set
> > to "ISO-8859-1" but rather to "ASCII" or "SHIFT_JIS".


Serge Pouliquen wrote:

> When entering    cdtext_set(cdtext_t *p_cdtext, cdtext_field_t key, const
> uint8_t *value, track_t track, const char *charset)
> The charset value is ASCII


That is wrong and most probably the reason for the conversion failure.


> problematic value list (for that cd/media):
> in_toc:\347   expected:ç   matching_character_without_accent:c   track6


The c-cedille is correctly encoded in ISO-8859-1. iconv would be able
to translate it to 0xC3 0xA7, the two-byte UTF-8 code, if it was told
that the input is ISO-8859-1 and not ASCII.

So we will most probably not need to reduce semi-exotic characters to
their nearest ASCII equivalent.
We rather need to find out why the conversion input character set is
not correctly.

If it is libcdio's fault, then the gdb stack trace would help to find
possible program errors. See below for instructions. But first obtain
the raw CD-TEXT packs by the following cdrskin run. Maybe we do not
need the stack trace at all.

If the CD is at fault, then we might see the reason by looking at the
raw CD-TEXT data, pack type 0x8f, where byte 4 tells the character set
of the text packs: 0x00 = ISO-8859, 0x01 = 7-bit ASCII, 0x80 = MS-JIS.

You may obtain the raw packs from CD by a run of my burn program
cdrskin:

  cdrskin dev=/dev/sr0 -vvv -toc

After other information about the CD you will see the list of raw packs
like

  CD-TEXT data from CD Lead-in:
     0 : 80 00 00 00  J  o  y  f  u  l     N  i  g  h  t f0 f7
     1 : 80 00 01 0c  s 00  S  o  n  g     o  f     J  o 43 1c
     ...
     43 : 8f 00 2b 00 00 04 06 03 05 06 06 05 03 06 01 02 07 0d
     44 : 8f 01 2c 00 00 00 00 00 00 04 05 03 2d 00 00 00 c9 b4
     45 : 8f 02 2d 00 00 00 00 00 09 00 00 00 00 00 00 00 6a 24

Character set info is encoded in the byte after the first four bytes
of the lines which start by "8f". E.g. in pack #43 the first four
bytes are "8f 00 2b 00" (which we do not interpret further) and the
next byte is "00". So this CD announces ISO-8859-1.

If it was "01", then the CD would announce ASCII, where all character
codes must be in the range of 1 to 127 (decimal). The c-cedille has
ISO-8859-1 code 231, which would then be not allowed in 7-bit ASCII.

So please show your CD's raw pack list (best as attached file).


> On my side, "cdtext from that media is buggy and cannot be read" is
> quite an acceptable response.


This might be the result of the examination if the byte of interest
shows value "01" rather than "00".
But if the CD announces ISO-8859-1 by "00", then we need to hunt a bug
in libcdio.

In case that the CD is to blame, we could add some code to libcdio
which changes character codes larger than 127 to e.g. character '_' if
the character set is announced as 7-bit ASCII.
A loud warning to the user will then be appropriate.

---------------------------------------------------------------------

Only if the CD announces ISO-8859-1 as character set, we need to find
out why libcdio tries to convert the texts from ASCII.

Then:

> > It would be interesting to see a stack trace


> I'm not sure about your expectation


You would have to run a libcdio based program under control of program
"gdb", set a break point at the occasion when the conversion fails,
and then start the program.
For example in lib/driver/utf8.c , function do_convert() at the line

            cdio_warn("Iconv failed: %s", strerror(errno));

set the break point by gdb command

  b utf8.c:224

and then start the program by command

  r

The line number 224 might differ in your local version of libcdio.
Check by a text editor whether this line of lib/driver/utf8.c really
bears the warning line (and correct the number as needed).

When gdb stops at that line, you would execute gdb command

  where

to get printed a "stack trace". It looks roughly like this one from
a program of mine:

  #0  0x00007ffff7776add in read () at ../sysdeps/unix/syscall-template.S:81
  #1  0x00007ffff7bbeb7d in rl_getc ()
     from /lib/x86_64-linux-gnu/libreadline.so.6
  ...
  #7  0x00000000004442b9 in Xorriso_dialog (xorriso=0x7ffff7e09010, flag=0)
      at ./libisoburn-develop/xorriso/parse_exec.c:2178
  #8  0x0000000000404683 in main (argc=14, argv=0x86f1c0)
      at ./libisoburn-develop/xorriso/xorriso_main.c:272

Main point of interest is what function names, source file names, and
line numbers are listed below a line with "in cdtext_set". This is
the stack of callers which wait for cdtext_set() do return from its job.
Please show the whole output of command "where".

(You may then remove the breakpoint by command "clear" and continue
 program execution by command "c".)

Have a nice day :)

Thomas

Thomas Schmitt <scdbackup>
Group Member
Fri 26 Apr 2019 08:43:18 PM UTC, comment #8: 

Hi,

Thanks for answers...

> independend of CD-TEXT is passing ISO-8859-1 french accent characters unchanged as UTF-8 indeed a bad idea.

ok, my suggestion was bad...

> Main suspect is that variable charset (i.e. the input encoding) is not set to "ISO-8859-1" but rather to "ASCII" or "SHIFT_JIS".

When entering

cdtext_set(cdtext_t *p_cdtext, cdtext_field_t key, const uint8_t *value, track_t track, const char *charset)
The charset value is ASCII

> It would be interesting to see a stack trace when charset_to_utf8() returns failure.
I'm not sure about your expectation (english is not my first language, and I'm not really skilled in C/C++ ).
I run 'strace cd-info' with the cd media in tray. output is the attached file named starmania_strace_cd-info.txt

> Currently i fail to find a libcdio example or test program by which i could load toc-file-starmania.toc and run into the problem.
On my side I failed to make an image of the media which is producing the same error.

> The best solution would be to identify the offending input character, replace its byte(s) by those of translatable characters, and then try conversion again.
I failed to understand because of my poor english.
Are you proposing to replace characters with a bad encoding with the correct one ?
The list can be long.

problematic value list (for that cd/media):
in_toc:\347 expected:ç matching_character_without_accent:c track6
\350 è e track7
\340 à a track11
\351 é e track18
\352 ê e track19
These are also some of the most used accentuated characters from french language (but there is also ù ô...)

On my side, "cdtext from that media is buggy and cannot be read" is quite an acceptable response.
But maybe, it can be interesting to help the user to understand where is the error or add information about at least the used encoding on media.

If you want me to run some additional test with that media, I can do it.

Regards,

Serge Pouliquen <sp31415>
Thu 25 Apr 2019 06:15:27 PM UTC, comment #7: 

Hi,

independend of CD-TEXT is passing ISO-8859-1 french accent characters
unchanged as UTF-8 indeed a bad idea.

In the .toc file i see

>     TITLE "Le R\352ve de Stella Spotlight"


This is e-circumflex in
  https://en.wikipedia.org/wiki/ISO/IEC_8859
decimal 232, hex 0xe8.
According to
  https://en.wikipedia.org/wiki/UTF-8
this byte value announces an UTF-8 code of 3 bytes. An UTF-8 interpreter
will be disappointed to see in the next byte a number smaller than 128.

The best solution would be to identify the offending input character,
replace its byte(s) by those of translatable characters, and then try
conversion again.

----------------------------------------------------------------------

The question is why the ISO-8859-1 characters do not convert to UTF-8.
Main suspect is that variable charset (i.e. the input encoding) is not
set to "ISO-8859-1" but rather to "ASCII" or "SHIFT_JIS".

It would be interesting to see a stack trace when charset_to_utf8()
returns failure.

Currently i fail to find a libcdio example or test program by which
i could load toc-file-starmania.toc and run into the problem.


Have a nice day :)

Thomas

Thomas Schmitt <scdbackup>
Group Member
Thu 25 Apr 2019 05:28:30 PM UTC, comment #6: 

I don't have an opinion and defer to those of @scdbackup @vext01 @gmerlin or others who have worked more on CD-TEXT

Rocky Bernstein <rocky>
Group administrator
Thu 25 Apr 2019 01:35:47 PM UTC, comment #5: 

Sorry, I probably forgot to add a comment.
(I don't really remember)

I made a run to the branch (serge-cdtext), it is unfixed (to me)

Your code is testing utf8 conversion, and not using any string in case of error.
My proposition was using the unconverted string in case of error.
Your commit and the previous are producing the same output (with my cd) : no output

the proposed commit :
http://git.savannah.gnu.org/cgit/libcdio.git/commit/?h=serge-cdtext&id=75c55514e6c7e621874762e14c6d40d648ecf87c

Is the test case blocking ?
because I don't believe I will be able to make a test case (not enough skill in C language)

> In case of error, the read value from cd wil be used without any convertion. In my case, only one character was useless instead of the whole line to be absent.

Did you find it was a bad idea to use the string without utf8 convertion ?

Did you
Serge

Serge Pouliquen <sp31415>
Sat 13 Apr 2019 07:40:48 AM UTC, comment #4: 

No comment on this, so assuming this has been fixed.

Rocky Bernstein <rocky>
Group administrator
Fri 18 May 2018 09:55:42 AM UTC, comment #3: 

This patch has been applied to the serge-cdtext branch.

Some small logic improvements have been applied.

I haven't tested it. In fact, it would be great if you could add a test case for it.

Your patch pasted as text didn't apply automatically, so please review the change.

In the future please don't cut and paste patch but instead either upload the patch, use use git-format-patch or fork the code and point to that. Thanks

Rocky Bernstein <rocky>
Group administrator
Thu 17 May 2018 01:10:26 PM UTC, comment #2: 

Many thanks!

When I get a chance, I will make a git branch
with these changes so that others can comment on them. (I have not looked in detail at it yet.)

Unless there are objections, that branch will be merged in.

Rocky Bernstein <rocky>
Group administrator
Thu 17 May 2018 01:09:03 PM UTC, comment #1: 

Many thanks!

When I get a chance, I will make a git branch
with these changes so that others can comment on them. (I have not looked in detail at it yet.)

Unless there are objections, that branch will be merged in.

Rocky Bernstein <rocky>
Group administrator
Thu 17 May 2018 12:40:28 PM UTC, original submission:  

Hi,

my cd text is containing bad characters and convertion to utf8 is failing

I attached cd-toc from cdrdao

I propose to check error returned during utf8 convertion.
In case of error, the read value from cd wil be used without any convertion. In my case, only one character was useless instead of the whole line to be absent.
I don't know if it's a good idea to add that fallback mode...


+++ b/lib/driver/cdtext.c
@@ -501,9 +503,14 @@ cdtext_set(cdtext_t *p_cdtext, cdtext_field_t key, const uint8_t *value,
   /* recode to UTF-8 */
   if (NULL != charset) {
     cdio_utf8_t *utf8_str = NULL;
-    cdio_charset_to_utf8((const char*) value, strlen((const char*)value),
+    bool retval;
+    retval = cdio_charset_to_utf8((const char*) value, strlen((const char*)value),
                         &utf8_str, charset);
-    p_cdtext->block[p_cdtext->block_i].track[track].field[key] = (char *)utf8_str;
+    if (1 == retval) {
+      p_cdtext->block[p_cdtext->block_i].track[track].field[key] = (char *)utf8_str;
+    } else {
+      p_cdtext->block[p_cdtext->block_i].track[track].field[key] = strdup((const char *)value);
+    }
   } else
     p_cdtext->block[p_cdtext->block_i].track[track].field[key] = strdup((const char *)value);
 }

Regards,

Serge Pouliquen <sp31415>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

Attached Files
file #46851:  bulle_bob_plage_cd-info.txt added by sp31415 (2KiB - text/plain - media bulle_bob_plage)
file #46852:  bulle_bob_plage_cdtext.dat added by sp31415 (580B - application/octet-stream - media bulle_bob_plage)
file #46853:  bulle_bob_plage_cdrskin_output.txt added by sp31415 (5KiB - text/plain - media bulle_bob_plage)
file #46836:  cdrskin_output.txt added by sp31415 (8KiB - text/plain - cdrskin (output+dat))
file #46837:  cdtext.dat added by sp31415 (1KiB - application/octet-stream - cdrskin (output+dat))
file #46835:  starmania_strace_cd-info.txt added by sp31415 (25KiB - text/plain - strace cd-info)
file #44171:  toc-file-starmania.toc added by sp31415 (5KiB - application/x-cdrdao-toc)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by scdbackup (Posted a comment)
  • -email is unavailable- added by rocky (Posted a comment)
  • -email is unavailable- added by sp31415 (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only logged-in users can vote.

     

    Follow 12 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2019-04-28 sp31415 Attached File- Added bulle_bob_plage_cd-info.txt, #46851
        Attached File- Added bulle_bob_plage_cdtext.dat, #46852
        Attached File- Added bulle_bob_plage_cdrskin_output.txt, #46853
    2019-04-27 sp31415 Attached File- Added cdrskin_output.txt, #46836
        Attached File- Added cdtext.dat, #46837
    2019-04-26 sp31415 Attached File- Added starmania_strace_cd-info.txt, #46835
    2019-04-25 rocky Assigned torocky scdbackup
    2019-04-13 rocky Open/ClosedOpen Closed
    2019-04-13 rocky StatusIn Progress Fixed
    2018-05-18 rocky StatusNone In Progress
    2018-05-17 rocky Assigned toNone rocky
    2018-05-17 sp31415 Attached File- Added toc-file-starmania.toc, #44171

    Back to the top

    Powered by Savane 3.13-f8d8.
    Corresponding source code