[vlc-devel] [PATCH V3] dcp: Creation of access-demux module for DCP (using asdcplib)

[Nicolas Bertrand]

Le 03/09/2013 00:57, Jean-Baptiste Kempf a écrit :
> On 02 Sep, Nicolas Bertrand wrote :
>> +dnl  DCP plugin (using asdcplib)
>> +dnl
>> +AC_ARG_ENABLE(dcp,
>> +  AS_HELP_STRING([--enable-dcp],[Digital Cinema Package support using asdcplib (default auto)]))
>> +have_asdcp="no"
>> +AS_IF([test "x${enable_dcp}" != "no"], [
>> +  AC_LANG_PUSH(C++)
>> +  AC_CHECK_HEADERS( [[AS@&t at _DCP.h]],
>> +    [have_asdcp="yes"],
>> +    [AS_IF( [test "x${enable_dcp}" = "yes"],
>> +      [AC_MSG_ERROR( [ ASDCP library cannot be found (needed for dcp module). Either use --enable-dcp=no or install asdcp library: http://www-dev.pub.cinecert.com/asdcplib/download/ ] )])
>> +     ])
>> +  AC_LANG_POP(C++)
>> +])
>> +AM_CONDITIONAL(HAVE_ASDCP, [test "${have_asdcp}" != "no"])
>
> Oh god, no .pc?

Sorry, no .pc.
I'll sent a request to the dev.

>
>
>
> You should factor the error path.
If you meant genarlization of goto error -->OK
> And I think you can accept files where one track fails, no?
Yes, Maybe, No :-)
Lets start with simple case of a DCP with a video and a audio track.
Much more complicetd case commming later. ( case of several video, 
several audio, several subs) or at reverse only one audio track to 
complete an already existent DCP



>
>> +bool endsWith( const char * str, const char * suffix )
>> +{
>> +    if( !str || !suffix )
>> +        return false;
>> +    size_t lenstr = strlen( str );
>> +    size_t lensuffix = strlen( suffix );
>> +    if( lensuffix >  lenstr )
>> +        return false;
>> +    if( strncasecmp( str + lenstr - lensuffix, suffix, lensuffix ) == 0 )
>> +        return true;
>> +    else
>> +        return false;
>> +}
>
> This is not very beautiful, tbh.*
Pretitfication done ( ready fot next patch)

>
>> +int dcpInit ( demux_t *p_demux )
>> +{
>> +    int retval;
>> +    /* Allocate internal state */
>> +    demux_sys_t *p_sys = new ( nothrow ) demux_sys_t();
>> +    if( unlikely( p_sys == NULL ) )
>> +        return VLC_ENOMEM;
>> +    p_demux->p_sys = p_sys;
>> +
>> +    /* Allocate DCP object */
>> +    dcp_t *p_dcp = new ( nothrow ) dcp_t;
>> +    if( unlikely( p_dcp == NULL ) )
>> +        return VLC_ENOMEM;
>> +
>> +    p_sys->p_dcp = p_dcp;
>> +
>> +    p_dcp->path = p_demux->psz_file;
>> +    /* We check if there is a "/" at the end of the path */
>> +    if( !endsWith( p_demux->psz_file, "/" ) )
>> +    {
>> +        try
>> +        {
>> +            p_dcp->path.append( "/" );
>> +        }
>> +        catch(...)
>> +        {
>> +            msg_Err( p_demux, "String handling failed" );
>> +            return VLC_EGENERIC;
>> +        }
>> +    }
>> +
>> +    /* Parsing XML files to get audio and video files */
>> +    msg_Dbg( p_demux, "parsing XML files..." );
>> +    if( ( retval = parserXML( p_demux ) ) )
>> +        return retval;
>.....
>> +    return VLC_SUCCESS;
>> +}
>
> I don't really get the retval use, but OK.
I'm not sure to get the point is there a simple or other way to write : ?
 >> +    if( ( retval = parserXML( p_demux ) ) )
 >> +        return retval;
>
>> +

>
> Please reduce the nesting levels. This is getting crazy.
OK, implemented for next patch