[MPlayer-dev-eng] [PATCH] Simplify and factorize EOSD code

Nicolas George nicolas.george at normalesup.org
Thu Feb 26 16:09:46 CET 2009


Hi.

L'octidi 8 ventôse, an CCXVII, Nicolas George a écrit :
> Anyways, I will try a more direct approach. Expect a new patch soon.

Here is a new version of my proposal. What this patch does:

- vo_gl and vo_vdpau do not change at all, I keep the control messages
  mechanism.

- In vf_ass and vf_vo, all calls to libass are wrapped in functions located
  in eosd.c. That makes a certain amount of "useless" code (almost forty
  lines), but helps keeping different things separate and similar things
  grouped. It also reduces a little code duplication (the sub_visibility
  stuff).

- The wrapper renderer function have been changed to be able to merge
  together the images list from several clients.

- The annoying bouncing ball is back.

This is not the clean design I was aiming at yesterday, but it does work and
allow to add new sources without too much trouble. And it does not change
the structure of the code paths at all, so it should not be too hard to
review.

Regards,

-- 
  Nicolas George
-------------- next part --------------
 Makefile             |    1 +
 eosd.c               |  217 ++++++++++++++++++++++++++++++++++++++++++++++++++
 eosd.h               |   67 +++++++++++++++
 libmpcodecs/vf_ass.c |   35 +++-----
 libmpcodecs/vf_vo.c  |   58 +++++--------
 5 files changed, 321 insertions(+), 57 deletions(-)

diff --git a/Makefile b/Makefile
index 449b058..fa52118 100644
--- a/Makefile
+++ b/Makefile
@@ -37,6 +37,7 @@ SRCS_COMMON = asxparser.c \
               codec-cfg.c \
               cpudetect.c \
               edl.c \
+              eosd.c \
               find_sub.c \
               fmt-conversion.c \
               get_path.c \
