[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-core-devel
Subject:    Re: kdemultimedia/kioslave/audiocd [POSSIBLY UNSAFE]
From:       Scott Wheeler <wheeler () kde ! org>
Date:       2004-02-08 19:04:06
Message-ID: 200402082004.07896.wheeler () kde ! org
[Download RAW message or body]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Sorry, I haven't looked at the code yet -- I will soon, but it's not 
acceptable to rewrite code that you don't maintain without mentioning it to 
the maintainer and then to check it in.  There was already a rewrite in 
progress and in CVS and it's been on the feature plan (originally for the 3.2 
release but moved to 3.3).

- -Scott

On Sunday 08 February 2004 2:59, Benjamin Meyer wrote:
> CVS commit by bmeyer: 
> 
> Audiocd Code Refactoring commit
> 
> CCMAIL: kde-core-devel@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
> 
> 
> 
> 

- -- 
Audience Member: "What was the hardest part of building TeX?"
Donald Knuth: "It was all pretty easy."
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.0.7 (GNU/Linux)

iD8DBQFAJogmQu0ByfY5QTkRAttXAJ43K1TP7OR+FSCXIrmTuredIRD3xQCeLXJf
Tv71Sh5+iTTeMaHe0AR7oy4=
=+hiZ
-----END PGP SIGNATURE-----

[prev in list] [next in list] [prev in thread] [next in thread]