kdemultimedia/kioslave/audiocd [POSSIBLY UNSAFE]

[Sebastian Trueg]

k3b already has an encoder plugin system with encoding plugins for ogg-vorbis, 
wave, monkey-audio, and one that uses external tools. Maybe it's possible to 
prevent code duplication?

Cheers,
Sebastian

On Sunday 08 February 2004 02:59, Benjamin Meyer wrote:
> CVS commit by bmeyer:
>
> Audiocd Code Refactoring commit
>
> CCMAIL: kde-core-devel at kde.org
> -------------
> The commit you all (ok, maybe just me and a few others) have been waiting
> for.
>
> audiocd:/ no longer uses ifdefs!!!
>
> There is a new base class called Encoder which the different encoders are
> based off of.  Wav is in fact based upon CDA which is based upon Encoder.
>
> This means that the audiocd.h and audiocd.cpp files only deal with
> ripping the cd and with the ioslave work and just make a call to the
> selected encoder to do the work, no more case, ifdef, if else else else
> else...
>
> In the process of converting the module I discovered countless edge
> cases bugs that a) never were reported, b) probably would be too hard to
> find if you didn't know where to look. c) most arn't noticable.  By simply
> having the one call to the base class rather then the bunch of ifdefs
> acidents in copy/paste were all removed.
>
> The work that I did was 99% just refactoring of the code.  Very little new
> code was added to the project.  The majority of the actuall code changes
> were bug fixes.  Taking this approch first and then enhancing it I thought
> would be best.  In fact will all the fixes it might be worth while to put
> in the 3_2 tree.
>
> Future work(?)
> --------------
> 1) Currently the Encoder are build into the audiocd binary, but one could
> easily make them into libraries.  This offers the really neat idea that
> one could re-use these in other applications (KAudioCreator, Juk, etc).
> Maybe not re-using the encoding portion, but simply to get the configure
> dialog for that encoder so that the third party application could use it
> and then just make an audiocd ioslave call to get the file after saving
> the settings.
>
> 2) Now that the Encoding is abstracted maybe doing the same with the
> ripper would be a good idea.  This would allow for a cdda2wav module.
>
> 3) With the Encoder class it is 1000% times easier to add a new encoders
> to the audiocd framework.  (Anyone feel free to write a Flac encoder!)
>
> A far as the user is concerned it should be very similar.  I think the only
> difference is that there is now a CDA and a Wav directory for the two
> encoders.  I am very happy with how smoothly this went and hope it will
> encurage others to take a look into working on this module.
>
> P.S. TODO the lame encoder doesn't set the year (never did) in the id3 tag.
>
>
>   A            encoder.h   1.1 [GPL (v2+)]
>   A            encodercda.cpp   1.1 [GPL (v2+)]
>   A            encodercda.h   1.1 [GPL (v2+)]
>   A            encoderlame.cpp   1.1 [POSSIBLY UNSAFE: printf] [GPL (v2+)]
>   A            encoderlame.h   1.1 [GPL (v2+)]
>   A            encodervorbis.cpp   1.1 [GPL (v2+)]
>   A            encodervorbis.h   1.1 [GPL (v2+)]
>   A            encoderwav.cpp   1.1 [GPL (v2+)]
>   A            encoderwav.h   1.1 [GPL (v2+)]
>   M +2 -2      Makefile.am   1.12
>   M +146 -859  audiocd.cpp   1.85 [POSSIBLY UNSAFE: qDebug]
>   M +23 -10    audiocd.h   1.17