Bug 467323 - adding BiDi support into VDR (with patch)
adding BiDi support into VDR (with patch)
Status: CLOSED WONTFIX
Product: Fedora
Classification: Fedora
Component: vdr (Show other bugs)
9
All Linux
medium Severity medium
: ---
: ---
Assigned To: Ville Skyttä
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2008-10-16 16:40 EDT by Muayyad Alsadi
Modified: 2008-10-25 04:31 EDT (History)
3 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2008-10-25 04:31:10 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)
add BiDi langugaes support to vdr (36.40 KB, patch)
2008-10-16 16:40 EDT, Muayyad Alsadi
no flags Details | Diff
modified vdr bidi patch (30.40 KB, patch)
2008-10-19 17:56 EDT, alrawab
no flags Details | Diff

  None (edit)
Description Muayyad Alsadi 2008-10-16 16:40:50 EDT
Created attachment 320608 [details]
add BiDi langugaes support to vdr

Description of problem:
Arabic language and derived scripts and sister Semitic languages needs BiDi support appear correctly as they are written from right not left

Version-Release number of selected component (if applicable):
1.6

How reproducible:
alway

Additional info:
I was told that the provided patch was upstream and will appear in next release
Comment 1 alrawab 2008-10-16 17:26:50 EDT
(In reply to comment #0)
> Created an attachment (id=320608) [details]
> add BiDi langugaes support to vdr
> 
> Description of problem:
> Arabic language and derived scripts and sister Semitic languages needs BiDi
> support appear correctly as they are written from right not left
> 
> Version-Release number of selected component (if applicable):
> 1.6
> 
> How reproducible:
> alway
> 
> Additional info:
> I was told that the provided patch was upstream and will appear in next release

>(In reply to comment #0)
> Created an attachment (id=320608) [details]
> add BiDi langugaes support to vdr
> 
> Description of problem:
> Arabic language and derived scripts and sister Semitic languages needs BiDi
> support appear correctly as they are written from right not left
> 
> Version-Release number of selected component (if applicable):
> 1.6
> 
> How reproducible:
> alway
> 
> Additional info:
> I was told that the provided patch was upstream and will appear in next release
>any one had been test this patch
>alrawab
Comment 2 Ville Skyttä 2008-10-17 14:34:06 EDT
Questions copied from bug 467433: How does compiling with BIDI affect non-Arabic/Semitic language support in VDR? Do they still work as they used to?

What about plugins, does this patch affect them in any way?
Comment 3 Muayyad Alsadi 2008-10-18 10:32:40 EDT
it's the same patch, created by alrawab in bug #467433 as he made it for 1.6 but it will be upstreamed for 1.7

This patch is not supposed to affect them in any way, except adding fribid dependency and adding a .po file, BiDi stands for Bidirectional ie. to support both ways it's not supposed to affect other languages, but please test it with other locales just to make sure.
Comment 4 Ville Skyttä 2008-10-19 12:15:15 EDT
I skimmed through the patch, and it seems to me that it needs some work:

1) It contains a lot of unnecessary whitespace/comment only changes.

2) It adds some apparently unused defines (LINE_LEN, max).

3) It makes an apparently entirely unrelated change to cChannels::Load()'s AllowComment default value.

4) It changes all default OSD fonts (normal, small, fixed width) to Arial:Bold if compiled with BIDI.  Why?

5) Would be better to use pkgconfig instead of hardcoding fribidi includes and libs.

6) It adds some weird looking includes to various files that are not touched otherwise (eg. tools.c).

7) Adding "ar" to the front of LanguageCodeList probably isn't a good idea, better would be to add it as the last item (before NULL).
Comment 5 Muayyad Alsadi 2008-10-19 13:31:57 EDT
> I skimmed through the patch, and it seems to me that it needs some work:
thanks, the patch developer should answer you

but I'll try to help,
regarding 
> 4) It changes all default OSD fonts (normal, small, fixed width) to Arial:Bold if compiled with BIDI.  Why?

+#ifdef BIDI
+const char *DefaultFontOsd = "Arial:Bold";
+const char *DefaultFontSml = "Arial:Bold";
+const char *DefaultFontFix = "Arial:Bold";
+#else
 const char *DefaultFontOsd = "Sans Serif:Bold";
 const char *DefaultFontSml = "Sans Serif";
 const char *DefaultFontFix = "Courier:Bold";
-
+#endif


maybe on his system, no good-looking Arabic font is aliased to provide the standard Sans,Sans Serif and monospace
in Fedora we have good configuration if DeJaVu fonts are installed
so this portion of the patch can be ignored but we need to replace
 const char *DefaultFontFix = "Courier:Bold";
with the standard monospace font

> 7) Adding "ar" to the front of LanguageCodeList probably isn't a good idea,
better would be to add it as the last item (before NULL).

 const char *LanguageCodeList[] = {
+  "ar", 
   "eng,dos",
   "deu,ger",
   "slv,slo",
guessing an encoding by trying a sequence of well-defined encodings like UTF-8, UTF-16, ...etc will be broken if sequence contains an encoding like ISO-8859-X or windows codepage 125X
maybe one of those CodeLists contains one of such encodings
if so, then such encodings should be removed from all languages CodeList and adding Arabic at the end, if that is not the case, and they can't be removed, then this should not be forced at compile time and allow users to add encoding at run time
Comment 6 alrawab 2008-10-19 17:56:25 EDT
Created attachment 320827 [details]
modified vdr bidi patch

 
this is a new modified patch againest vdr-1.6.0
1-remove all unnecessary comments and changes
2-edit i18.c and move Arabic to the end so its not affect other languages as well
plugins translations 
3-remove the part concerning default font 
4-lib fribidi don't affect other languages (I test Russian , Italian and English).
Br,
Sis
alrawab
Comment 7 Ville Skyttä 2008-10-22 14:50:20 EDT
Thanks, that looks much cleaner.  I have some other queued VDR changes but I'll have a look at testing this soon.

The only thing I noticed about the patch is that it still hardcodes these:

    INCLUDES += -I/usr/include/fribidi
    LIBS += -lfribidi

This is most likely no problem in Fedora, but I bet Klaus would appreciate something like this better:

    INCLUDES += $(shell pkg-config fribidi --cflags)
    LIBS += $(shell pkg-config fribidi --libs)

No need to submit a new patch for this, I'll make the change if/when applying the patch.
Comment 8 alrawab 2008-10-23 19:38:52 EDT
sorry this patch cause freeze for vdr and refused from the program author
because to enable fribidi you must change a lot of the original code those concerning epg , recording and other plugins .
sorry this patch is not valid and add bidi support just announces
alrawab
Comment 9 Ville Skyttä 2008-10-24 02:16:59 EDT
Ok, thanks for the report, but:

> sorry this patch is not valid and add bidi support just announces

I'm afraid I don't understand what you mean by "and add bidi support just announces", could you rephrase?
Comment 10 alrawab 2008-10-24 06:42:39 EDT
hey
I'm sorry dude i meant nonsense .
To be realistic to do bidi in vdr we must do :
1-Reverse the direction of all menus
2-this will be dramatically affect the code (Reinvent the wheel).
3- fribidi is a for light jobs such translation of subtitles (such in mplayer and xbmc) but not for whole code.
4-sorry for disturbing you and i hope one of Arabic programmers do program like vdr support theire native language .
Br,
Sis
Dr alrawab
Comment 11 Ville Skyttä 2008-10-25 04:31:10 EDT
Ok, thanks for the info, closing as WONTFIX then.

Note You need to log in before you can comment on or make changes to this bug.