[FFmpeg-devel] [PATCH 1/2] Add libfribidi support to configure script

2014-07-09 Thread Marc Jeffreys
---
 configure | 22 ++
 1 file changed, 22 insertions(+)

diff --git a/configure b/configure
index e20bf8e..cb7d392 100755
--- a/configure
+++ b/configure
@@ -209,6 +209,7 @@ External library support:
   --enable-libfdk-aac  enable AAC de/encoding via libfdk-aac [no]
   --enable-libfliteenable flite (voice synthesis) support via libflite 
[no]
   --enable-libfreetype enable libfreetype [no]
+  --enable-libfribidi  enable libfribidi [no]
   --enable-libgme  enable Game Music Emu via libgme [no]
   --enable-libgsm  enable GSM de/encoding via libgsm [no]
   --enable-libiec61883 enable iec61883 via libiec61883 [no]
@@ -1215,6 +1216,25 @@ require_libfreetype(){
 add_extralibs $(get_safe ${pkg}_libs)
 }
 
+require_libfribidi(){
+log require_libfribidi "$@"
+pkg="fribidi"
+check_cmd $pkg_config --exists --print-errors $pkg \
+  || die "ERROR: $pkg not found"
+pkg_cflags=$($pkg_config --cflags $pkg_config_flags $pkg)
+pkg_libs=$($pkg_config --libs $pkg_config_flags $pkg)
+{
+echo "#include "
+echo "char* check_func(void) { return (char*) fribidi_version_info; }"
+echo "int main(void) { return 0; }"
+} | check_ld "cc" $pkg_cflags $pkg_libs \
+  && set_safe ${pkg}_cflags $pkg_cflags \
+  && set_safe ${pkg}_libs   $pkg_libs \
+  || die "ERROR: $pkg not found"
+add_cflags$(get_safe ${pkg}_cflags)
+add_extralibs $(get_safe ${pkg}_libs)
+}
+
 hostcc_e(){
 eval printf '%s\\n' $HOSTCC_E
 }
@@ -1332,6 +1352,7 @@ EXTERNAL_LIBRARY_LIST="
 libflite
 libfontconfig
 libfreetype
+libfribidi
 libgme
 libgsm
 libiec61883
@@ -4719,6 +4740,7 @@ enabled libflite  && require2 libflite 
"flite/flite.h" flite_init $flite
 enabled fontconfig&& enable libfontconfig
 enabled libfontconfig && require_pkg_config fontconfig 
"fontconfig/fontconfig.h" FcInit
 enabled libfreetype   && require_libfreetype
+enabled libfribidi&& require_libfribidi
 enabled libgme&& require  libgme gme/gme.h gme_new_emu -lgme 
