[MPlayer-dev-eng] [PATCH] Subtitles directories
Clément Bœsch
ubitux at gmail.com
Sun Nov 21 23:16:27 CET 2010
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.
More information about the MPlayer-dev-eng
mailing list