[vlc-devel] [PATCH] dvdnav: add Demux submodule

Thomas Guillem thomas at gllm.fr
Mon Mar 23 19:48:35 CET 2015



On Mon, Mar 23, 2015, at 12:10, Rémi Denis-Courmont wrote:
> Le 2015-03-19 11:28, Thomas Guillem a écrit :
> > It allows to use dvdnav via a VLC access.
> >
> > When using a VLC access, dvdnav readahead flag is disabled in order 
> > to read
> > small chunk of data from pf_demux (that is 2kB).
> > ---
> >  modules/access/dvdnav.c | 224
> > +++++++++++++++++++++++++++++++++++-------------
> >  1 file changed, 164 insertions(+), 60 deletions(-)
> >
> > diff --git a/modules/access/dvdnav.c b/modules/access/dvdnav.c
> > index ad1240a..a0a7b24 100644
> > --- a/modules/access/dvdnav.c
> > +++ b/modules/access/dvdnav.c
> > @@ -76,7 +76,8 @@
> >
> >  #define LANGUAGE_DEFAULT ("en")
> >
> > -static int  Open ( vlc_object_t * );
> > +static int  AccessDemuxOpen ( vlc_object_t * );
> > +static int  DemuxOpen ( vlc_object_t * );
> >  static void Close( vlc_object_t * );
> >
> >  vlc_module_begin ()
> > @@ -90,14 +91,19 @@ vlc_module_begin ()
> >          MENU_TEXT, MENU_LONGTEXT, false )
> >      set_capability( "access_demux", 5 )
> >      add_shortcut( "dvd", "dvdnav", "file" )
> > -    set_callbacks( Open, Close )
> > +    set_callbacks( AccessDemuxOpen, Close )
> > +    add_submodule()
> > +        set_description( N_("DVDnav demuxer") )
> > +        set_category( CAT_INPUT )
> > +        set_subcategory( SUBCAT_INPUT_DEMUX )
> > +        set_capability( "demux", 5 )
> > +        set_callbacks( DemuxOpen, Close )
> > +        add_shortcut( "iso" )
> >  vlc_module_end ()
> >
> >  /* Shall we use libdvdnav's read ahead cache? */
> > -#ifdef __OS2__
> > -#define DVD_READ_CACHE 0
> > -#else
> > -#define DVD_READ_CACHE 1
> > +#ifndef __OS2__
> > +#define DVD_READ_CACHE
> 
> ????
> 
> >  #endif
> >
> >
> > 
> > /*****************************************************************************
> > @@ -109,6 +115,7 @@ struct demux_sys_t
> >
> >      /* */
> >      bool        b_reset_pcr;
> > +    bool        b_readahead;
> >
> >      struct
> >      {
> > @@ -160,6 +167,7 @@ static void ButtonUpdate( demux_t *, bool );
> >
> >  static void ESNew( demux_t *, int );
> >  static int ProbeDVD( const char * );
> > +static int StreamProbeDVD( stream_t * );
> >
> >  static char *DemuxGetLanguageCode( demux_t *p_demux, const char 
> > *psz_var );
> >
> > @@ -173,66 +181,43 @@ static int EventIntf( vlc_object_t *, char 
> > const *,
> >                        vlc_value_t, vlc_value_t, void * );
> >
> >
> > 
> > /*****************************************************************************
> > - * DemuxOpen:
> > + * CommonOpen:
> >
> > 
> > *****************************************************************************/
> > -static int Open( vlc_object_t *p_this )
> > +static int CommonOpen( vlc_object_t *p_this, const char *psz_file,
> > +                       void *p_dvd_stream,
> > +                       dvdnav_stream_cb *p_dvd_stream_cb )
> >  {
> >      demux_t     *p_demux = (demux_t*)p_this;
> >      demux_sys_t *p_sys;
> >      dvdnav_t    *p_dvdnav;
> >      int         i_angle;
> > -    char        *psz_file;
> >      char        *psz_code;
> > -    bool forced = false;
> > -
> > -    if( p_demux->psz_access != NULL
> > -     && !strncmp(p_demux->psz_access, "dvd", 3) )
> > -        forced = true;
> > -
> > -    if( !p_demux->psz_file || !*p_demux->psz_file )
> > -    {
> > -        /* Only when selected */
> > -        if( !forced )
> > -            return VLC_EGENERIC;
> >
> > -        psz_file = var_InheritString( p_this, "dvd" );
> > -    }
> > -    else
> > -        psz_file = strdup( p_demux->psz_file );
> > +    if( unlikely( psz_file == NULL &&
> > +        ( p_dvd_stream == NULL || p_dvd_stream_cb == NULL ) ) )
> > +        return VLC_EGENERIC;
> >
> > -#if defined( _WIN32 ) || defined( __OS2__ )
> > -    if( psz_file != NULL )
> > +    if( p_dvd_stream != NULL )
> >      {
> > -        /* Remove trailing backslash, otherwise dvdnav_open will 
> > fail */
> > -        size_t flen = strlen( psz_file );
> > -        if( flen > 0 && psz_file[flen - 1] == '\\' )
> > -            psz_file[flen - 1] = '\0';
> > +        /* Open dvdnav with stream callbacks */
> > +        if( dvdnav_open_stream( &p_dvdnav, p_dvd_stream,
> > +                                p_dvd_stream_cb ) != 
> > DVDNAV_STATUS_OK )
> > +            p_dvdnav = NULL;
> >      }
> >      else
> > -        psz_file = strdup("");
> > -#endif
> > -    if( unlikely(psz_file == NULL) )
> > -        return VLC_EGENERIC;
> > -
> > -    /* Try some simple probing to avoid going through dvdnav_open
> > too often */
> > -    if( !forced && ProbeDVD( psz_file ) != VLC_SUCCESS )
> >      {
> > -        free( psz_file );
> > -        return VLC_EGENERIC;
> > +        /* Open dvdnav */
> > +        const char *psz_path = ToLocale( psz_file );
> > +        if( dvdnav_open( &p_dvdnav, psz_path ) != DVDNAV_STATUS_OK )
> > +            p_dvdnav = NULL;
> > +        LocaleFree( psz_path );
> >      }
> >
> > -    /* Open dvdnav */
> > -    const char *psz_path = ToLocale( psz_file );
> > -    if( dvdnav_open( &p_dvdnav, psz_path ) != DVDNAV_STATUS_OK )
> > -        p_dvdnav = NULL;
> > -    LocaleFree( psz_path );
> >      if( p_dvdnav == NULL )
> >      {
> >          msg_Warn( p_demux, "cannot open DVD (%s)", psz_file);
> > -        free( psz_file );
> >          return VLC_EGENERIC;
> >      }
> > -    free( psz_file );
> >
> >      /* Fill p_demux field */
> >      DEMUX_INIT_COMMON(); p_sys = p_demux->p_sys;
> > @@ -247,6 +232,9 @@ static int Open( vlc_object_t *p_this )
> >      p_sys->b_spu_change = false;
> >      p_sys->i_vobu_index = 0;
> >      p_sys->i_vobu_flush = 0;
> > +#ifdef DVD_READ_CACHE
> > +    p_sys->b_readahead = p_dvd_stream ? false : true;
> 
> C has an operator for logical negation.
> 
> > +#endif
> >
> >      if( 1 )
> >      {
> > @@ -266,7 +254,7 @@ static int Open( vlc_object_t *p_this )
> >      }
> >
> >      /* Configure dvdnav */
> > -    if( dvdnav_set_readahead_flag( p_sys->dvdnav, DVD_READ_CACHE ) 
> > !=
> > +    if( dvdnav_set_readahead_flag( p_sys->dvdnav, p_sys->b_readahead
> > ? 1 : 0 ) !=
> 
> Pointless ternary.
> 
> >            DVDNAV_STATUS_OK )
> >      {
> >          msg_Warn( p_demux, "cannot set read-a-head flag" );
> > @@ -370,6 +358,94 @@ static int Open( vlc_object_t *p_this )
> >  }
> >
> >
> > 
> > /*****************************************************************************
> > + * AccessDemuxOpen:
> > +
> > 
> > *****************************************************************************/
> > +static int AccessDemuxOpen ( vlc_object_t *p_this )
> > +{
> > +    demux_t *p_demux = (demux_t*)p_this;
> > +    char *psz_file;
> > +    int i_ret;
> > +    bool forced = false;
> > +
> > +    if( p_demux->psz_access != NULL
> > +     && !strncmp(p_demux->psz_access, "dvd", 3) )
> > +        forced = true;
> > +
> > +    if( !p_demux->psz_file || !*p_demux->psz_file )
> > +    {
> > +        /* Only when selected */
> > +        if( !forced )
> > +            return VLC_EGENERIC;
> > +
> > +        psz_file = var_InheritString( p_this, "dvd" );
> > +    }
> > +    else
> > +        psz_file = strdup( p_demux->psz_file );
> > +
> > +#if defined( _WIN32 ) || defined( __OS2__ )
> > +    if( psz_file != NULL )
> > +    {
> > +        /* Remove trailing backslash, otherwise dvdnav_open will 
> > fail */
> > +        size_t flen = strlen( psz_file );
> > +        if( flen > 0 && psz_file[flen - 1] == '\\' )
> > +            psz_file[flen - 1] = '\0';
> > +    }
> > +    else
> > +        psz_file = strdup("");
> > +#endif
> > +
> > +    /* Try some simple probing to avoid going through dvdnav_open
> > too often */
> > +    if( !forced && psz_file != NULL && ProbeDVD( psz_file ) != 
> > VLC_SUCCESS )
> 
> That does not seem right. That's not how it used to work as an 
> access_demux.

Ah yes, good catch.

> 
> > +    {
> > +        free( psz_file );
> > +        return VLC_EGENERIC;
> > +    }
> > +
> > +    i_ret = CommonOpen( p_this, psz_file, NULL, NULL );
> > +    free( psz_file );
> > +    return i_ret;
> > +}
> > +
> > 
> > +/*****************************************************************************
> > + * dvdnav stream callbacks
> > +
> > 
> > *****************************************************************************/
> > +static int stream_cb_seek( void *s, uint64_t pos )
> > +{
> > +    return stream_Seek( (stream_t *)s, pos );
> > +}
> > +
> > +static int stream_cb_read( void *s, void* buffer, int size )
> > +{
> > +    return stream_Read( (stream_t *)s, buffer, size );
> > +}
> > +
> > 
> > +/*****************************************************************************
> > + * DemuxOpen:
> > +
> > 
> > *****************************************************************************/
> > +static int DemuxOpen ( vlc_object_t *p_this )
> > +{
> > +    demux_t *p_demux = (demux_t*)p_this;
> > +    bool forced = false;
> > +
> > +    if( p_demux->psz_access != NULL
> > +     && !strncmp(p_demux->psz_access, "dvd", 3) )
> > +        forced = true;
> > +
> > +    static dvdnav_stream_cb stream_cb =
> > +    {
> > +        .pf_seek = stream_cb_seek,
> > +        .pf_read = stream_cb_read,
> > +        .pf_readv = NULL,
> > +    };
> > +
> 
> Missing check for CAN_FASTSEEK.

Ah yes, we can't play dvd without seek.
dvdread has now an internal cache when probing IFO, therefore there is
much less seeks than before. So, can we use CAN_SEEK instead ?

> 
> > +    /* Try some simple probing to avoid going through dvdnav_open
> > too often */
> > +    if( !forced && StreamProbeDVD( p_demux->s ) != VLC_SUCCESS )
> > +        return VLC_EGENERIC;
> > +
> > +    return CommonOpen( p_this, NULL, p_demux->s, &stream_cb );
> > +}
> > +
> > 
> > +/*****************************************************************************
> >   * Close:
> >
> > 
> > *****************************************************************************/
> >  static void Close( vlc_object_t *p_this )
> > @@ -636,14 +712,15 @@ static int Demux( demux_t *p_demux )
> >      uint8_t *packet = buffer;
> >      int i_event;
> >      int i_len;
> > +    dvdnav_status_t status;
> >
> > -#if DVD_READ_CACHE
> > -    if( dvdnav_get_next_cache_block( p_sys->dvdnav, &packet,
> > &i_event, &i_len )
> > -        == DVDNAV_STATUS_ERR )
> > -#else
> > -    if( dvdnav_get_next_block( p_sys->dvdnav, packet, &i_event, 
> > &i_len )
> > -        == DVDNAV_STATUS_ERR )
> > -#endif
> > +    if( p_sys->b_readahead )
> > +        status = dvdnav_get_next_cache_block( p_sys->dvdnav,
> > &packet, &i_event,
> > +                                              &i_len );
> > +    else
> > +        status = dvdnav_get_next_block( p_sys->dvdnav, packet, 
> > &i_event,
> > +                                        &i_len );
> > +    if( status == DVDNAV_STATUS_ERR )
> >      {
> >          msg_Warn( p_demux, "cannot get next block (%s)",
> >                    dvdnav_err_to_string( p_sys->dvdnav ) );
> > @@ -902,9 +979,8 @@ static int Demux( demux_t *p_demux )
> >      case DVDNAV_STOP:   /* EOF */
> >          msg_Dbg( p_demux, "DVDNAV_STOP" );
> >
> > -#if DVD_READ_CACHE
> > -        dvdnav_free_cache_block( p_sys->dvdnav, packet );
> > -#endif
> > +        if( p_sys->b_readahead )
> > +            dvdnav_free_cache_block( p_sys->dvdnav, packet );
> >          return 0;
> >
> >      case DVDNAV_HIGHLIGHT:
> > @@ -945,9 +1021,8 @@ static int Demux( demux_t *p_demux )
> >          break;
> >      }
> >
> > -#if DVD_READ_CACHE
> > -    dvdnav_free_cache_block( p_sys->dvdnav, packet );
> > -#endif
> > +    if( p_sys->b_readahead )
> > +        dvdnav_free_cache_block( p_sys->dvdnav, packet );
> >
> >      return 1;
> >  }
> > @@ -1482,3 +1557,32 @@ bailout:
> >      close( fd );
> >      return ret;
> >  }
> > +
> > 
> > +/*****************************************************************************
> > + * StreamProbeDVD: very weak probing that avoids going too often
> > into a dvdnav_open()
> > +
> > 
> > *****************************************************************************/
> > +static int StreamProbeDVD( stream_t *s )
> > +{
> > +    int64_t i_init_pos;
> > +    int ret = VLC_EGENERIC;
> > +
> > +    i_init_pos = stream_Tell( s );
> > +    /* ISO 9660 volume descriptor */
> > +    char iso_dsc[6];
> > +    if( stream_Seek( s, 0x8000 + 1 ) != VLC_SUCCESS
> > +     || stream_Read( s, iso_dsc, sizeof (iso_dsc) ) < (int)sizeof 
> > (iso_dsc)
> > +     || memcmp( iso_dsc, "CD001\x01", 6 ) )
> > +        goto bailout;
> 
> In demux probing phase, you can only peek and query stream properties. 
> You cannot move the offset, so you cannot seek and/or read.

Ah OK, good to know.

> 
> > +
> > +    /* Try to find the anchor (2 bytes at LBA 256) */
> > +    uint16_t anchor;
> > +
> > +    if( stream_Seek( s, 256 * DVD_VIDEO_LB_LEN ) == VLC_SUCCESS
> > +     && stream_Read( s, &anchor, 2 ) == 2
> > +     && GetWLE( &anchor ) == 2 )
> > +        ret = VLC_SUCCESS; /* Found a potential anchor */
> > +
> > +bailout:
> > +    stream_Seek( s, i_init_pos );
> > +    return ret;
> > +}
> 

Thanks for your review, I'll propose a new patch with all comments
addressed.

> -- 
> Rémi Denis-Courmont
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel



More information about the vlc-devel mailing list