[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: New upload of PenguinSound



Peter Burns wrote:

>I have removed the pps prefix from all my classes and files. I now use
>the pp::internal namespace for all my classes. I have placed the result
>in ftpspace
>in the file "PenguinPlay-0.0.0.tar.gz".
>
>If you approve of it, it will be ready for release.
>
>There are two major known bugs. 
>- The timeout functions in DirectSound don't work.
>- Sequencer::SetFrequency doesn't work.

PFile still needs bugfixing as well, so you still have some time to fix it
;)

>I added two directories to the tree -- ./windows and

I prefer the approach Bjarke suggested - Having the VC++ Workspace file at
/src/ and the project files in /src/PenguinFile etc
We can add a seperate dir for the windows stuff later when we really need
it (when we have really much win-specific things).

>./include/PenguinPlay/Sound

Having all public includes directly in include/PenguinPlay/ (no further
subdirs) is better IMHO. That requires renaming some of the headers to
something less ambiguous, but that's ok as the classes contained in them
need the same treatment (after all they are just in the one, PPlay-wide
namespace).

My suggestion:
Cdrom -> AudioCD
ExceptionTypes -> SoundExceptions (some can also be moved to the generic
                                   exceptions.h)
Factory is not sound specific -> fine
Fixed is not sound specific -> fine

Stream.h: ??? #include <ppsExport.h> will surely fail etc. Some leftover ?

About the namespaces: public symbols should be "exported" into the main pp
namespace, like here (the change to your code is the line between the two
final PP_*NAMESPACE_END macros):

------------
PP_NAMESPACE_BEGIN
PP_I_NAMESPACE_BEGIN

//=============================================================================
/// ESounD support
///  Loads libesd.so at runtime and gets the esd_play_stream_fallback symbol.
///  Then obtains a file descriptor to write to.
//=============================================================================
class PP_EXPORT Esd : public AudioToFile
{
public:
  Esd(int rate = 44100, int bits = 16, bool stereo = true);
  virtual ~Esd();

private:
  class EsdPrivate* m_private;
};

PP_I_NAMESPACE_END

using internal::Esd; // make ... visible in the main namespace

PP_NAMESPACE_END
--------------

Through that the user can simply access all public PPlay symmbols via the
"pp" namespace.


>./src/PenguinSound now contains only source and no subdirectories.

Very good.

>I also tried doxygen for generating api references. I like it much more
>than perceps.

Uh, ok. I'll have a look at it.


	Christian
-- 

Drive A: not responding...Formatting C: instead