bugCompact Disc Input and Control Library - Bugs: bug #53928, cdtext not displayed in case of...

 
 

bug #53928: cdtext not displayed in case of CDTEXT_LANGUAGE_UNKNOWN

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

Add a New Comment Rich Markup
   

Jump to the original submission

Thu 25 Apr 2019 12:37:24 PM UTC, comment #13: 

I made a test with the new version 2.1.0. It looks fixed with my cd media.
Thanks a lot. Serge

Serge Pouliquen <sp31415>
Mon 21 May 2018 10:54:46 AM UTC, comment #12: 

yes

CD Analysis Report

Language 0 'Unknown':
CD-TEXT for Disc:
TITLE: CD trans 2013
CD-TEXT for Track  1:
TITLE: Oum shatt-Power to the Women of the Morning Shift
CD-TEXT for Track  2:
TITLE: Moodoid-Je suis la montagne
CD-TEXT for Track  3:
TITLE: Michael Paskalev-Jive Babe
CD-TEXT for Track  4:
TITLE: chic gamine-DaysAndDays
CD-TEXT for Track  5:
TITLE: Escort-Barbarians
CD-TEXT for Track  6:
TITLE: Molotv jukebox-i need it
CD-TEXT for Track  7:
TITLE: Har Mar Superstar-Prisoner
CD-TEXT for Track  8:
TITLE: Ibibio sound machine-lets dance AGM
CD-TEXT for Track  9:
TITLE: The Midnight Beast-Quirky
CD-TEXT for Track 10:
TITLE: Rhume-Biarritz
CD-TEXT for Track 11:
TITLE: Le vasco-It's a War Game sirene
CD-TEXT for Track 12:
TITLE: Melt yourself down-We Are Enough
CD-TEXT for Track 13:
TITLE: Public Service Broadcasting-Spitfire
CD-TEXT for Track 14:
TITLE: Dead-Human Light agm
CD-TEXT for Track 15:
TITLE: Golden Teacher - Dante & Pilgrim (JD Twitch edit) agm
CD-TEXT for Track 16:
TITLE: Boston bun-Flasher
CD-TEXT for Track 17:
TITLE: Gang do Eletro-Velocidade Do Eletro
CD-TEXT for Track 18:
TITLE: La yegros-viene de mi
CD-TEXT for Track 19:
TITLE: Dakhabrakha-Specialy For You

Serge Pouliquen <sp31415>
Mon 21 May 2018 10:53:09 AM UTC, comment #11: 

Does it say:

Language 0 'Unknown':
...

(That's what i would expect from reading the code.)

Thomas Schmitt <scdbackup>
Group Member
Mon 21 May 2018 10:42:37 AM UTC, comment #10: 

Hi,

>I could now need instructions how to help preparing a test version for
>Serge Pouliquen and his CD.
>I Cc: Serge in the hope that he can make use of the diff below directly.


>My git knowledge is restricted to very few gestures. If I shall clone
>again with a better git command, just tell me.


I failed to use the provided diff directly.
As there is not too many lines, I copy pasted them (hoping no mistake).

I tested with cd, It 's functional (and code looks like more comprehensive to me).
Thanks a lot

Regards
Serge

Serge Pouliquen <sp31415>
Sun 20 May 2018 06:05:02 PM UTC, comment #9: 

A first hopefully complete proposal can be seen at
http://lists.gnu.org/archive/html/libcdio-devel/2018-05/msg00005.html

Thomas Schmitt <scdbackup>
Group Member
Sat 19 May 2018 08:03:50 AM UTC, comment #8: 

Hi,

there is another API flaw, which is more ugly than harmful:

The API does not say how many language codes are in the array which
gets returned by cdtext_list_languages().
The examples assume 8, which is the maximum number defined by CD-TEXT
specs.
The implementation in lib/driver/cdtext.c uses CDTEXT_NUM_BLOCKS_MAX
which is defined in lib/driver/cdtext_private.h as 8.

This is only harmless because the old CD-TEXT specification will most
probably not change any more.

Exposing enum cdtext_format_enum_t with CDTEXT_NUM_BLOCKS_MAX and others
might have further consequences.
Shall i just publish "8" in the API description include/cdio/cdtext.h,
rather than making cdtext_format_enum_t public ?

Have a nice day :)

Thomas

Thomas Schmitt <scdbackup>
Group Member
Sat 19 May 2018 07:05:39 AM UTC, comment #7: 

Hi,

is there some kind of todo list for introducing new API functions ?
(I dimly remember a version file where the public symbols have to be
 declared. Was it  lib/driver/libcdio.sym  ?)

Have a nice day :)

Thomas

Thomas Schmitt <scdbackup>
Group Member
Fri 18 May 2018 09:33:39 PM UTC, comment #6: 

Hi,

the proposed introduction of CDTEXT_LANGUAGE_BLOCK_UNUSED as indication
of a non-existent language block would show up in the API.

The new value will be listed in the public include file

  include/cdio/cdtext.h

and the gesture

  if ( CDTEXT_LANGUAGE_UNKNOWN != languages[i]

which Serge found in src/cd-info.c is also to see in

  example/C++/OO/cdtext.cpp
  example/cdtext.c
  example/cdtext-raw.c

(I did not find CDTEXT_LANGUAGE_UNKNOWN in any documentation text.
Is there any outside the git repo ?)

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

So how does libcdio deal with such an API design flaw ?

Do we have to keep

  cdtext_list_languages()

as deprecated function which returns CDTEXT_LANGUAGE_UNKNOWN for all three
situations ?
And then create a new

  cdtext_list_languages_2()

which returns

  CDTEXT_LANGUAGE_UNKNOWN for valid blocks with language code 0x00
  ... other valid codes for valid blocks with language code <= 0x7F ...
  CDTEXT_LANGUAGE_INVALID for valid blocks with invalid language code > 0x7F
  CDTEXT_LANGUAGE_BLOCK_UNUSED for invalid blocks which were not read.

Have a nice day :)

