[vlc-devel] Patch for C64 SID suppport
Rémi Denis-Courmont
remi at remlab.net
Fri Dec 3 23:20:55 CET 2010
Hello,
A few comments inline...
diff --git a/modules/demux/sid.cpp b/modules/demux/sid.cpp
new file mode 100644
index 0000000..e765909
--- /dev/null
+++ b/modules/demux/sid.cpp
@@ -0,0 +1,337 @@
+/**
+ * @file sid.cpp
+ * @brief Sidplay demux module for VLC media player
+ */
+/*****************************************************************************
+ * Copyright © 2010 Rémi Denis-Courmont
Copied from the GME demux? You should _add_ your copyright then.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public License
+ * as published by the Free Software Foundation; either version 2.1
+ * of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301
USA
+
****************************************************************************/
+
+#ifdef HAVE_CONFIG_H
+# include <config.h>
+#endif
+
+#include <stdarg.h>
+#include <limits.h>
+#include <assert.h>
I don't see any assert()'ion. Why do you include this file?
+
+#include <vlc_common.h>
+#include <vlc_input.h>
+#include <vlc_demux.h>
+#include <vlc_plugin.h>
+
+#if SIDPLAY_VERSION==1
+# include <sidplay/player.h>
+#elif SIDPLAY_VERSION==2
+# include <sidplay/sidplay2.h>
+# include <sidplay/builders/resid.h>
+#endif
+
+#if SIDPLAY_VERSION==1
+ // libsidplay's engine can not be re-used once deallocated, so
allocate it statically
+ static emuEngine engine;
+#endif
This is not going to work. First, VLC plugins must be re-entrant, and I
don't suppose that the "engine" object is thread-safe? Second, the plug-in
may be unloaded or reloaded, in which case, the memory leak will still
occur. I would advise simply dropping support for sidplay 1. If you think
sidplay 1 is really worth supporting, then you probable need to add a
static mutex and mark the plug-in as not-to-be-unloaded.
+static int Open (vlc_object_t *);
+static void Close (vlc_object_t *);
+
+vlc_module_begin ()
+ set_shortname ("SID")
+#if SIDPLAY_VERSION==1
+ set_description ("Sidplay")
+#elif SIDPLAY_VERSION==2
+ set_description ("Sidplay2")
+#endif
+ set_category (CAT_INPUT)
+ set_subcategory (SUBCAT_INPUT_DEMUX)
+ set_capability ("demux", 10)
Is that high enough? IIRC, priority 10 is used by some rather "promiscuous"
demux plug-ins such as the MPEG-PS one.
+ set_callbacks (Open, Close)
+vlc_module_end ()
+
+struct demux_sys_t
+{
+#if SIDPLAY_VERSION==1
+ emuEngine *player;
+ emuConfig config;
+ sidTune *tune;
+ sidTuneInfo tuneInfo;
+#elif SIDPLAY_VERSION==2
+ sidplay2 *player;
+ sid2_config_t config;
+ sid2_info_t info;
+ SidTune *tune;
+ SidTuneInfo tuneInfo;
+#endif
+
+ es_format_t fmt;
+ es_out_id_t *es;
+ date_t pts;
+};
+
+
+static int Demux (demux_t *);
+static int Control (demux_t *, int, va_list);
+
+static int Open (vlc_object_t *obj)
+{
+ demux_t *demux = (demux_t *)obj;
+
+ int64_t size = stream_Size (demux->s);
+ if (size < 4 || size > LONG_MAX) /* We need to load the whole file for
sidplay */
+ return VLC_EGENERIC;
+
+ uint8_t *data=(uint8_t*)malloc(size);
+ if(data==NULL)
+ return VLC_ENOMEM;
+
+ if(stream_Read(demux->s,data,size)<size){
+ free(data);
+ return VLC_EGENERIC;
+ }
You cannot do this here. You need to probe the file type first. Otherwise
you will break any plug-in with lower priority.
+#if SIDPLAY_VERSION==1
+ sidTune *tune=new sidTune(0);
+#elif SIDPLAY_VERSION==2
+ SidTune *tune=new SidTune(0);
+#endif
+ if(unlikely(tune==NULL))
+ return VLC_ENOMEM;
Memory leak: free(data)
+#if SIDPLAY_VERSION==1
+ bool result=tune->load(data,size);
+#elif SIDPLAY_VERSION==2
+ bool result=tune->read(data,size);
+#endif
+ free(data);
+ if(result==false){
+ delete tune;
+ return VLC_EGENERIC;
+ }
+
+#if SIDPLAY_VERSION==1
+ emuEngine *player=&engine;
+#elif SIDPLAY_VERSION==2
+ sidplay2 *player=new sidplay2();
+#endif
+ if(unlikely(player==NULL)){
+ delete tune;
+ return VLC_ENOMEM;
+ }
+
+ demux_sys_t *sys = (demux_sys_t*) malloc(sizeof(demux_sys_t));
+ if (unlikely(sys == NULL)){
+#if SIDPLAY_VERSION==2
+ delete player;
+#endif
+ delete tune;
+ return VLC_ENOMEM;
+ }
+ memset(sys,0,sizeof(demux_sys_t));
+
+ sys->player=player;
+ sys->tune=tune;
+
+ tune->getInfo(sys->tuneInfo);
+
+#if SIDPLAY_VERSION==1
+ player->getConfig(sys->config);
+ sys->config.sampleFormat=SIDEMU_UNSIGNED_PCM;
Unsigned? Then why do you use the S16N (signed 16-bits native endianess)
codec?
+ sys->config.bitsPerSample=16;
+ sys->config.channels=2;
+ player->setConfig(sys->config);
+#elif SIDPLAY_VERSION==2
+ sys->info=player->info();
+ sys->config=player->config();
+ {
+ ReSIDBuilder *resid=new ReSIDBuilder("ReSID");
+ if (unlikely(resid == NULL)){
+#if SIDPLAY_VERSION==2
+ delete player;
+#endif
+ delete tune;
+ free(sys);
+ return VLC_ENOMEM;
+ }
+ resid->create(sys->info.maxsids);
+ resid->sampling(sys->config.frequency);
+ sys->config.sidEmulation=resid;
+ }
+ sys->config.precision=16;
+ sys->config.playback=sys->info.channels==2?sid2_stereo:sid2_mono;
+ player->config(sys->config);
+#endif
+
+ es_format_Init (&sys->fmt, AUDIO_ES, VLC_CODEC_S16N);
+
+ sys->fmt.audio.i_rate = sys->config.frequency;
+#if SIDPLAY_VERSION==1
+ sys->fmt.audio.i_channels = sys->config.channels;
+ sys->fmt.audio.i_bitspersample = sys->config.bitsPerSample;
+#elif SIDPLAY_VERSION==2
+ sys->fmt.audio.i_channels = sys->info.channels;
+ sys->fmt.audio.i_bitspersample = sys->config.precision;
+#endif
+ sys->fmt.audio.i_bytes_per_frame = sys->fmt.audio.i_channels *
sys->fmt.audio.i_bitspersample/8;
+ sys->fmt.audio.i_frame_length = sys->fmt.audio.i_bytes_per_frame;
+ sys->fmt.audio.i_blockalign = sys->fmt.audio.i_bytes_per_frame;
+ sys->fmt.i_bitrate = sys->fmt.audio.i_rate *
sys->fmt.audio.i_bytes_per_frame;
+
+ sys->es = es_out_Add (demux->out, &sys->fmt);
+ date_Init (&sys->pts, sys->fmt.audio.i_rate, 1);
+ date_Set (&sys->pts, 0);
+
+#if SIDPLAY_VERSION==1
+ result=sidEmuInitializeSong(*sys->player,*sys->tune,0);
+#elif SIDPLAY_VERSION==2
+ sys->tune->selectSong(0);
+ result=(sys->player->load(sys->tune)>=0);
+ sys->player->fastForward(100);
+#endif
+ if(result==false){
+#if SIDPLAY_VERSION==2
+ delete sys->player;
+ delete sys->config.sidEmulation;
+#endif
+ delete sys->tune;
+ free(sys);
Could be factored to Close(obj);
+ return VLC_ENOMEM;
+ }
+
+ /* Callbacks */
+ demux->pf_demux = Demux;
+ demux->pf_control = Control;
+ demux->p_sys = sys;
+
+ return VLC_SUCCESS;
+}
+
+
+static void Close (vlc_object_t *obj)
+{
+ demux_t *demux = (demux_t *)obj;
+ demux_sys_t *sys = demux->p_sys;
+
+#if SIDPLAY_VERSION==2
+ delete sys->player;
+ delete sys->config.sidEmulation;
+#endif
+ delete sys->tune;
+ free (sys);
+}
+
+static int Demux (demux_t *demux)
+{
+ demux_sys_t *sys = demux->p_sys;
+ int i_bk = ( sys->fmt.audio.i_bitspersample / 8 ) *
sys->fmt.audio.i_channels;
+ block_t *block = block_New( p_demux, sys->fmt.audio.i_rate / 10 * i_bk
);
+ if (unlikely(block == NULL))
+ return 0;
+
+ if(sys->tune->getStatus()==false)
+ return 0;
+
+ int i_read=block->i_buffer;
+#if SIDPLAY_VERSION==1
+
sidEmuFillBuffer(*sys->player,*sys->tune,(void*)block->p_buffer,block->i_buffer);
+#elif SIDPLAY_VERSION==2
+ i_read=sys->player->play((void*)block->p_buffer,block->i_buffer);
+#endif
+ if(i_read<=0){
+ block_Release (block);
+ return 0;
+ }
+ block->i_buffer=i_read;
+ block->i_pts = block->i_dts = VLC_TS_0 + date_Get (&sys->pts);
+
+ es_out_Control (demux->out, ES_OUT_SET_PCR, block->i_pts);
+
+ es_out_Send (demux->out, sys->es, block);
+
+ date_Increment (&sys->pts, i_read/i_bk);
+
+ return 1;
+}
+
+
+static int Control (demux_t *demux, int query, va_list args)
+{
+ demux_sys_t *sys = demux->p_sys;
+
+ switch (query)
+ {
+ case DEMUX_GET_TIME:{
+ int64_t *v=va_arg(args,int64_t*);
+ *v=0;
This statement has no effects.
+#if SIDPLAY_VERSION==1 && defined(SIDEMU_TIME_COUNT)
+ *v=sys->player->getSecondsThisSong()*1000;
Use CLOCK_FREQ.
+#elif SIDPLAY_VERSION==2
+ *v=sys->player->time()*sys->player->timebase()*10;
+#endif
+ *v=*v * 1000;
+ return VLC_SUCCESS;
+ }
+
+ case DEMUX_GET_META:{
+ vlc_meta_t *p_meta = (vlc_meta_t *)va_arg( args, vlc_meta_t*
);
+ vlc_meta_SetTitle( p_meta, sys->tuneInfo.infoString[0] );
+ vlc_meta_SetArtist( p_meta, sys->tuneInfo.infoString[1] );
+ vlc_meta_SetCopyright( p_meta, sys->tuneInfo.infoString[2] );
+ return VLC_SUCCESS;
+ }
+
+ case DEMUX_GET_TITLE_INFO:
+ if( sys->tuneInfo.songs > 1 )
+ {
+ input_title_t ***ppp_title = (input_title_t***)va_arg(
args, input_title_t*** );
+ int *pi_int = (int*)va_arg( args, int* );
+
+ *pi_int = sys->tuneInfo.songs;
+ *ppp_title = (input_title_t**)xmalloc( sizeof(
input_title_t**) * sys->tuneInfo.songs );
+
+ for( int i = 0; i < sys->tuneInfo.songs; i++ )
+ {
+ char psz_temp[16];
+ snprintf(psz_temp, 15, "Song %i", i);
+ (*ppp_title)[i] = vlc_input_title_New();
+ (*ppp_title)[i]->psz_name = strdup(psz_temp);
+ }
+
+ return VLC_SUCCESS;
+ }
+ return VLC_EGENERIC;
+
+ case DEMUX_SET_TITLE:{
+ int i_idx = (int)va_arg( args, int );
+ bool result=false;
+#if SIDPLAY_VERSION==1
+ result=sidEmuInitializeSong(*sys->player,*sys->tune,i_idx+1);
+#elif SIDPLAY_VERSION==2
+ sys->tune->selectSong(i_idx+1);
+ result=(sys->player->load(sys->tune)>=0);
+#endif
+ if(result==false){
+ return VLC_EGENERIC;
+ }
+ demux->info.i_title = i_idx;
+ demux->info.i_update = INPUT_UPDATE_TITLE;
+ msg_Dbg( demux, "set song %i", i_idx);
+ return VLC_SUCCESS;
+ }
+
+ }
+
+ return VLC_EGENERIC;
+}
--
Rémi Denis-Courmont
http://www.remlab.net
http://fi.linkedin.com/in/remidenis
More information about the vlc-devel
mailing list