[MPlayer-dev-eng] [PATCHES] VOBsub cleanup

[Reimar Döffinger]

On Sat, Jan 22, 2011 at 12:17:21PM +0100, Clément Bœsch wrote:
> On Sat, Jan 22, 2011 at 11:31:39AM +0100, Reimar Döffinger wrote:
> > On Sat, Jan 22, 2011 at 11:23:35AM +0100, Clément Bœsch wrote:
> > > On Sat, Jan 22, 2011 at 11:12:06AM +0100, Reimar Döffinger wrote:
> > > > On Fri, Jan 21, 2011 at 10:50:42PM +0100, Clément Bœsch wrote:
> > > > > Ok, changed by passing the vo_vobsub instance (I can't make the function
> > > > > return it because of the error handling).
> > > > 
> > > > You'll have to explain that. I only see you use 0 and -1, so why can't
> > > > you return a vo_vobsub or NULL respectively?
> > > 
> > > load_vob_subtitle could return the vobsub instance on sucess and NULL on
> > > failure, but not being able to load a vobsub does not mean there is an
> > > error. There is two cases where the loading raise an error: if vobsub name
> > > is stated but the file does not exist, or allocation error. In the other
> > > cases, if vobsub_open fails, this just means the file wasn't found, but it
> > > does not mean there is an error.
> > 
> > It is a bit inconsistent with load_subtitles which never returns any error.
> 
> What about this version?

Maybe, I do not really know what the best approach is.
You worked more with that code recently and I think you might have a
better idea.
But if you want a typedef for the function type maybe load_subtitles
should be changed afterwards to use one as well?