[MPlayer-dev-eng] [PATCH] FRIBIDI fix (subreader.c)

[Shachar Raindel]

On Fri, 30 Jul 2004 00:15:34 +0200, Diego Biurrun <diego at biurrun.de> wrote:
> 
> 
> Shachar Raindel writes:
> > On Tue, 20 Jul 2004 16:39:57 +0200, Diego Biurrun <diego at biurrun.de> wrote:
> > > attila writes:
> > > > On Thu, Jun 17, 2004 at 02:01:43PM +0200, Diego Biurrun wrote:
> > > > > Eliran Gonen writes:
> > > > > > Just to make things more clear:
> > > > > >
> > > > > > Most windows subtitles displayer doesn't show up hebrew subtitles correctly,
> > > > > > this makes hebrew subtitles writers to intentionally misplace commas and
> > > > > > dots
> > > > > > so they will display correctly on screen. Most hebrew subtitles are this
> > > > > > way and
> > > > > > the fix I just sent should "correct" it. Anyway, I'm planning to write
> > > > > > something that
> > > > > > will let the user himself decide wether the subtitle file is "correct"
> > > > > > (commas, dots, etc
> > > > > > placed correctly) or not, switching between :
> > > > > >
> > > > > >    base = FRIBIDI_TYPE_ON;
> > > > > >
> > > > > > and
> > > > > >
> > > > > >    base = FRIBIDI_TYPE_L;
> > > > > >
> > > > > > As soon as I will finish it, I will send a diff file. In the meantime
> > > > > > the above solution should
> > > > > > do it as most if not all hebrew subtitles are written that way.
> > > > >
> > > > > What happened to the patch you wanted to send along?
> > > >
> > > > Yes, what happend to this patch ?
> > >
> > > Just drop it.  He'll either send along a patch or never have his
> > > feature.
> > >
> > > Diego
> > >
> > Here is a new and updated patch, including man-page updates as well.
> > Now my commas are displayed in the correct (left) side of the
> > sentence...
> 
> Your patch does not apply cleanly, please redo it.
> 
Hopefully fixed (hasn't had time to check it, but this time it created
using a single diff run, instead of multiple runs as the previous was)
> Maybe you should also change the names of the options into
> -(no)flip-hebrew-commas, it's more consistent with -flip-hebrew.
> 
> >       {"noflip-hebrew", &flip_hebrew, CONF_TYPE_FLAG, 0, 1, 0, NULL},
> > +        {"flip-heb-commas", &fribidi_flip_commas, CONF_TYPE_FLAG, 0, 1, 0, NULL},
> > +        {"noflip-heb-commas", &fribidi_flip_commas, CONF_TYPE_FLAG, 0, 0, 1, NULL},
> 
> I have no idea why you are changing indentation here and I see no good
> reason.
> 

I didn't want to change indentation. maybe it is a \t vs. normal
spaces issue. Hopefully this patch doesn't change indentation. If it
does, please fix it.

> > --- mplayer.1 16 Oct 2003 00:09:55 -0000      1.454
> > +++ mplayer.1 29 Jul 2004 12:40:20 -0000
> > @@ -1108,6 +1108,11 @@
> >  .B \-flip_hebrew
> >  Turns on flipping subtitles using FriBiDi.
> >  .TP
> > +.B \-noflip-heb-commas
> > +Change FriBiDi's assumptions aboiut the placements of the commas in the
> > +subtitles. Use this if the commas in the subtitles are shown in the start of
> > +the sentence instead of in the end of it.
> 
> Start sentences on a new line in the manual page, you have a typo in
> aboIut and it should be "at the start".  Please also remove the
> trailing whitespace, like so:
> 
>   Change FriBiDi's assumptions about the placements of commas in subtitles.
>   Use this if commas in subtitles are shown at the start of a sentence
>   instead of at the end.
> 

Thanks. Fixed the text.

> >  .B \-font <path\ to\ font.desc\ file>
> >  Search for the OSD/\:SUB fonts in an alternative directory (default for normal
> >  fonts: ~/\:.mplayer/\:font/\:font.desc, default for FreeType fonts:
> > @@ -1133,7 +1138,7 @@
> >  Display only "forced subtitles" for the DVD subtitle stream selected by e.g.
> >  \-slang.
> >  .TP
> > -.B \-fribidi_charset <charset\ name>
> > +.B \-fribidi-charset <charset\ name>
> 
> This was changed ages ago, you seem to have an old version of the
> manual page.
> 
I was looking at an old man-page copy CVS had left in main/DOCS/en/ ,
instead of fixing main/DOCS/man/en/
> 
> 
> Diego

Thanks for the comments. I have encorperated your fixes into a newer
version of the patch, which I attach to this e-mail
-------------- next part --------------
A non-text attachment was scrubbed...
Name: heb-commas-2nd-try-raw.diff
Type: text/x-patch
Size: 3102 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/mplayer-dev-eng/attachments/20040730/c9fd9ae3/attachment.bin>