<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">2016-01-30 20:36 GMT+06:00 Rémi Denis-Courmont <span dir="ltr"><<a href="mailto:remi@remlab.net" target="_blank">remi@remlab.net</a>></span>:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div class=""><div class="h5">Le 2016-01-30 10:27, Sergey Radionov a écrit :<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
---<br>
 modules/video_output/vmem.c | 36 +++++++++++++++++++++++-------------<br>
 1 file changed, 23 insertions(+), 13 deletions(-)<br>
<br>
diff --git a/modules/video_output/vmem.c b/modules/video_output/vmem.c<br>
index 5a6d869..ce9584d 100644<br>
--- a/modules/video_output/vmem.c<br>
+++ b/modules/video_output/vmem.c<br>
@@ -3,6 +3,7 @@<br>
<br>
<br>
*****************************************************************************<br>
  * Copyright (C) 2008 VLC authors and VideoLAN<br>
  * Copyrgiht (C) 2010 Rémi Denis-Courmont<br>
+ * Copyrgiht (C) 2016 Sergey Radionov <<a href="mailto:rsatom@gmail.com" target="_blank">rsatom@gmail.com</a>><br>
  *<br>
  * Authors: Sam Hocevar <<a href="mailto:sam@zoy.org" target="_blank">sam@zoy.org</a>><br>
  *<br>
@@ -109,32 +110,37 @@ static void           Prepare(vout_display_t *,<br>
picture_t *, subpicture_t *);<br>
 static void           Display(vout_display_t *, picture_t *,<br>
subpicture_t *);<br>
 static int            Control(vout_display_t *, int, va_list);<br>
<br>
-static void Lock(void *data, picture_t *pic)<br>
+static int Lock(picture_t *pic)<br>
 {<br>
-    vout_display_sys_t *sys = data;<br>
     picture_sys_t *picsys = pic->p_sys;<br>
+    vout_display_sys_t *sys = picsys->sys;<br>
     void *planes[PICTURE_PLANE_MAX];<br>
+    memset(planes, 0, sizeof(planes));<br>
</blockquote>
<br></div></div>
memset()ing pointers does not make much sense.<span class=""><br></span></blockquote><div><br></div><div>it's just to have ability check if it was changed inside callback.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span class="">
<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
     picsys->id = sys->lock(sys->opaque, planes);<br>
<br>
+    for (int i = 0; i < pic->i_planes; ++i) {<br>
+        if (!planes[i])<br>
+            return VLC_EGENERIC;<br>
</blockquote>
<br></span>
Wut? Why?</blockquote><div><br></div><div>Sometimes memory buffer could be not available right on startup. But this not mandatory, and could be removed. Should I?<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div><div class="h5"><br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
+    }<br>
+<br>
     for (int i = 0; i < pic->i_planes; i++)<br>
         pic->p[i].p_pixels = planes[i];<br>
+<br>
+    return VLC_SUCCESS;<br>
 }<br>
<br>
-static void Unlock(void *data, picture_t *pic)<br>
+static void Unlock(picture_t *pic)<br>
 {<br>
-    vout_display_sys_t *sys = data;<br>
     picture_sys_t *picsys = pic->p_sys;<br>
+    vout_display_sys_t *sys = picsys->sys;<br>
     void *planes[PICTURE_PLANE_MAX];<br>
<br>
-    assert(!picture_IsReferenced(pic));<br>
-<br>
     for (int i = 0; i < pic->i_planes; i++)<br>
         planes[i] = pic->p[i].p_pixels;<br>
<br>
     if (sys->unlock != NULL)<br>
         sys->unlock(sys->opaque, picsys->id, planes);<br>
-<br>
 }<br>
<br>
<br>
<br>
/*****************************************************************************<br>
@@ -269,7 +275,6 @@ static void Close(vlc_object_t *object)<br>
<br>
     if (sys->pool)<br>
     {<br>
-        picture_pool_Enum(sys->pool, Unlock, sys);<br>
         picture_pool_Release(sys->pool);<br>
     }<br>
     free(sys);<br>
@@ -315,23 +320,29 @@ static picture_pool_t *Pool(vout_display_t *vd,<br>
unsigned count)<br>
     }<br>
<br>
     /* */<br>
-    sys->pool = picture_pool_New(count, pictures);<br>
+    picture_pool_configuration_t cfg = {<br>
+        .picture_count = count,<br>
+        .picture = pictures,<br>
+        .lock = Lock,<br>
+        .unlock = Unlock<br>
</blockquote>
<br></div></div>
lock and unlock callbacks are quite broken, and mostly unfixable or not worth fixing. I would rather not use them.</blockquote><div><br></div><div>Hm... but some modules use it:<br></div><div><a href="https://github.com/videolan/vlc/blob/master/modules/codec/omxil/vout.c#L318">https://github.com/videolan/vlc/blob/master/modules/codec/omxil/vout.c#L318</a><br><a href="https://github.com/videolan/vlc/blob/master/modules/video_output/directfb.c#L233">https://github.com/videolan/vlc/blob/master/modules/video_output/directfb.c#L233</a><br></div><div><br></div><div>Does it mean it broken too?<br><br></div><div>And does it mean you will not accept patch with lock/unlock callbacks?<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div class=""><div class="h5"><br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
+    };<br>
+<br>
+    sys->pool = picture_pool_NewExtended(&cfg);<br>
     if (!sys->pool) {<br>
         for (unsigned i = 0; i < count; i++)<br>
             picture_Release(pictures[i]);<br>
     }<br>
<br>
-    picture_pool_Enum(sys->pool, Lock, sys);<br>
     return sys->pool;<br>
 }<br>
<br>
 static void Prepare(vout_display_t *vd, picture_t *pic, subpicture_t<br>
*subpic)<br>
 {<br>
-    Unlock(vd->sys, pic);<br>
+    VLC_UNUSED(vd);<br>
+    VLC_UNUSED(pic);<br>
     VLC_UNUSED(subpic);<br>
 }<br>
<br>
-<br>
 static void Display(vout_display_t *vd, picture_t *pic, subpicture_t<br>
*subpic)<br>
 {<br>
     vout_display_sys_t *sys = vd->sys;<br>
@@ -339,7 +350,6 @@ static void Display(vout_display_t *vd, picture_t<br>
*pic, subpicture_t *subpic)<br>
     if (sys->display != NULL)<br>
         sys->display(sys->opaque, pic->p_sys->id);<br>
<br>
-    Lock(sys, pic);<br>
     picture_Release(pic);<br>
     VLC_UNUSED(subpic);<br>
 }<br>
</blockquote>
<br>
-- <br></div></div><span class=""><font color="#888888">
Rémi Denis-Courmont<br>
<a href="http://www.remlab.net/" rel="noreferrer" target="_blank">http://www.remlab.net/</a><br>
_______________________________________________<br>
vlc-devel mailing list<br>
To unsubscribe or modify your subscription options:<br>
<a href="https://mailman.videolan.org/listinfo/vlc-devel" rel="noreferrer" target="_blank">https://mailman.videolan.org/listinfo/vlc-devel</a><br>
</font></span></blockquote></div><br></div></div>