[MPlayer-dev-eng] [PATCH] Subtitles directories

[Clément Bœsch]

On Sun, Nov 21, 2010 at 11:12:32PM +0100, Clément Bœsch wrote:
> On Sun, Nov 21, 2010 at 09:39:03PM +0100, Reimar Döffinger wrote:
> > On Sun, Nov 21, 2010 at 05:46:16PM +0100, Clément Bœsch wrote:
> > > +/**
> > > + * \brief Append a subtitle to a list
> > > + * \param dst Destination subtitles list
> > > + * \param src Source subtitle
> > > + * \warning Source data are copied and not reallocated
> > > + */
> > > +static int append_sub(struct sub_list *dst, struct subfn *src)
> > >  {
> > > +    if (dst->sid >= MAX_SUBTITLE_FILES)
> > > +        return -1;
> > > +    memcpy(&dst->subs[dst->sid], src, sizeof(*src));
> > > +    dst->sid++;
> > > +    return 0;
> > 
> > Either getting rid of this function or at least making it take
> > the name etc. as arguments and having it do the strdup will
> > make things even simpler.
> > 
> 
> Changed with arguments method.
> 
> > > @@ -2012,8 +2013,7 @@ char** sub_filenames(const char* path, char *fname)
> > >  		    }
> > >  		    if (!prio) {
> > >  			// doesn't contain the movie name
> > > -			// don't try in the mplayer subtitle directory
> > > -			if ((j == 0) && (sub_match_fuzziness >= 2)) {
> > > +			if (sub_match_fuzziness >= 2) {
> > 
> > Huh? Where did handling of this end up? Loading any subtitle file anywhere in the
> > path doesn't sound to me like it would ever be desireable.
> > 
> 
> I added a limit_fuzziness flag to keep ignoring sub_fuzziness=2 on
> ~/.mplayer/sub directory. subdirs patch updates a little the documentation
> about this point too.
> 
> > > +    path = realloc(path, plen + strlen(dir) + 1);
> > > +    if (!path)
> > > +        return -1;
> > 
> > memleak in case realloc fails.
> > 
> 
> Fixed.
> 
> > > +    strcpy(path + plen, dir);
> > 
> > av_strlcpy might make it more obvious.
> > 
> 
> Why? It would either add a second strlen call or a new variable. Maybe you
> meant strlcat; but even so, the problem is the same. Or maybe I missed
> something.
> 
> > > +    // Load subtitles specified by sub option with highest priority
> > > +    if (sub_name) {
> > > +        int i;
> > > +        for (i = 0; sub_name[i]; i++) {
> > > +            struct subfn sub = {
> > > +                .fname    = strdup(sub_name[i]),
> > > +                .priority = INT_MAX - i,
> > > +                .noerr    = 0
> > > +            };
> > > +            append_sub(&slist, &sub);
> > > +        }
> > 
> > Seems a bit overkill to push them through qsort.
> > Though it might be the simplest way.
> 
> Yes but qsort was already in use, I didn't want to change that.
> 
> Patches re-attached.
> 

Oups wrong patches, sorry. Patches reattached.

-- 
Clément B.
Not sent from a jesusPhone.