[vlc-devel] [RFC] dcp.cpp: Creation of access-demux module for DCP (using asdcplib)
ileoo at videolan.org
Mon Jun 3 12:05:38 CEST 2013
On Mon, Jun 03, 2013 at 11:01:12AM +0200, Simona-Marinela Prodea wrote:
> The module only handles audio and video files in DCP, no subtitles yet. Is this form worthy of a patch?
Many smaller things are worthy of a patch. Few comments inline here and there
that caught my eye.
> +libdcp_plugin_la_SOURCES = dcp.cpp
> +libdcp_plugin_la_CPPFLAGS = $(AM_CPPFLAGS) $(LIBXML2_CFLAGS)
> +libdcp_plugin_la_LIBADD = $(AM_LIBADD) $(LIBXML2_LIBS) -lasdcp
> +libvlc_LTLIBRARIES += libdcp_plugin.la
This compiles libdcp_plugin unconditionally and it fails if system
doesn't have lasdcp library. Is this intented or just syntax for
checking/enabling it in configure.ac was not so clear?
> +++ b/modules/access/dcp.cpp
> +/* ASDCP headers */
> +#include "AS_DCP.h"
Should this file be included in patch too?
> +using namespace ASDCP;
> +#define FRAME_BUFFER_SIZE 4 * 1024 * 1024
This could maybe include some small mention where/why it is that amount
> +/* Module descriptor */
> + set_shortname( N_( "DCP" ) )
> + add_shortcut( "dcp" )
> + set_description( N_( "DCP access-demux module" ) )
> + set_capability( "access_demux", 100 ) /* score? */
setting capability to 0 means it is only used when forced (eg using
> +static int Demux( demux_t *p_demux )
> + return 1;
Maybe return VLC_SUCCESS ?
> +static int Control( demux_t *p_demux, int query, va_list args )
> + case DEMUX_CAN_CONTROL_PACE:
Maybe also CAN_SEEK ?
> + * Function which parses XML files in DCP directory in order to get video and audio files
> + * @param p_demux Demux pointer.
> + * @return Integer according to the success or not of the operation
> + */
> + int parserXML ( demux_t * p_demux )
> + return 1; /* TODO : perform checking on XML parsing */
Any particular reason not using vlc xml_reader ability but directly
> + * Function to check if a file name ends with a given suffix
> + * @param str File name
> + * @param suffix Suffix to look for
> + * @return true if the file name ends with the given suffix, fale otherwise
Nobody knows the trouble I've been.
-------------- next part --------------
A non-text attachment was scrubbed...
Size: 490 bytes
Desc: Digital signature
More information about the vlc-devel