Re: [Gnash-dev] Re: Nellymoser audio segfaults (bug #24355])

[Sandro Santilli]

On Tue, Sep 23, 2008 at 03:05:28PM +0200, Benjamin Wolsey wrote:
> > >From MediaHandler.h documentation I read:
> > 
> > 
> >         /// Create a VideoDecoder for the specified codec_type
> >         //
> >         /// @return 0 if no decoder could be created for the specified 
> > encoding
> >         ///
> >         virtual std::auto_ptr<VideoDecoder> createVideoDecoder(VideoInfo& 
> > info)=0;
> > 
> > I like that design in general.
> > Yes, there might be other exceptions thrown for OOM conditions or similar,
> > but I'd rather have MediaHandler catch them then MediaHandler users.
> > 
>
> But it's not how the VideoDecoder ever worked. Even less since I made it
> throw an exception, but in that case there never was any check at all on
> the validity of the object, or any possibility to do so.

If the code doesn't do what the documentation says is a bug.
Particularly important when code is virtual, and documentation
is in base class.

Looks like this discussion should have been triggered *before* :)

> > The bug I see is that MediaHandlerFfmpeg::createAudioDecoder is just
> > not checking return code from AudioDecoderFfmpeg::setup.
> 
> I don't think it's a good idea to have an object that can be created but
> isn't valid until you've called setup() *and* checked the return code of
> it. It's pointless and an unnecessary risk, when you can make absolutely
> sure that no invalid objects ever exist by using the constructor and
> exceptions.

I've no problem for MediaHandlerFfmpeg to catch exceptions thrown
by VideoDecoder constructor.

The API to maintain (and only change if we really mean it) is
the MediaHandler one. Corelib doesn't have to do with construction
of those decoders so you can do whatever you want with them.

> And it has the advantage of allowing a single debugging
> message telling you exactly what failed and where, instead of several
> that don't tell you very much.

This is an interesting point. I'm new to exceptions, dunno how appropriate
it'd be to use them as a logging mechanism.
What I've learnt as a use for them was "exceptional behaviour".
How exceptional is for a media handler to NOT support a specific codec ?

--strk;