On Sun, Nov 21, 2010 at 11:12:32PM +0100, Clément Bœsch wrote:
@@ -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.
Sure it doesn't make more sense to only apply it to the movie directory and nothing else at all? Doesn't make sense to have a path where there's only the subtitle(s) for one single movie - and in all other cases it will end up with wrong subtitles.
+ 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.
Yes I meant strlcat. Doesn't matter much, it's just that this is kind of a reimplementation of strcat, which is not so great from a code-readability standpoint.
+ // 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.
qsort wasn't used on the subtitles specified with -sub. And actually, -sub did disable auto-loading before. I was assuming you just added more search paths, but you changed the behaviour all over the place, that makes this review really a major pain.