[FFmpeg-devel] [PATCH 1/2] lavfi: use a new field for automatic buffer copy.

[Nicolas George]

The code currently use cur_buf as the target of the copy,
but cur_buf can be cleared by the filter if it has given
the reference away or stored it elsewhere as soon as start_frame.

The code still relies on the fact that the reference is not
destroyed until end_frame. All filters currently follow that condition.
An av_assert1() is added to check it; it should at least cause
very visible errors in valgrind.

Signed-off-by: Nicolas George <nicolas.george at normalesup.org>
---
 libavfilter/avfilter.h |    9 +++++++++
 libavfilter/video.c    |   20 ++++++++++++++++----
 2 files changed, 25 insertions(+), 4 deletions(-)

diff --git a/libavfilter/avfilter.h b/libavfilter/avfilter.h
index 91820dd..d256eeb 100644
--- a/libavfilter/avfilter.h
+++ b/libavfilter/avfilter.h
@@ -669,6 +669,15 @@ struct AVFilterLink {
      * called with more samples, it will split them.
      */
     int max_samples;
+
+    /**
+     * The buffer reference currently being received across the link by the
+     * destination filter. This is used internally by the filter system to
+     * allow automatic copying of buffers which do not have sufficient
+     * permissions for the destination. This should not be accessed directly
+     * by the filters.
+     */
+    AVFilterBufferRef *dst_buf;
 };
 
 /**
diff --git a/libavfilter/video.c b/libavfilter/video.c
index 92b16b2..6ee3315 100644
--- a/libavfilter/video.c
+++ b/libavfilter/video.c
@@ -20,6 +20,7 @@
  * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
  */
 
+#include "libavutil/avassert.h"
 #include "libavutil/imgutils.h"
 
 #include "avfilter.h"
@@ -187,6 +188,7 @@ static void clear_link(AVFilterLink *link)
     avfilter_unref_bufferp(&link->cur_buf);
     avfilter_unref_bufferp(&link->src_buf);
     avfilter_unref_bufferp(&link->out_buf);
+    link->dst_buf = NULL; /* we do not own the reference */
 }
 
 /* XXX: should we do the duplicating of the picture ref here, instead of
@@ -229,6 +231,8 @@ int ff_start_frame(AVFilterLink *link, AVFilterBufferRef *picref)
     else
         link->cur_buf = picref;
 
+    link->dst_buf = link->cur_buf;
+
     while(cmd && cmd->time <= picref->pts * av_q2d(link->time_base)){
         av_log(link->dst, AV_LOG_DEBUG,
                "Processing command time:%f command:%s arg:%s\n",
@@ -242,6 +246,10 @@ int ff_start_frame(AVFilterLink *link, AVFilterBufferRef *picref)
     ff_update_link_current_pts(link,link->cur_buf ?  link->cur_buf->pts : pts);
     if (ret < 0)
         clear_link(link);
+    else
+        /* incoming buffers must not be freed in start frame,
+           because they can still be in use by the automatic copy mechanism */
+        av_assert1(link->dst_buf->buf->refcount > 0);
 
     return ret;
 }
@@ -318,22 +326,22 @@ int ff_draw_slice(AVFilterLink *link, int y, int h, int slice_dir)
             if (link->src_buf->data[i]) {
                 src[i] = link->src_buf-> data[i] +
                     (y >> (i==1 || i==2 ? vsub : 0)) * link->src_buf-> linesize[i];
-                dst[i] = link->cur_buf->data[i] +
-                    (y >> (i==1 || i==2 ? vsub : 0)) * link->cur_buf->linesize[i];
+                dst[i] = link->dst_buf->data[i] +
+                    (y >> (i==1 || i==2 ? vsub : 0)) * link->dst_buf->linesize[i];
             } else
                 src[i] = dst[i] = NULL;
         }
 
         for (i = 0; i < 4; i++) {
             int planew =
-                av_image_get_linesize(link->format, link->cur_buf->video->w, i);
+                av_image_get_linesize(link->format, link->dst_buf->video->w, i);
 
             if (!src[i]) continue;
 
             for (j = 0; j < h >> (i==1 || i==2 ? vsub : 0); j++) {
                 memcpy(dst[i], src[i], planew);
                 src[i] += link->src_buf->linesize[i];
-                dst[i] += link->cur_buf->linesize[i];
+                dst[i] += link->dst_buf->linesize[i];
             }
         }
     }
@@ -343,6 +351,10 @@ int ff_draw_slice(AVFilterLink *link, int y, int h, int slice_dir)
     ret = draw_slice(link, y, h, slice_dir);
     if (ret < 0)
         clear_link(link);
+    else
+        /* incoming buffers must not be freed in start frame,
+           because they can still be in use by the automatic copy mechanism */
+        av_assert1(link->dst_buf->buf->refcount > 0);
     return ret;
 }
 
-- 
1.7.10.4