Thomas

Thomas Schmitt <scdbackup>
Group Member
Fri 18 May 2018 08:41:07 PM UTC, comment #5: 

My comment: forcing one iteration as cdtext is not NULL
was based only on one part of code (same function, 20 lines above):

>  if(NULL == p_cdtext) {
>    printf("No CD-TEXT on Disc.\n");
>    return;
>  }


>So maybe he already has verified that a block was surely read.

no

Serge Pouliquen <sp31415>
Fri 18 May 2018 02:04:51 PM UTC, comment #4: 

Hi,

Rocky Bernstein wrote:

> would you work up revised code.


I can give it a try, but i lack of testing experience with libcdio.
Also: We should wait for the opinion of the person who implemented that
code part. I understand it was not you. Is it by Leon Merten Lohse ?

As for the patch proposal by Serge Pouliquen:

It should be ok if at least one language block was read before that code
is reached. We would have to examine whether this is guaranteed currently.
Serge wrote as comment: "forcing one iteration as cdtext is not NULL".
So maybe he already has verified that a block was surely read.

More than one block of real CDTEXT_LANGUAGE_UNKNOWN would not be plausible.
But of course there could be several blocks with invald codes > 0x7F.
They all would be registered as CDTEXT_LANGUAGE_UNKNOWN.
So as long as we do not distinguish the three cases (real, invalid,
non-existent) it is best to accept CDTEXT_LANGUAGE_UNKNOWN only with
the first block.

Have a nice day :)

Thomas

Thomas Schmitt <scdbackup>
Group Member
Fri 18 May 2018 01:43:57 PM UTC, comment #3: 

Thanks for the information, Thomas. Since you clearly know the most about this, would you work up revised code.

Adding another enumeration with the names you propose is fine.

Again, thanks.

Rocky Bernstein <rocky>
Group administrator
Fri 18 May 2018 01:03:11 PM UTC, comment #2: 

Hi,

i wrote a reply to Rocky's request for comments at libcdio-devel:
  http://lists.gnu.org/archive/html/libcdio-devel/2018-05/msg00001.html

In summary:
CDTEXT_LANGUAGE_UNKNOWN = 0x00 is a valid language code for CD-TEXT.
libcdio seems to misuse it for two internal purposes in its block
array of 8 members:
- indicate that the CD-TEXT block has an invalid language code >= 0x7F.
- indicate that the CD-TEXT block does not exist in the CD-TEXT data.

The former blocks should be interpreted with caution.
The latter should not be interpreted at all.

Real CDTEXT_LANGUAGE_UNKNOWN should be treated as any other language.

Have a nice day :)

Thomas

Thomas Schmitt <scdbackup>
Group Member
Fri 18 May 2018 09:54:15 AM UTC, comment #1: 

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

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 12:29:51 PM UTC, original submission:  

Hi,

I tried to get cd text data from one of my cds.

v0.83 cd-info can display these information
v2.0.0 cd-info can't display information
v2.0.0 example/cdtext can display information

According to cdrdao toc has only data for language 0 (as indicated in attached file)
In code, that lang is skipped and used in order to initialise variables.

I read code, and found that in lib/driver/cdtext.c

> avail[i] = CDTEXT_LANGUAGE_UNKNOWN

I don't understand why it is inited with that value.
maybe there are some cd with multiple language

I don't think I really understood the meaning of unknown language, but maybe it could be interesting to display it data.

I tested :
diff --git a/src/cd-info.c b/src/cd-info.c
index 002f0f9..41dd7ff 100644
--- a/src/cd-info.c
+++ b/src/cd-info.c
@@ -450,10 +450,24 @@ print_cdtext_info(CdIo_t *p_cdio, track_t i_tracks, track_t i_first_track) {
     return;
   }
 
   languages = cdtext_list_languages(p_cdtext);
   for(i=0; i<8; i++)
-    if ( CDTEXT_LANGUAGE_UNKNOWN != languages[i]
-         && cdtext_select_language(p_cdtext, languages[i]))
+    if ( ( CDTEXT_LANGUAGE_UNKNOWN != languages[i]
+         && cdtext_select_language(p_cdtext, languages[i]) )
+       || (i == 0) ) forcing one iteration as cdtext is not NULL
     {
       printf("\nLanguage %d '%s':\n", i, cdtext_lang2str(languages[i]));
 

It is not really clean code. It's more quick and dirty because i failed to really understand CDTEXT_LANGUAGE_UNKNOWN
I don't know code is considering unknown language as an invalid language. Preferring an other one is good, but I found a cd without any other language (maybe not conformed to cd specification).

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 #44170:  toc-file-trans2013.toc added by sp31415 (4KiB - 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 (Updated the item)
  • -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 6 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2019-04-13 rocky Open/ClosedOpen Closed
    2019-04-13 rocky StatusIn Progress Fixed
    2018-05-18 rocky Assigned torocky scdbackup
    2018-05-18 rocky StatusNone In Progress
    2018-05-17 rocky Assigned toNone rocky
    2018-05-17 sp31415 Attached File- Added toc-file-trans2013.toc, #44170

    Back to the top

    Powered by Savane 3.13-4448.
    Corresponding source code