diff --git a/eosd.c b/eosd.c
new file mode 100644
index 0000000..1578358
--- /dev/null
+++ b/eosd.c
@@ -0,0 +1,217 @@
+/*
+ * EOSD subsystem
+ *
+ * Copyright (C) 2009 Nicolas George
+ *
+ * This file is part of MPlayer.
+ *
+ * MPlayer is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * MPlayer is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with MPlayer; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ */
+
+#include "eosd.h"
+#include "libass/ass_mp.h"
+#include "libvo/sub.h"
+#include "libmpcodecs/vf.h"
+#include "mpcommon.h"
+
+extern float sub_delay; /* urgh */
+
+/* List of EOSD clients */
+enum {
+    EOSDC_ASS,
+    EOSDC_BOUNCING,
+    EOSDC_NUM /* number of clients */
+};
+
+struct eosd_environment {
+    ass_renderer_t *ass_renderer;
+    ass_image_t *images[EOSDC_NUM];
+    ass_image_t *images_prev[EOSDC_NUM];
+    ass_image_t **images_tail[EOSDC_NUM];
+    int images_changed;
+    int coords_changed;
+};
+
+/* CIGAES -- ANNOYING BOUNCING BALL */
+
+#include "eosd.h"
+
+static int bouncing_ball_twidth, bouncing_ball_y;
+static ass_image_t bouncing_ball_image;
+
+static void
+bouncing_ball_configure(int width, int height)
+{
+    bouncing_ball_twidth = width - 16;
+    bouncing_ball_y = (height - 16) / 2;
+}
+
+static void
+bouncing_ball_render(eosd_environment_t *eosd, double pts)
+{
+    int x;
+
+    pts /= 8;
+    x = (pts - (int)pts) * bouncing_ball_twidth;
+    bouncing_ball_image.dst_x = x;
+    bouncing_ball_image.dst_y = bouncing_ball_y;
+    eosd->images[EOSDC_BOUNCING] = &bouncing_ball_image;
+    eosd->images_changed = 1;
+}
+
+static void
+bouncing_ball_init(void)
+{
+    int x, y, r;
+
+    bouncing_ball_image.w = 16;
+    bouncing_ball_image.h = 16;
+    bouncing_ball_image.stride = 16;
+    bouncing_ball_image.bitmap = malloc(16 * 16);
+    bouncing_ball_image.color = 0x00FFFF00;
+    bouncing_ball_image.next = NULL;
+    for(y = 0; y < 16; y++) {
+        for(x = 0; x < 16; x++) {
+            r = 255 * ((x - 7.5) * (x - 7.5) + (y - 7.5) * (y - 7.5)) / 56.25;
+            bouncing_ball_image.bitmap[x + y * 16] = r < 256 ? 255 - r : 0;
+        }
+    }
+}
+
+/* CIGAES -- ANNOYING BOUNCING BALL */
+
+static void render_ass(eosd_environment_t *eosd, double pts)
+{
+    int c;
+
+    if (eosd->ass_renderer) {
+        if (sub_visibility && ass_track && pts != MP_NOPTS_VALUE)
+            eosd->images[EOSDC_ASS] =
+                ass_mp_render_frame(eosd->ass_renderer, ass_track,
+                    (pts + sub_delay) * 1000 + .5, &c);
+        eosd->images_changed += (c >= 2);
+        eosd->coords_changed += (c >= 1);
+    } else {
+        eosd->images[EOSDC_ASS] = NULL;
+    }
+}
+
+void eosd_configure(eosd_environment_t *eosd, int width, int height, 
+                    int unscaled, double aspect_ratio,
+                    int margin_left, int margin_right,
+                    int margin_top, int margin_bottom)
+{
+    if (!eosd)
+        return;
+
+    /* EOSDC_ASS */
+    if (eosd->ass_renderer) {
+        ass_configure(eosd->ass_renderer, width, height, unscaled);
+        if (aspect_ratio)
+            ass_set_aspect_ratio(eosd->ass_renderer, aspect_ratio);
+        ass_set_margins(eosd->ass_renderer, margin_top, margin_bottom,
+            margin_left, margin_right);
+    }
+
+    /* EOSDC_BOUNCING */
+    bouncing_ball_configure(width, height);
+}
+
+void eosd_render_frame(eosd_environment_t *eosd, double pts,
+                       mp_eosd_images_t *images)
+{
+    int i;
+    ass_image_t **tail, *img;
+
+    images->imgs = NULL;
+    images->changed = 0;
+    eosd->images_changed = 0;
+    eosd->coords_changed = 0;
+
+    if (!eosd)
+        return;
+
+    render_ass(eosd, pts); /* EOSDC_ASS */
+    bouncing_ball_render(eosd, pts); /* EOSDC_BOUNCING */
+
+    /* Now that we have the images of all the clients, we concatenate all
+     * the lists.
+     * But the lists belong to each client, and some will use them again, so
+     * we will need to unconcatenate them when the rendering is done, so we
+     * keep a pointer to the 'next' link of the last element.
+     * This is an ugly hack; we probably will want to change it when we use
+     * a more generic data structure for the images.
+     */
+    tail = &images->imgs;
+    for (i = 0; i < EOSDC_NUM; i++) {
+        if (eosd->images[i] == NULL && eosd->images_prev[i] != NULL)
+            eosd->images_changed++;
+        eosd->images_prev[i] = eosd->images[i];
+        *tail = eosd->images[i];
+        for (img = eosd->images[i]; img != NULL; img = img->next)
+            tail = &img->next;
+        eosd->images_tail[i] = eosd->images[i] ? tail : NULL;
+    }
+    *tail = NULL;
+    images->changed = eosd->images_changed ? 2 : eosd->coords_changed ? 1 : 0;
+}
+
+void eosd_release_frame(eosd_environment_t *eosd)
+{
+    int i;
+
+    if (!eosd)
+        return;
+    /* Remember, we have kept a pointer to the last 'next' link of each
+     * list. Now is time to reset those links to NULL.
+     */
+    for (i = 0; i < EOSDC_NUM; i++)
+        if (eosd->images_tail[i])
+            *(eosd->images_tail[i]) = NULL;
+}
+
+eosd_environment_t *eosd_environment_init(ass_library_t *ass_library)
+{
+    eosd_environment_t *eosd;
+
+    eosd = calloc(1, sizeof(eosd_environment_t));
+    if (!eosd)
+        return NULL;
+
+    /* EOSDC_ASS */
+    eosd->ass_renderer = ass_renderer_init(ass_library);
+    if (eosd->ass_renderer)
+        ass_configure_fonts(eosd->ass_renderer);
+
+    /* EOSDC_BOUNCING */
+    bouncing_ball_init();
+
+    return eosd;
+}
+
+void eosd_environment_done(eosd_environment_t *eosd)
+{
+    if (!eosd)
+        return;
+
+    /* EOSDC_ASS */
+    if (eosd->ass_renderer)
+        ass_renderer_done(eosd->ass_renderer);
+
+    /* EOSDC_BOUNCING */
+    /* nothing */
+
+    free(eosd);
+}
diff --git a/eosd.h b/eosd.h
new file mode 100644
index 0000000..797b82a
--- /dev/null
+++ b/eosd.h
@@ -0,0 +1,67 @@
+/*
+ * EOSD subsystem
+ *
+ * Copyright (C) 2009 Nicolas George
+ *
+ * This file is part of MPlayer.
+ *
+ * MPlayer is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * MPlayer is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with MPlayer; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ */
+
+#ifndef MPLAYER_EOSD_H
+#define MPLAYER_EOSD_H
+
+#include "libass/ass.h"
+#include "libass/ass_mp.h"
+
+/**
+ * Abstract structure holding the status of the EOSD subsystem
+ */
+typedef struct eosd_environment eosd_environment_t;
+
+/**
+ * Creates a new EOSD environment
+ */
+eosd_environment_t *eosd_environment_init(ass_library_t *ass_library);
+
+/**
+ * Destroys an EOSD environment and all its elements
+ * If eosd is NULL, does nothing.
+ */
+void eosd_environment_done(eosd_environment_t *eosd);
+
+/**
+ * Configures an EOSD environment for the geometry of the driver
+ * If eosd is NULL, does nothing.
+ */
+void eosd_configure(eosd_environment_t *eosd, int width, int height, 
+                    int unscaled, double aspect_ratio,
+                    int margin_left, int margin_right,
+                    int margin_top, int margin_bottom);
+
+/**
+ * Renders a frame into the images structure
+ * If eosd is NULL, returns an empty list.
+ */
+void eosd_render_frame(eosd_environment_t *eosd, double pts,
+                       mp_eosd_images_t *images);
+
+/**
+ * Releases the images returned by eosd_render_frame
+ * If eosd is NULL, does nothing.
+ */
+void eosd_release_frame(eosd_environment_t *eosd);
+
+#endif /* MPLAYER_EOSD_H */
diff --git a/libmpcodecs/vf_ass.c b/libmpcodecs/vf_ass.c
index f33aba4..f84e110 100644
--- a/libmpcodecs/vf_ass.c
+++ b/libmpcodecs/vf_ass.c
@@ -41,7 +41,7 @@
 #include "m_option.h"
 #include "m_struct.h"
 
