Skip to content
Snippets Groups Projects

[3.0] misc CDDA/cdrom backports

Open Steve Lhomme requested to merge robUx4/vlc:30-cdda-fixes into 3.0.x
1 unresolved thread

Better CD-Text support, use better Windows API (XP compatible), fix CD-Extra/Enhanced CD/Hybrid CD support.

Merge request reports

Members who can merge are allowed to add commits.

Merge request pipeline #137546 passed

Merge request pipeline passed for 2b89c58d

Approval is optional
Merge blocked: 3 checks failed
Unresolved discussions must be resolved.
Merge conflicts must be resolved.
Merge request must be rebased, because a fast-forward merge is not possible.

Merge details

  • The source branch is 1734 commits behind the target branch.
  • 40 commits will be added to 3.0.x.
  • Source branch will be deleted.

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • changed milestone to %3.0.x maintenance

  • Steve Lhomme added 40 commits

    added 40 commits

    • c4181817 - access: vcd: cdrom: extensions are case-insensitive
    • 96634d26 - access: cdrom: use defines for cdtext
    • bf268f45 - access: cdrom: don't merge cdtext across blocks
    • e6d5243f - access: cdrom: parse cdtext blockinfo
    • 974888ce - access: cdrom: support cdtext non ascii charsets
    • 5a7a5fc1 - access: cdrom: implement cdtext repeats
    • 650b8178 - access: cdda: use track cdtext over album cdtext data
    • 0224d3a2 - access: vcd: fix compilation
    • 2f64c15a - cdda: fix format-truncation warning
    • 6bf6c230 - cdda: cddb: compute the track offset once
    • 1ff46c92 - cdda: cddb: compute the total track frames once
    • b374e0c1 - cdda: cddb: don't multiply the size/length by 1000000
    • fd8561e0 - cdda: cddb: compute the length in seconds once
    • 8546e7f5 - cdrom: remove always true parameter
    • b7ce46f0 - cdrom: reindent after previous patch
    • bba6720f - cdrom: warn when the lead out of an audio CD is not found
    • 761c2548 - cdrom: use CDROM_READ_TOC_EX_FORMAT_TOC instead of deprecated IOCTL_CDROM_READ_TOC
    • 85f26776 - access: only compile and use DiscProbeMacOSPermission() on Apple platforms
    • f8e369aa - cdrom: remove unused CDDA_DATA_START
    • 30dfe9d7 - cdda: use a define for the invalid/default sector LBA values
    • 1a8682c8 - cdrom: define the MinSecFrame to LBA based on duration of CDDA frames
    • 46321f67 - cdda: document CDDA_BLOCKS_ONCE
    • aedc4935 - cdda: read/set the time/length using vlc_tick API and CD_ROM_CDDA_FRAMES
    • 87d1788b - cdrom: don't eat the next line after we read the cue file mode
    • 9979e59f - cdrom: read the track type (audio or data) from the cuesheet
    • 6f9857e1 - cdda: fix shift for possible Mode 2 track at the start
    • 885b05b3 - cdrom: add known subcode values
    • 2684bfbd - cdda: report the total usable track in the metadata
    • bec77237 - cdrom: simplify the CDROM_READ_TOC_EX structure init
    • 4125e12a - cdrom: don't try to read CD-Text information if we only have 4 bytes
    • c88c1f5d - cdrom: remove always false CD-Text test
    • 4865c2a6 - cdrom: discard empty CD-Text fields
    • 2e4220f6 - cdrom: add the CD-Text album artist field for each track
    • 86febc2a - cdrom: use an enum for CD-Text pack types
    • 13131a92 - cdrom: use the CD-Text EAN/ISRC code as extra metadata
    • bfbc019a - cdrom: add support for CD-Text extra metadata
    • 771cfd35 - cdda: Fix handling of GIO CDDA URIs for the whole disk
    • 4ae0c66a - cdda: favor track metadata over album metadata from CD-Text
    • 91f5a2fe - cdda: forward the CD-Text album artist to the input item
    • 2b89c58d - cdda: forward extra metadata from CD-Text to the input item

    Compare with previous version

  • Author Developer

    Added some older fixes so the cdrom/cdda 4.0 and 3.0 files are (almost) the same.

    • Maybe this is a bit much?

    • Author Developer

      You want to split in smaller MRs ?

    • I'm afraid of breaking so much, in a stable release. But maybe I'm too conservative here.

    • Author Developer

      IMO the CD support is not in a good state in 3.0. It doesn't even show the proper durations.

    • Author Developer

      For the record, here are the differences between 4.0 and 3.0 after applying this MR

      • mtime_t vs vlc_tick_t
      • VLC_TICK_0 shift of dates
      • module hint set differently
      • no extra param in module parameters
      • single open differentiate between track 0 (disc -> access module) and other tracks (demux module)
      • no more memstream open checks
      • module sys are typedefs
      ---
       modules/access/cdda.c    | 148 +++++++++++++++++----------------------
       modules/access/vcd/vcd.c |  14 ++--
       2 files changed, 74 insertions(+), 88 deletions(-)
      
      diff --git a/modules/access/cdda.c b/modules/access/cdda.c
      index be5089aa6b..857bb1df6c 100644
      --- a/modules/access/cdda.c
      +++ b/modules/access/cdda.c
      @@ -45,7 +45,7 @@
       #include <vlc_common.h>
       #include <vlc_demux.h>
       #include <vlc_plugin.h>
      -#include <vlc_input.h>
      +#include <vlc_input_item.h>
       #include <vlc_access.h>
       #include <vlc_meta.h>
       #include <vlc_charset.h> /* ToLocaleDup */
      @@ -145,7 +145,7 @@ static vcddev_t *DiscOpen(vlc_object_t *obj, const char *location,
       /* how many blocks Demux() will read in each iteration */
       #define CDDA_BLOCKS_ONCE 20 // ~267 ms
       
      -struct demux_sys_t
      +typedef struct
       {
           vcddev_t    *vcddev;                            /* vcd device descriptor */
           es_out_id_t *es;
      @@ -154,7 +154,7 @@ struct demux_sys_t
           unsigned start; /**< Track first sector */
           unsigned length; /**< Track total sectors */
           unsigned position; /**< Current offset within track sectors */
      -};
      +} demux_sys_t;
       
       static int Demux(demux_t *demux)
       {
      @@ -186,11 +186,11 @@ static int Demux(demux_t *demux)
           sys->position += count;
       
           block->i_nb_samples = block->i_buffer / 4;
      -    block->i_dts = block->i_pts = VLC_TS_0 + date_Get(&sys->pts);
      +    block->i_dts = block->i_pts = date_Get(&sys->pts);
           date_Increment(&sys->pts, block->i_nb_samples);
       
           es_out_Send(demux->out, sys->es, block);
      -    es_out_SetPCR(demux->out, VLC_TS_0 + date_Get(&sys->pts));
      +    es_out_SetPCR(demux->out, date_Get(&sys->pts));
           return VLC_DEMUXER_SUCCESS;
       }
       
      @@ -206,8 +206,8 @@ static int DemuxControl(demux_t *demux, int query, va_list args)
                   *va_arg(args, bool*) = true;
                   break;
               case DEMUX_GET_PTS_DELAY:
      -            *va_arg(args, int64_t *) =
      -                INT64_C(1000) * var_InheritInteger(demux, "disc-caching");
      +            *va_arg(args, vlc_tick_t *) =
      +                VLC_TICK_FROM_MS( var_InheritInteger(demux, "disc-caching") );
                   break;
       
               case DEMUX_SET_PAUSE_STATE:
      @@ -223,13 +223,13 @@ static int DemuxControl(demux_t *demux, int query, va_list args)
                   break;
       
               case DEMUX_GET_LENGTH:
      -            *va_arg(args, mtime_t *) = CLOCK_FREQ * sys->length / CD_ROM_CDDA_FRAMES;
      +            *va_arg(args, vlc_tick_t *) = vlc_tick_from_samples(sys->length, CD_ROM_CDDA_FRAMES);
                   break;
               case DEMUX_GET_TIME:
      -            *va_arg(args, mtime_t *) = CLOCK_FREQ * sys->position / CD_ROM_CDDA_FRAMES;
      +            *va_arg(args, vlc_tick_t *) = vlc_tick_from_samples(sys->position, CD_ROM_CDDA_FRAMES);
                   break;
               case DEMUX_SET_TIME:
      -            sys->position = (va_arg(args, mtime_t) * CD_ROM_CDDA_FRAMES) / CLOCK_FREQ;
      +            sys->position = samples_from_vlc_tick(va_arg(args, vlc_tick_t), CD_ROM_CDDA_FRAMES);
                   break;
       
               default:
      @@ -299,17 +299,11 @@ static int TOC_GetAudioRange(vcddev_toc_t *p_toc,
           return (i_last >= i_first) ? i_last - i_first + 1 : 0;
       }
       
      -static int DemuxOpen(vlc_object_t *obj)
      +static int DemuxOpen(vlc_object_t *obj, vcddev_t *dev, unsigned track)
       {
           demux_t *demux = (demux_t *)obj;
      -    unsigned track;
       
      -    vcddev_t *dev = DiscOpen(obj, demux->psz_location, demux->psz_file,
      -                             &track);
      -    if (dev == NULL)
      -        return VLC_EGENERIC;
      -
      -    if (track == 0 /* Whole disc -> use access plugin */)
      +    if (demux->out == NULL)
               goto error;
       
           demux_sys_t *sys = vlc_obj_malloc(obj, sizeof (*sys));
      @@ -357,7 +351,7 @@ static int DemuxOpen(vlc_object_t *obj)
           sys->es = es_out_Add(demux->out, &fmt);
       
           date_Init(&sys->pts, 44100, 1);
      -    date_Set(&sys->pts, 0);
      +    date_Set(&sys->pts, VLC_TICK_0);
       
           sys->position = 0;
           demux->pf_demux = Demux;
      @@ -369,18 +363,10 @@ error:
           return VLC_EGENERIC;
       }
       
      -static void DemuxClose(vlc_object_t *obj)
      -{
      -    demux_t *demux = (demux_t *)obj;
      -    demux_sys_t *sys = demux->p_sys;
      -
      -    ioctl_Close(obj, sys->vcddev);
      -}
      -
       /*****************************************************************************
        * Access: local prototypes
        *****************************************************************************/
      -struct access_sys_t
      +typedef struct
       {
           vcddev_t    *vcddev;                            /* vcd device descriptor */
           vcddev_toc_t *p_toc;                            /* Tracks TOC */
      @@ -393,7 +379,7 @@ struct access_sys_t
           cddb_disc_t *cddb;
       #endif
           musicbrainz_recording_t *mbrecord;
      -};
      +} access_sys_t;
       
       static inline int LBAPregap( int i_sector )
       {
      @@ -503,13 +489,10 @@ static musicbrainz_recording_t * GetMusicbrainzInfo( vlc_object_t *obj,
           else /* Fuzzy lookup using TOC */
           {
               struct vlc_memstream ms;
      -        if( vlc_memstream_open(&ms) )
      -        {
      -            free( psz_mbserver );
      -            return NULL;
      -        }
       
      +        vlc_memstream_open(&ms);
               vlc_memstream_printf(&ms, "toc=%u+%u", i_first, i_last );
      +
               /* LEAD OUT sector info
                * https://github.com/metabrainz/libdiscid/blob/e46249415eb6d657ecc63667b03d670a4347712f/src/toc.c#L90 */
               int i_last_track_end;
      @@ -520,18 +503,11 @@ static musicbrainz_recording_t * GetMusicbrainzInfo( vlc_object_t *obj,
               vlc_memstream_printf(&ms, "+%u", i_last_track_end );
               for( int i = 0; i<i_total; i++ ) /* skipped LEAD OUT, audio only */
                   vlc_memstream_printf(&ms, "+%u", LBAPregap(p_toc->p_sectors[i].i_lba) );
      -        if( vlc_memstream_flush(&ms) )
      -        {
      -            if( vlc_memstream_close(&ms) )
      -                free( ms.ptr );
      -            free( psz_mbserver );
      -            return NULL;
      -        }
      -
      -        recording = musicbrainz_lookup_recording_by_toc( &cfg, ms.ptr );
      -
               if( vlc_memstream_close(&ms) == 0 )
      +        {
      +            recording = musicbrainz_lookup_recording_by_toc( &cfg, ms.ptr );
                   free( ms.ptr );
      +        }
           }
       
           free( psz_mbserver );
      @@ -732,8 +708,8 @@ static int ReadDir(stream_t *access, input_item_node_t *node)
                  p_toc->i_last_track > sys->i_cdda_last)
                   i_last_sector -= CD_ROM_XA_INTERVAL;
       
      -        const mtime_t duration =
      -            CLOCK_FREQ * (i_last_sector - i_first_sector) / CD_ROM_CDDA_FRAMES;
      +        const vlc_tick_t duration =
      +            vlc_tick_from_samples(i_last_sector - i_first_sector, CD_ROM_CDDA_FRAMES);
       
               input_item_t *item = input_item_NewDisc(access->psz_url,
                                                       (name != NULL) ? name :
      @@ -909,22 +885,10 @@ static int AccessControl(stream_t *access, int query, va_list args)
           return access_vaDirectoryControlHelper(access, query, args);
       }
       
      -static int AccessOpen(vlc_object_t *obj)
      +static int AccessOpen(vlc_object_t *obj, vcddev_t *dev)
       {
           stream_t *access = (stream_t *)obj;
      -    unsigned track;
      -
      -    vcddev_t *dev = DiscOpen(obj, access->psz_location, access->psz_filepath,
      -                             &track);
      -    if (dev == NULL)
      -        return VLC_EGENERIC;
      -
      -    if (track != 0 /* Only whole discs here */)
      -    {
      -        ioctl_Close(obj, dev);
      -        return VLC_EGENERIC;
      -    }
      -
      +    /* Only whole discs here */
           access_sys_t *sys = vlc_obj_malloc(obj, sizeof (*sys));
           if (unlikely(sys == NULL))
           {
      @@ -984,11 +948,8 @@ error:
           return VLC_EGENERIC;
       }
       
      -static void AccessClose(vlc_object_t *obj)
      +static void AccessClose(access_sys_t *sys)
       {
      -    stream_t *access = (stream_t *)obj;
      -    access_sys_t *sys = access->p_sys;
      -
           for (int i = 0; i < sys->cdtextc; i++)
           {
               vlc_meta_t *meta = sys->cdtextv[i];
      @@ -1001,14 +962,40 @@ static void AccessClose(vlc_object_t *obj)
           if (sys->cddb != NULL)
               cddb_disc_destroy(sys->cddb);
       #endif
      -
      -    ioctl_Close(obj, sys->vcddev);
      -
           if(sys->mbrecord)
               musicbrainz_recording_release(sys->mbrecord);
           vcddev_toc_Free(sys->p_toc);
       }
       
      +static int Open(vlc_object_t *obj)
      +{
      +    stream_t *stream = (stream_t *)obj;
      +    unsigned track;
      +
      +    vcddev_t *dev = DiscOpen(obj, stream->psz_location, stream->psz_filepath,
      +                             &track);
      +    if (dev == NULL)
      +        return VLC_EGENERIC;
      +
      +    if (track == 0)
      +        return AccessOpen(obj, dev);
      +    else
      +        return DemuxOpen(obj, dev, track);
      +}
      +
      +static void Close(vlc_object_t *obj)
      +{
      +    stream_t *stream = (stream_t *)obj;
      +    void *sys = stream->p_sys;
      +
      +    if (stream->pf_readdir != NULL)
      +        AccessClose(sys);
      +
      +    static_assert(offsetof(demux_sys_t, vcddev) == 0, "Invalid cast");
      +    static_assert(offsetof(access_sys_t, vcddev) == 0, "Invalid cast");
      +    ioctl_Close(obj, *(vcddev_t **)sys);
      +}
      +
       /*****************************************************************************
        * Module descriptior
        *****************************************************************************/
      @@ -1030,39 +1017,36 @@ static void AccessClose(vlc_object_t *obj)
       # endif
       #endif
       
      +#define HELP_TEXT N_("Usage hint: [cdda:][device][@[track]]")
      +
       vlc_module_begin ()
           set_shortname( N_("Audio CD") )
           set_description( N_("Audio CD input") )
      -    set_capability( "access", 10 )
      +    set_help( HELP_TEXT )
      +    set_capability( "access", 0 )
           set_category( CAT_INPUT )
           set_subcategory( SUBCAT_INPUT_ACCESS )
      -    set_callbacks(AccessOpen, AccessClose)
      +    set_callbacks(Open, Close)
       
      -    add_loadfile( "cd-audio", CD_DEVICE, CDAUDIO_DEV_TEXT,
      -                  CDAUDIO_DEV_LONGTEXT, false )
      +    add_loadfile("cd-audio", CD_DEVICE, CDAUDIO_DEV_TEXT, CDAUDIO_DEV_LONGTEXT)
       
      -    add_usage_hint( N_("[cdda:][device][@[track]]") )
      -    add_integer( "cdda-track", 0 , NULL, NULL, true )
      +    add_integer( "cdda-track", 0 , NULL, NULL )
               change_volatile ()
      -    add_integer( "cdda-first-sector", INVALID_SECTOR, NULL, NULL, true )
      +    add_integer( "cdda-first-sector", INVALID_SECTOR, NULL, NULL )
               change_volatile ()
      -    add_integer( "cdda-last-sector", INVALID_SECTOR, NULL, NULL, true )
      +    add_integer( "cdda-last-sector", INVALID_SECTOR, NULL, NULL )
               change_volatile ()
       
           add_string( "musicbrainz-server", MUSICBRAINZ_DEFAULT_SERVER,
                       N_( "Musicbrainz Server" ),
      -                N_( "Address of the musicbrainz server to use." ), true )
      +                N_( "Address of the musicbrainz server to use." ) )
       #ifdef HAVE_LIBCDDB
           add_string( "cddb-server", "freedb.videolan.org", N_( "CDDB Server" ),
      -            N_( "Address of the CDDB server to use." ), true )
      +            N_( "Address of the CDDB server to use." ) )
           add_integer( "cddb-port", 80, N_( "CDDB port" ),
      -            N_( "CDDB Server port to use." ), true )
      +            N_( "CDDB Server port to use." ) )
               change_integer_range( 1, 65535 )
       #endif
       
           add_shortcut( "cdda", "cddasimple" )
      -
      -    add_submodule()
      -    set_capability( "access_demux", 10 )
      -    set_callbacks(DemuxOpen, DemuxClose)
       vlc_module_end ()
      diff --git a/modules/access/vcd/vcd.c b/modules/access/vcd/vcd.c
      index 2fad4b69e7..3bf88f48c7 100644
      --- a/modules/access/vcd/vcd.c
      +++ b/modules/access/vcd/vcd.c
      @@ -43,15 +43,17 @@
       static int  Open ( vlc_object_t * );
       static void Close( vlc_object_t * );
       
      +#define HELP_TEXT N_("Usage hint: [vcd:][device][#[title][,[chapter]]]")
      +
       vlc_module_begin ()
           set_shortname( N_("VCD"))
           set_description( N_("VCD input") )
      -    set_capability( "access", 60 )
      +    set_help( HELP_TEXT )
      +    set_capability( "access", 0 )
           set_callbacks( Open, Close )
           set_category( CAT_INPUT )
           set_subcategory( SUBCAT_INPUT_ACCESS )
       
      -    add_usage_hint( N_("[vcd:][device][#[title][,[chapter]]]") )
           add_shortcut( "vcd", "svcd" )
       vlc_module_end ()
       
      @@ -63,7 +65,7 @@ vlc_module_end ()
       #define VCD_BLOCKS_ONCE 20
       #define VCD_DATA_ONCE   (VCD_BLOCKS_ONCE * VCD_DATA_SIZE)
       
      -struct access_sys_t
      +typedef struct
       {
           vcddev_t    *vcddev;                            /* vcd device descriptor */
           uint64_t    offset;
      @@ -79,7 +81,7 @@ struct access_sys_t
           int         i_current_title;
           unsigned    i_current_seekpoint;
           int         i_sector;                                  /* Current Sector */
      -};
      +} access_sys_t;
       
       static block_t *Block( stream_t *, bool * );
       static int      Seek( stream_t *, uint64_t );
      @@ -258,8 +260,8 @@ static int Control( stream_t *p_access, int i_query, va_list args )
       
               /* */
               case STREAM_GET_PTS_DELAY:
      -            *va_arg( args, int64_t * ) = INT64_C(1000)
      -                * var_InheritInteger(p_access, "disc-caching");
      +            *va_arg( args, vlc_tick_t * ) = VLC_TICK_FROM_MS(
      +                var_InheritInteger(p_access, "disc-caching") );
                   break;
       
               /* */
      -- 
      2.27.0.windows.1
    • Please register or sign in to reply
  • Jean-Baptiste Kempf resolved all threads

    resolved all threads

  • checked with Shift-JIS CD-TEXT

  • Ping? What should we do with this MR?

  • why do we need to change adv prefs to regular ones ?

  • added MRStatus::Stale label and removed MRStatus::InReview label

  • added MRStatus::InReview label and removed MRStatus::Stale label

  • added MRStatus::Stale label and removed MRStatus::InReview label

Please register or sign in to reply
Loading