Comment 23 for bug 412647

(In reply to comment #22)
> >Don't use XPCOM unless either a) we want the interface to be accessible or
> >implementable by non-C++ or b) we want the interface to be *dynamically*
> >pluggable. I don't think either applies in this case, so it should just be
> >plain C++. And we don't need an abstract base class, just a concrete class.
>
> I used XPCOM to make it easier for other people to implement different audio
> backends if Sydney wasn't available for their platform. It also made it easier
> for me to swap PortAudio and Sydney for testing purposes. I can make it plain
> C++ inheriting from an abstract base class and get the same effect but you're
> suggesting not to do that as well? Is the preference to have a concrete class
> with different implementations used via #defining out other platform code
> and/or different implementation files?

Since we don't need multiple audio backends right now, I would not bother making it easy to add new ones. If someone comes along and wants to add one, they can either extend Sydney (the best option), or add #ifdefs, or patch the code to use an abstract base class instead.

> The same question applies to nsVideoDecoder. I was moving towards having an
> abstract video decoder class, with Ogg being the provided implementation. So
> if someone wanted a gstreamer, or other platform specific support, they could
> just implement that. Is it not worth doing that?

That makes more sense since we know we want to support additional decoders in the future.

> >if you do the inheritance thing I suggested, you could use static_cast<> here.
> >...
> >Use "short(scaled_value)".
>
> In some places It's suggested to change to using a C++ cast, in others, to a C
> style casts. What's the rule for deciding which I should be using so I can get
> it right first time?

Use C++ casts wherever you can, which should be almost everywhere; they're safer. I don't recall why or where you needed to use a C-style cast.

> >We're not using std::nothrow elsewhere are we? If not, not much point in
> >starting to use it here.
>
> One reviewer suggesting using PR_Malloc to avoid the problem of operator new
> throwing an exception in an out of memory condition which, since we don't use
> exception handling anywhere, is likely to be bad. You suggested changing back
> to operator new. To solve both requests I used nothrow. Has operator new been
> changed in Mozilla to return 0 in out of memory conditions? Is it something
> that's not worth worrying about?

It hasn't been changed, but it needs to be. I guess you can just leave std::nothrow in for now.