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

[Clément Bœsch]

On Sat, Jan 22, 2011 at 12:27:32PM +0100, Reimar Döffinger wrote:
> 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.

Neither I do, but this one is more consistent, I'm going to commit this
version (with the assert).

> 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?

Sure, I'll do that.

-- 
Clément B.