-#include "libass/ass.h"
+#include "eosd.h"
 #include "libass/ass_mp.h"
 
 #define _r(c)  ((c)>>24)
@@ -62,7 +62,7 @@ static const struct vf_priv_s {
 	// 0 = insert always
 	int auto_insert;
 
-	ass_renderer_t* ass_priv;
+	eosd_environment_t* eosd;
 
 	unsigned char* planes[3];
 	unsigned char* dirty_rows;
@@ -71,10 +71,6 @@ static const struct vf_priv_s {
 extern int opt_screen_size_x;
 extern int opt_screen_size_y;
 
-extern ass_track_t* ass_track;
-extern float sub_delay;
-extern int sub_visibility;
-
 static int config(struct vf_instance_s* vf,
 	int width, int height, int d_width, int d_height,
 	unsigned int flags, unsigned int outfmt)
@@ -93,10 +89,9 @@ static int config(struct vf_instance_s* vf,
 	vf->priv->planes[2] = malloc(vf->priv->outw * vf->priv->outh);
 	vf->priv->dirty_rows = malloc(vf->priv->outh);
 	
-	if (vf->priv->ass_priv) {
-		ass_configure(vf->priv->ass_priv, vf->priv->outw, vf->priv->outh, 0);
-		ass_set_aspect_ratio(vf->priv->ass_priv, ((double)d_width) / d_height);
-	}
+	eosd_configure(vf->priv->eosd, vf->priv->outw, vf->priv->outh, 0,
+		(double)d_width / d_height,
+		0, 0, ass_top_margin, ass_bottom_margin);
 
 	return vf_next_config(vf, vf->priv->outw, vf->priv->outh, d_width, d_height, flags, outfmt);
 }
@@ -326,12 +321,11 @@ static int render_frame(struct vf_instance_s* vf, mp_image_t *mpi, const ass_ima
 
 static int put_image(struct vf_instance_s* vf, mp_image_t *mpi, double pts)
 {
-	ass_image_t* images = 0;
-	if (sub_visibility && vf->priv->ass_priv && ass_track && (pts != MP_NOPTS_VALUE))
-		images = ass_mp_render_frame(vf->priv->ass_priv, ass_track, (pts+sub_delay) * 1000 + .5, NULL);
-	
+	mp_eosd_images_t images;
+	eosd_render_frame(vf->priv->eosd, pts, &images);
 	prepare_image(vf, mpi);
-	if (images) render_frame(vf, mpi, images);
+	if (images.imgs) render_frame(vf, mpi, images.imgs);
+	eosd_release_frame(vf->priv->eosd);
 
 	return vf_next_put_image(vf, vf->dmpi, pts);
 }
@@ -351,12 +345,11 @@ static int control(vf_instance_t *vf, int request, void *data)
 {
 	switch (request) {
 	case VFCTRL_INIT_EOSD:
-		vf->priv->ass_priv = ass_renderer_init((ass_library_t*)data);
-		if (!vf->priv->ass_priv) return CONTROL_FALSE;
-		ass_configure_fonts(vf->priv->ass_priv);
+		vf->priv->eosd = eosd_environment_init((ass_library_t*)data);
+		if (!vf->priv->eosd) return CONTROL_FALSE;
 		return CONTROL_TRUE;
 	case VFCTRL_DRAW_EOSD:
-		if (vf->priv->ass_priv) return CONTROL_TRUE;
+		if (vf->priv->eosd) return CONTROL_TRUE;
 		break;
 	}
 	return vf_next_control(vf, request, data);
@@ -364,8 +357,8 @@ static int control(vf_instance_t *vf, int request, void *data)
 
 static void uninit(struct vf_instance_s* vf)
 {
-	if (vf->priv->ass_priv)
-		ass_renderer_done(vf->priv->ass_priv);
+	if (vf->priv->eosd)
+		eosd_environment_done(vf->priv->eosd);
 	if (vf->priv->planes[1])
 		free(vf->priv->planes[1]);
 	if (vf->priv->planes[2])
diff --git a/libmpcodecs/vf_vo.c b/libmpcodecs/vf_vo.c
index affffa6..0639597 100644
--- a/libmpcodecs/vf_vo.c
+++ b/libmpcodecs/vf_vo.c
@@ -10,11 +10,7 @@
 
 #include "libvo/video_out.h"
 
-#ifdef CONFIG_ASS
-#include "libass/ass.h"
-#include "libass/ass_mp.h"
-extern ass_track_t* ass_track;
-#endif
+#include "eosd.h"
 
 //===========================================================================//
 
@@ -24,10 +20,8 @@ extern float sub_delay;
 struct vf_priv_s {
     double pts;
     const vo_functions_t *vo;
-#ifdef CONFIG_ASS
-    ass_renderer_t* ass_priv;
+    eosd_environment_t *eosd;
     int prev_visibility;
-#endif
 };
 #define video_out (vf->priv->vo)
 
@@ -68,8 +62,8 @@ static int config(struct vf_instance_s* vf,
 	return 0;
 
 #ifdef CONFIG_ASS
-    if (vf->priv->ass_priv)
-	ass_configure(vf->priv->ass_priv, width, height, !!(vf->default_caps & VFCAP_EOSD_UNSCALED));
+    eosd_configure(vf->priv->eosd, width, height,
+        !!(vf->default_caps & VFCAP_EOSD_UNSCALED), 0, 0, 0, 0, 0);
 #endif
 
     ++vo_config_count;
@@ -116,34 +110,29 @@ static int control(struct vf_instance_s* vf, int request, void* data)
 #ifdef CONFIG_ASS
     case VFCTRL_INIT_EOSD:
     {
-        vf->priv->ass_priv = ass_renderer_init((ass_library_t*)data);
-        if (!vf->priv->ass_priv) return CONTROL_FALSE;
-        ass_configure_fonts(vf->priv->ass_priv);
-        vf->priv->prev_visibility = 0;
+        vf->priv->eosd = eosd_environment_init((ass_library_t*)data);
+        if (!vf->priv->eosd) return CONTROL_FALSE;
         return CONTROL_TRUE;
     }
     case VFCTRL_DRAW_EOSD:
     {
         mp_eosd_images_t images = {NULL, 2};
         double pts = vf->priv->pts;
-        if (!vo_config_count || !vf->priv->ass_priv) return CONTROL_FALSE;
-        if (sub_visibility && vf->priv->ass_priv && ass_track && (pts != MP_NOPTS_VALUE)) {
-            mp_eosd_res_t res;
-            memset(&res, 0, sizeof(res));
-            if (video_out->control(VOCTRL_GET_EOSD_RES, &res) == VO_TRUE) {
-                ass_set_frame_size(vf->priv->ass_priv, res.w, res.h);
-                ass_set_margins(vf->priv->ass_priv, res.mt, res.mb, res.ml, res.mr);
-                ass_set_aspect_ratio(vf->priv->ass_priv, (double)res.w / res.h);
-            }
-
-            images.imgs = ass_mp_render_frame(vf->priv->ass_priv, ass_track, (pts+sub_delay) * 1000 + .5, &images.changed);
-            if (!vf->priv->prev_visibility)
-                images.changed = 2;
-            vf->priv->prev_visibility = 1;
-        } else
-            vf->priv->prev_visibility = 0;
-        vf->priv->prev_visibility = sub_visibility;
-        return (video_out->control(VOCTRL_DRAW_EOSD, &images) == VO_TRUE) ? CONTROL_TRUE : CONTROL_FALSE;
+        int r;
+        mp_eosd_res_t res;
+        if (!vo_config_count || !vf->priv->eosd) return CONTROL_FALSE;
+        memset(&res, 0, sizeof(res));
+        if (video_out->control(VOCTRL_GET_EOSD_RES, &res) == VO_TRUE) {
+            eosd_configure(vf->priv->eosd, res.w, res.h, 
+                !!(vf->default_caps & VFCAP_EOSD_UNSCALED),
+                (double)res.w / res.h,
+                res.ml, res.mr, res.mt, res.mb);
+        }
+
+        eosd_render_frame(vf->priv->eosd, pts, &images);
+        r = (video_out->control(VOCTRL_DRAW_EOSD, &images) == VO_TRUE) ? CONTROL_TRUE : CONTROL_FALSE;
+	eosd_release_frame(vf->priv->eosd);
+        return r;
     }
 #endif
     case VFCTRL_GET_PTS:
@@ -208,10 +197,7 @@ static void draw_slice(struct vf_instance_s* vf,
 static void uninit(struct vf_instance_s* vf)
 {
     if (vf->priv) {
-#ifdef CONFIG_ASS
-        if (vf->priv->ass_priv)
-            ass_renderer_done(vf->priv->ass_priv);
-#endif
+        eosd_environment_done(vf->priv->eosd);
         free(vf->priv);
     }
 }
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/mplayer-dev-eng/attachments/20090226/ea40d740/attachment.pgp>


More information about the MPlayer-dev-eng mailing list