[vlc-devel] Patch for C64 SID suppport
Alan Fischer
alan at lightningtoads.com
Tue Dec 7 05:28:24 CET 2010
I apologize if this email arrives twice, I initially sent it from the wrong
account, which is not subscribed to the list.
Thank you for all the feedback,
I have attached a new patch which only uses sidplay2, since sidplay1 seemed
to be too problematic to support, and should take into account the rest of
the comments I received.
The only thing I was unsure about was the alignment issue. I am setting
sys->fmt.audio.i_blockalign, is there additional alignment I should be
adding somewhere?
Thanks again,
Alan Fischer
>
>
> 2010/12/3 Rémi Denis-Courmont <remi at remlab.net>
>
>
>> 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
>>
>> _______________________________________________
>> vlc-devel mailing list
>> To unsubscribe or modify your subscription options:
>> http://mailman.videolan.org/listinfo/vlc-devel
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20101206/4ed03a0d/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Added-C64-sid-demux-module.patch
Type: application/octet-stream
Size: 10200 bytes
Desc: not available
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20101206/4ed03a0d/attachment.obj>
More information about the vlc-devel
mailing list