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

[Diego Biurrun]

On Thu, Dec 16, 2010 at 09:58:39PM +0100, Clément Bœsch wrote:
> 
> Well, since I still don't have any feedback, I tried to split it more. So
> There is now 3 patches. I wonder how I can split it more. Please review
> this so I can move on :)
> 
> --- a/sub/subreader.c
> +++ b/sub/subreader.c
> @@ -1889,7 +1890,7 @@ static int compare_sub_priority(const void *a, const void *b)
>  
> -char** sub_filenames(const char* path, char *fname)
> +static char** sub_filenames(const char* path, const char *fname)

Please fix the * placement to K&R while you're changing this line anyway.

> @@ -2070,6 +2071,30 @@ char** sub_filenames(const char* path, char *fname)
>  
> +/**
> + * \brief Load all subtitles matching the subtitle filename
> + * \param fname Path to subtitle filename
> + * \param fps FPS parameter for the add subtitle function
> + * \param add_f Add subtitle function to call for each sub

Please use @ syntax with Doxygen.  I'm afraid we have a mix of both in
MPlayer, but @ is more common and easier to grep for :)

> --- a/DOCS/man/en/mplayer.1
> +++ b/DOCS/man/en/mplayer.1
> @@ -2625,6 +2625,27 @@ Guess the encoding for Polish, fall back on cp1250.
>  .
>  .TP
> +.B \-subdirs <dirname1,dirname2,...>
> +Specify extra subtitles directories to track in the media directory.
> +.sp 1
> +.I EXAMPLE:
> +Assuming that /path/\:to/\:movie/\:movie.avi is played and \-subdirs
> +sub,subtitles,/tmp/subs is specified, MPlayer searches for subtitles files in these

subtitle files

And please break this line earlier.

> --- a/sub/subreader.c
> +++ b/sub/subreader.c
> @@ -2123,6 +2123,18 @@ void load_subtitles(const char *fname, int fps, void add_f(char *, float, int))
>  
> +    // Load subtitles in dirs specified by subdirs option
> +    if (sub_dirs)
> +        for (i = 0; sub_dirs[i]; i++)
> +#if HAVE_DOS_PATHS
> +            if (sub_dirs[i][1] == ':')
> +#else
> +            if (sub_dirs[i][0] == '/')
> +#endif

Maybe this can be factorized into a global PATH_SEP #define or similar?
I suspect we have something similar in other places...

Diego