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

[Diego Biurrun]

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.

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.

> --- 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.


>  .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.

Diego