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

[Clément Bœsch]

On Thu, Dec 23, 2010 at 04:33:19PM +0100, Clément Bœsch wrote:
> On Wed, Dec 22, 2010 at 05:53:24PM +0100, Reimar Döffinger wrote:
> > On 21 dec 2010, at 11:00, Clément Bœsch <ubitux at gmail.com> wrote:
> > 
> > > On Thu, Dec 16, 2010 at 09:58:39PM +0100, Clément Bœsch wrote:
> > >> On Mon, Nov 29, 2010 at 11:08:55PM +0100, Clément Bœsch wrote:
> > >>> On Tue, Nov 23, 2010 at 08:07:25AM +0100, Reimar Döffinger wrote:
> > >>>> On Tue, Nov 23, 2010 at 12:50:45AM +0100, Clément Bœsch wrote:
> > >>>>> Oh. This is bad. How do you want me to handle that? Of course I personally
> > >>>>> consider the factorization also being a fix for the MEncoder code, but
> > >>>>> something tell me you won't agree with that :-(
> > >>>> 
> > >>>> I am ok with considering a fix, I am just not convinced it is fixing it
> > >>>> the right way round. But it probably is more flexible this way.
> > >>>> 
> > >>>>> By the way, is the rest of this first patch clear enough now?
> > >>>> 
> > >>>> I didn't look that closely yet. I still think it is too much code,
> > >>>> but that is just a gut feeling and may be wrong of course.
> > >>> 
> > >>> Sorry I couldn't simplify it more. Still not good to commit?
> > >>> 
> > >>> Also, I noticed the VOB-Sub loading is totally different from the default
> > >>> one. I'd like to work on it to make it benefit from other subtitles
> > >>> options (like sub-fuzziness, sub-directories too, etc); I'd like to get
> > >>> rid of the hackish loading in mplayer.c. I really can't integrate the
> > >>> subdirs option for vobsub in the current state without huge changes (and I
> > >>> don't think you want that, neither I do since it will delay the patch
> > >>> again).
> > >>> 
> > >> 
> > >> 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 :)
> > >> 
> > > 
> > > [...]
> > > 
> > >> Subject: [PATCH 1/3] Factorize subtitles loading between mplayer and mencoder.
> > >> 
> > >> ---
> > >> mencoder.c      |   18 +-----------------
> > >> mplayer.c       |   16 +---------------
> > >> sub/subreader.c |   27 ++++++++++++++++++++++++++-
> > >> sub/subreader.h |    2 +-
> > >> 4 files changed, 29 insertions(+), 34 deletions(-)
> > >> 
> > > 
> > > I will commit this first patch in the next three days so we can move on.
> > 
> > You removed the filename == NULL check that was in the mencoder code,
> > either keep that or double and triple check the code.  Otherwise it
> > seems ok to me, did not yet properly check the other patches.
> 
> There should not be any problem with the mencoder since there is an exit
> upper in case of no filename. But I don't get how it works with mplayer
> code...
> 
> I fixed this in the patch (reattached).
> 
> Btw, the check was put back in the second patch. For the other patches,
> they're attached to the previous mail I just sent.
> 
> -- 
> Clément B.
> Not sent from a jesusPhone.

> From d50c977cf7c082e7ed39ab7a7355d5157c26d52e Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?Cl=C3=A9ment=20B=C5=93sch?= <ubitux at gmail.com>
> Date: Thu, 16 Dec 2010 21:24:48 +0100
> Subject: [PATCH 1/3] Factorize subtitles loading between mplayer and mencoder.
> 
> ---
>  mencoder.c      |   18 +-----------------
>  mplayer.c       |   16 +---------------
>  sub/subreader.c |   28 +++++++++++++++++++++++++++-
>  sub/subreader.h |    2 +-
>  4 files changed, 30 insertions(+), 34 deletions(-)
> 

Applied.

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