-lstdc++
 enabled libgsm&& { for gsm_hdr in "gsm.h" "gsm/gsm.h"; do
check_lib "${gsm_hdr}" gsm_create -lgsm && 
break;
-- 
1.8.3.1

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


[FFmpeg-devel] [PATCH 2/2] drawtext: Use libfribidi to correctly render Arabic text - Fixes ticket #3758

2014-07-09 Thread Marc Jeffreys
I've added this as a "fribidi=1" option to drawtext rather than enabling it by 
default, so as not to break anything.

Difference can be seen by compiling with --enable-libfribidi and comparing:

ffplay -loglevel debug -f lavfi -i 
"color=color=white,drawtext=fontfile=/usr/share/fonts/dejavu/DejaVuSans.ttf:text=أحمد"

ffplay -loglevel debug -f lavfi -i 
"color=color=white,drawtext=fribidi=1:fontfile=/usr/share/fonts/dejavu/DejaVuSans.ttf:text=أحمد"

A slight note: This does create some -Wundef warnings on compilation, but 
they're from libfribidi itself rather than this code.
I opted to leave them alone rather than start defining macros just to keep 
things quiet, but if necessary that can be changed.

---
 doc/filters.texi  |   8 +++
 libavfilter/vf_drawtext.c | 137 --
 2 files changed, 139 insertions(+), 6 deletions(-)

diff --git a/doc/filters.texi b/doc/filters.texi
index ada33a7..a9e7360 100644
--- a/doc/filters.texi
+++ b/doc/filters.texi
@@ -3653,6 +3653,8 @@ To enable compilation of this filter, you need to 
configure FFmpeg with
 @code{--enable-libfreetype}.
 To enable default font fallback and the @var{font} option you need to
 configure FFmpeg with @code{--enable-libfontconfig}.
+To enable the @var{fribidi} option, you need to configure FFmpeg with
+@code{--enable-libfribidi}.
 
 @subsection Syntax
 
@@ -3707,6 +3709,9 @@ This parameter is mandatory if the fontconfig support is 
disabled.
 The font size to be used for drawing text.
 The default value of @var{fontsize} is 16.
 
+@item fribidi
+If set to 1, pass the text through libfribidi before rendering. By default 0.
+
 @item ft_load_flags
 The flags to be used for loading the fonts.
 
@@ -4010,6 +4015,9 @@ For more information about libfreetype, check:
 For more information about fontconfig, check:
 @url{http://freedesktop.org/software/fontconfig/fontconfig-user.html}.
 
+For more information about libfribidi, check:
+@url{http://fribidi.org/}.
+
 @section edgedetect
 
 Detect and draw edges. The filter uses the Canny Edge Detection algorithm.
diff --git a/libavfilter/vf_drawtext.c b/libavfilter/vf_drawtext.c
index 0d829a6..ecdc569 100644
--- a/libavfilter/vf_drawtext.c
+++ b/libavfilter/vf_drawtext.c
@@ -59,6 +59,10 @@
 #include "internal.h"
 #include "video.h"
 
+#if CONFIG_LIBFRIBIDI
+#include 
+#endif
+
 #include 
 #include FT_FREETYPE_H
 #include FT_GLYPH_H
@@ -182,6 +186,9 @@ typedef struct DrawTextContext {
 int tc24hmax;   ///< 1 if timecode is wrapped to 24 hours, 
0 otherwise
 int reload; ///< reload text file for each frame
 int start_number;   ///< starting frame number for n/frame_num 
var
+#if CONFIG_LIBFRIBIDI
+int fribidi;///< 1 if using fribidi
+#endif
 AVDictionary *metadata;
 } DrawTextContext;
 
@@ -226,6 +233,10 @@ static const AVOption drawtext_options[]= {
 {"fix_bounds", "if true, check and fix text coords to avoid clipping",  
OFFSET(fix_bounds), AV_OPT_TYPE_INT, {.i64=1}, 0, 1, FLAGS},
 {"start_number", "start frame number for n/frame_num variable", 
OFFSET(start_number), AV_OPT_TYPE_INT, {.i64=0}, 0, INT_MAX, FLAGS},
 
+#if CONFIG_LIBFRIBIDI
+{"fribidi", "pass text through libfribidi before rendering", 
OFFSET(fribidi), AV_OPT_TYPE_INT, {.i64=0}, 0, 1, FLAGS},
+#endif
+
 /* FT_LOAD_* flags */
 { "ft_load_flags", "set font loading flags for libfreetype", 
OFFSET(ft_load_flags), AV_OPT_TYPE_FLAGS, { .i64 = FT_LOAD_DEFAULT }, 0, 
INT_MAX, FLAGS, "ft_load_flags" },
 { "default", NULL, 0, AV_OPT_TYPE_CONST, { .i64 = 
FT_LOAD_DEFAULT }, .flags = FLAGS, .unit = "ft_load_flags" 
},
@@ -482,6 +493,113 @@ static int load_textfile(AVFilterContext *ctx)
 return 0;
 }
 
+static inline int is_newline(uint32_t c)
+{
+return c == '\n' || c == '\r' || c == '\f' || c == '\v';
+}
+
+#if CONFIG_LIBFRIBIDI
+static int fribidi_text(AVFilterContext *ctx)
+{
+DrawTextContext *s = ctx->priv;
+uint8_t *tmp;
+int ret = 0;
+static FriBidiFlags flags = FRIBIDI_FLAGS_DEFAULT   | \
+FRIBIDI_FLAGS_ARABIC;
+FriBidiChar *unicodestr = NULL;
+FriBidiStrIndex len;
+FriBidiParType direction = FRIBIDI_PAR_ON;
+FriBidiStrIndex line_start = 0;
+FriBidiStrIndex line_end = 0;
+FriBidiLevel *embedding_levels = NULL;
+FriBidiArabicProp *ar_props = NULL;
+FriBidiCharType *bidi_types = NULL;
+FriBidiStrIndex i,j;
+
+len = strlen(s->text);
+if (!(unicodestr = av_malloc(len * sizeof(*unicodestr {
+ret = AVERROR(ENOMEM);
+goto out;
+}
+len = fribidi_charset_to_unicode(FRIBIDI_CHAR_SET_UTF8,
+ s->text, len, unicodestr);
+
+bidi_types = av_malloc(len * sizeof(*bidi_types));
+if (!bidi_types) {
+ret = AVERROR(ENOMEM);
+goto out;
+}
+
+fribidi_get_bidi_types(un

Re: [FFmpeg-devel] [PATCH 2/2] drawtext: Use libfribidi to correctly render Arabic text - Fixes ticket #3758

2014-07-09 Thread Marc Jeffreys


Date: Wed, 9 Jul 2014 18:31:22 +0200
From: geo...@nsup.org
To: ffmpeg-devel@ffmpeg.org
Subject: Re: [FFmpeg-devel] [PATCH 2/2] drawtext: Use libfribidi to correctly 
render Arabic text - Fixes ticket #3758

> Do you have any idea about what it could break? IMHO, if ffmpeg is capable
> of correctly rendering Arabic text, it should do it by default.

The only case that comes to mind is if someone is reversing the arabic text
before sending it to ffmpeg, to counteract the existing issue. But you're
right; default-on would probably be better in the long-term.
 
> Also, I do not like the name: it is about the implementation and not the
> function. If we were to change the implementation (using another library),
> we would either have to change the option name or have an inconsistency.

Good point. I've been at a loss for names to to use, however. Perhaps, if
we default to using this, we could have something like "raw_text=1" to switch
the behaviour off.
 
> I think you should merge the change in configure with this patch.

I was following http://ffmpeg.org/developer.html#Development-Policy ,
point 5:

"Also do not forget that if part B depends on part A, but A does not
depend on B, then A can and should be committed first and separate from B."

Parts A and B being the configure patch and this patch, respectively.

But I'm happy to do a merge if necessary.
 
>> +@item fribidi
>> +If set to 1, pass the text through libfribidi before rendering. By default 
>> 0.
 
> I believe the doc should explain what it does to a normal, multimedia-savvy
> user, but not require knowledge of subtle free-software implementations of
> things that are only vaguely related to multimedia.

I'll see what I can do.
 
> Can you explain what scripts will be correctly rendered with this
> implementation?
>  
> If code has to be added for each script, then maybe using a more high-level
> library than fribidi would be better.


I'm not an expert in this field, but my understanding is:
 - All right-to-left/left-to-right reading issues should be resolved.
 - This won't fix any non-arabic scripts which require glyph substitution
  (But I don't think these are that common).
 - It may break some custom fonts (An ideal implementation would look at the
  Opentype layout tables in the font itself).

And yes, a better implementation might use something like harfbuzz,
but a small fix is perhaps better than none.  

>> +fribidi_shape(flags, embedding_levels, len, ar_props, unicodestr);
>> +
>> +for (line_end = 0, line_start = 0; line_end < len; line_end++) {
>> +if (is_newline(unicodestr[line_end]) || line_end == len - 1) {
>> +if (fribidi_reorder_line(flags, bidi_types, line_end - 
>> line_start,
>> + line_start, direction, 
>> embedding_levels,
>> + unicodestr, NULL) == 0) {
 
>> +ret = AVERROR(ENOMEM);
 
> According to the doc, ENOMEM is the most likely but not the only
> possibility. Can you elaborate?

Looking through the source of fribidi-0.19.6, I can't actually see a case in 
which
this function ever fails. So I suppose we should go by the documentation, just 
to be safe.
 
> Freeing NULL is legal, precisely in order to make these tests unnecessary.

Noted. I'll fix that.

I won't be able to make any more changes to my code until tomorrow morning, but 
I
can try to respond to any technical points. I'll resubmit as soon as I can.

  
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


[FFmpeg-devel] [PATCH v2] drawtext: Add basic text shaping using libfribidi - Fixes ticket #3758

2014-07-10 Thread Marc Jeffreys
Changes since last time:
I've made the changes to configure, and squashed the patches together.
Option changed from fribidi=1 (default 0) to text_shaping=1 (default 1).
(Ideas for better names are definitely welcome.)
Hopefully I've made the documentation more understandable.
No longer testing for NULL before av_free.

---
 configure |   3 ++
 doc/filters.texi  |  11 
 libavfilter/vf_drawtext.c | 130 +++---
 3 files changed, 138 insertions(+), 6 deletions(-)

diff --git a/configure b/configure
index 658efb2..6777d91 100755
--- a/configure
+++ b/configure
@@ -209,6 +209,7 @@ External library support:
   --enable-libfdk-aac  enable AAC de/encoding via libfdk-aac [no]
   --enable-libfliteenable flite (voice synthesis) support via libflite 
[no]
   --enable-libfreetype enable libfreetype [no]
+  --enable-libfribidi  enable libfribidi [no]
   --enable-libgme  enable Game Music Emu via libgme [no]
   --enable-libgsm  enable GSM de/encoding via libgsm [no]
   --enable-libiec61883 enable iec61883 via libiec61883 [no]
@@ -1332,6 +1333,7 @@ EXTERNAL_LIBRARY_LIST="
 libflite
 libfontconfig
 libfreetype
+libfribidi
 libgme
 libgsm
 libiec61883
@@ -4724,6 +4726,7 @@ enabled libflite  && require2 libflite 
"flite/flite.h" flite_init $flite
 enabled fontconfig&& enable libfontconfig
 enabled libfontconfig && require_pkg_config fontconfig 
"fontconfig/fontconfig.h" FcInit
 enabled libfreetype   && require_libfreetype
+enabled libfribidi&& require_pkg_config fribidi fribidi.h 
fribidi_version_info
 enabled libgme&& require  libgme gme/gme.h gme_new_emu -lgme 
-lstdc++
 enabled libgsm&& { for gsm_hdr in "gsm.h" "gsm/gsm.h"; do
check_lib "${gsm_hdr}" gsm_create -lgsm && 
break;
diff --git a/doc/filters.texi b/doc/filters.texi
index ada33a7..1b6f85e 100644
--- a/doc/filters.texi
+++ b/doc/filters.texi
@@ -3653,6 +3653,8 @@ To enable compilation of this filter, you need to 
configure FFmpeg with
 @code{--enable-libfreetype}.
 To enable default font fallback and the @var{font} option you need to
 configure FFmpeg with @code{--enable-libfontconfig}.
+To enable the @var{text_shaping} option, you need to configure FFmpeg with
+@code{--enable-libfribidi}.
 
 @subsection Syntax
 
@@ -3707,6 +3709,12 @@ This parameter is mandatory if the fontconfig support is 
disabled.
 The font size to be used for drawing text.
 The default value of @var{fontsize} is 16.
 
+@item text_shaping
+If set to 1, attempt to shape the text (for example, reverse the order of
+right-to-left text and join Arabic characters) before drawing it.
+Otherwise, just draw the text exactly as given.
+By default 1 (if supported).
+
 @item ft_load_flags
 The flags to be used for loading the fonts.
 
@@ -4010,6 +4018,9 @@ For more information about libfreetype, check:
 For more information about fontconfig, check:
 @url{http://freedesktop.org/software/fontconfig/fontconfig-user.html}.
 
+For more information about libfribidi, check:
+@url{http://fribidi.org/}.
+
 @section edgedetect
 
 Detect and draw edges. The filter uses the Canny Edge Detection algorithm.
diff --git a/libavfilter/vf_drawtext.c b/libavfilter/vf_drawtext.c
index 0d829a6..b29411e 100644
--- a/libavfilter/vf_drawtext.c
+++ b/libavfilter/vf_drawtext.c
@@ -59,6 +59,10 @@
 #include "internal.h"
 #include "video.h"
 
+#if CONFIG_LIBFRIBIDI
+#include 
+#endif
+
 #include 
 #include FT_FREETYPE_H
 #include FT_GLYPH_H
@@ -182,6 +186,9 @@ typedef struct DrawTextContext {
 int tc24hmax;   ///< 1 if timecode is wrapped to 24 hours, 
0 otherwise
 int reload; ///< reload text file for each frame
 int start_number;   ///< starting frame number for n/frame_num 
var
+#if CONFIG_LIBFRIBIDI
+int text_shaping;   ///< 1 to shape the text before drawing it
+#endif
 AVDictionary *metadata;
 } DrawTextContext;
 
@@ -226,6 +233,10 @@ static const AVOption drawtext_options[]= {
 {"fix_bounds", "if true, check and fix text coords to avoid clipping",  
OFFSET(fix_bounds), AV_OPT_TYPE_INT, {.i64=1}, 0, 1, FLAGS},
 {"start_number", "start frame number for n/frame_num variable", 
OFFSET(start_number), AV_OPT_TYPE_INT, {.i64=0}, 0, INT_MAX, FLAGS},
 
+#if CONFIG_LIBFRIBIDI
+{"text_shaping", "attempt to shape text before drawing", 
OFFSET(text_shaping), AV_OPT_TYPE_INT, {.i64=1}, 0, 1, FLAGS},
+#endif
+
 /* FT_LOAD_* flags */
 { "ft_load_flags", "set font loading flags for libfreetype", 
OFFSET(ft_load_flags), AV_OPT_TYPE_FLAGS, { .i64 = FT_LOAD_DEFAULT }, 0, 
INT_MAX, FLAGS, "ft_load_flags" },
 { "default", NULL, 0, AV_OPT_TYPE_CONST, { .i64 = 
FT_LOAD_DEFAULT }, .flags = FLAGS, .unit = "ft_load_flags" 
},
@@ -482,6 +493,106 @@ static int load_textfile(AVFilterCont

[FFmpeg-devel] [PATCH v3] drawtext: Add basic text shaping using libfribidi - Fixes ticket #3758

2014-07-12 Thread Marc Jeffreys
Regarding positioning of the '|' operator: Unless anyone objects, I think it's
fine being at the end of the line (if anything, that's the style used for '||'
in vf_drawtext.c). 

---8<---
---
 configure |3 +
 doc/filters.texi  |   11 
 libavfilter/vf_drawtext.c |  123 ++--
 3 files changed, 131 insertions(+), 6 deletions(-)

diff --git a/configure b/configure
index e39ecb9..2202272 100755
--- a/configure
+++ b/configure
@@ -209,6 +209,7 @@ External library support:
   --enable-libfdk-aac  enable AAC de/encoding via libfdk-aac [no]
   --enable-libfliteenable flite (voice synthesis) support via libflite 
[no]
   --enable-libfreetype enable libfreetype [no]
+  --enable-libfribidi  enable libfribidi [no]
   --enable-libgme  enable Game Music Emu via libgme [no]
   --enable-libgsm  enable GSM de/encoding via libgsm [no]
   --enable-libiec61883 enable iec61883 via libiec61883 [no]
@@ -1332,6 +1333,7 @@ EXTERNAL_LIBRARY_LIST="
 libflite
 libfontconfig
 libfreetype
+libfribidi
 libgme
 libgsm
 libiec61883
@@ -4724,6 +4726,7 @@ enabled libflite  && require2 libflite 
"flite/flite.h" flite_init $flite
 enabled fontconfig&& enable libfontconfig
 enabled libfontconfig && require_pkg_config fontconfig 
"fontconfig/fontconfig.h" FcInit
 enabled libfreetype   && require_libfreetype
+enabled libfribidi&& require_pkg_config fribidi fribidi.h 
fribidi_version_info
 enabled libgme&& require  libgme gme/gme.h gme_new_emu -lgme 
-lstdc++
 enabled libgsm&& { for gsm_hdr in "gsm.h" "gsm/gsm.h"; do
check_lib "${gsm_hdr}" gsm_create -lgsm && 
break;
diff --git a/doc/filters.texi b/doc/filters.texi
index b736b3f..6037e38 100644
--- a/doc/filters.texi
+++ b/doc/filters.texi
@@ -3653,6 +3653,8 @@ To enable compilation of this filter, you need to 
configure FFmpeg with
 @code{--enable-libfreetype}.
 To enable default font fallback and the @var{font} option you need to
 configure FFmpeg with @code{--enable-libfontconfig}.
+To enable the @var{text_shaping} option, you need to configure FFmpeg with
+@code{--enable-libfribidi}.
 
 @subsection Syntax
 
@@ -3707,6 +3709,12 @@ This parameter is mandatory if the fontconfig support is 
disabled.
 The font size to be used for drawing text.
 The default value of @var{fontsize} is 16.
 
+@item text_shaping
+If set to 1, attempt to shape the text (for example, reverse the order of
+right-to-left text and join Arabic characters) before drawing it.
+Otherwise, just draw the text exactly as given.
+By default 1 (if supported).
+
 @item ft_load_flags
 The flags to be used for loading the fonts.
 
@@ -4010,6 +4018,9 @@ For more information about libfreetype, check:
 For more information about fontconfig, check:
 @url{http://freedesktop.org/software/fontconfig/fontconfig-user.html}.
 
+For more information about libfribidi, check:
+@url{http://fribidi.org/}.
+
 @section edgedetect
 
 Detect and draw edges. The filter uses the Canny Edge Detection algorithm.
diff --git a/libavfilter/vf_drawtext.c b/libavfilter/vf_drawtext.c
index 0d829a6..0b93d14 100644
--- a/libavfilter/vf_drawtext.c
+++ b/libavfilter/vf_drawtext.c
@@ -59,6 +59,10 @@
 #include "internal.h"
 #include "video.h"
 
+#if CONFIG_LIBFRIBIDI
+#include 
+#endif
+
 #include 
 #include FT_FREETYPE_H
 #include FT_GLYPH_H
@@ -182,6 +186,9 @@ typedef struct DrawTextContext {
 int tc24hmax;   ///< 1 if timecode is wrapped to 24 hours, 
0 otherwise
 int reload; ///< reload text file for each frame
 int start_number;   ///< starting frame number for n/frame_num 
var
+#if CONFIG_LIBFRIBIDI
+int text_shaping;   ///< 1 to shape the text before drawing it
+#endif
 AVDictionary *metadata;
 } DrawTextContext;
 
@@ -226,6 +233,10 @@ static const AVOption drawtext_options[]= {
 {"fix_bounds", "if true, check and fix text coords to avoid clipping",  
OFFSET(fix_bounds), AV_OPT_TYPE_INT, {.i64=1}, 0, 1, FLAGS},
 {"start_number", "start frame number for n/frame_num variable", 
OFFSET(start_number), AV_OPT_TYPE_INT, {.i64=0}, 0, INT_MAX, FLAGS},
 
+#if CONFIG_LIBFRIBIDI
+{"text_shaping", "attempt to shape text before drawing", 
OFFSET(text_shaping), AV_OPT_TYPE_INT, {.i64=1}, 0, 1, FLAGS},
+#endif
+
 /* FT_LOAD_* flags */
 { "ft_load_flags", "set font loading flags for libfreetype", 
OFFSET(ft_load_flags), AV_OPT_TYPE_FLAGS, { .i64 = FT_LOAD_DEFAULT }, 0, 
INT_MAX, FLAGS, "ft_load_flags" },
 { "default", NULL, 0, AV_OPT_TYPE_CONST, { .i64 = 
FT_LOAD_DEFAULT }, .flags = FLAGS, .unit = "ft_load_flags" 
},
@@ -482,6 +493,99 @@ static int load_textfile(AVFilterContext *ctx)
 return 0;
 }
 
+static inline int is_newline(uint32_t c)
+{
+return c == '\n' || c == '\r' || c == '\f' || c == '\v