[FFmpeg-devel] [RFC] How to fix DR+lavfi+vflip crash

[Michael Niedermayer]

On Sun, Dec 26, 2010 at 01:51:21PM +0100, Michael Niedermayer wrote:
> On Sun, Dec 26, 2010 at 11:41:57AM +0100, Stefano Sabatini wrote:
> > On date Saturday 2010-12-25 02:22:48 +0100, Stefano Sabatini encoded:
> > > On date Thursday 2010-12-23 11:14:26 +0100, Stefano Sabatini encoded:
> > [...]
> > > > > > From ee6dc086923ed994a5dd70ab79513d9a510c29ce Mon Sep 17 00:00:00 2001
> > > > > > From: Stefano Sabatini <stefano.sabatini-lala at poste.it>
> > > > > > Date: Sun, 19 Dec 2010 16:35:50 +0100
> > > > > > Subject: [PATCH] Introduce the AV_PERM_NEG_LINESIZES flag, which allows to explicitely
> > > > > >  request a buffer which can have negative linesizes, so that if it is
> > > > > >  not specified in the permissions for the requested buffer the returned
> > > > > >  buffer must have positive linesizes.
> > > > > > 
> > > > > > This is required in particular with direct rendering as implemented in
> > > > > > ffplay, as some codec cannot process an image buffer with negative
> > > > > > linesizes.
> > > > > > 
> > > > > > Fix issue #1913.
> > > > > 
> > > > > Ping.
> > > > 
> > > > I'll apply in few days if I see no comments.
> > > 
> > > I'll apply in two days, Michael?
> > 
> > I'm going to apply tomorrow.
> 
> wait please

Heres a counterproposal to your patch, please tell me with what filter chain
this fails for you
Also keep in mind that if you have filters that cannot handle negative linesizes
they must declare this their rej_perm


diff --git a/ffplay.c b/ffplay.c
index 38a2fe1..df6bcbd 100644
--- a/ffplay.c
+++ b/ffplay.c
@@ -1616,6 +1616,9 @@ static int input_get_buffer(AVCodecContext *codec, AVFrame *pic)
     int i, w, h, stride[4];
     unsigned edge;

+    if (codec->codec->capabilities & CODEC_CAP_NEG_LINESIZES)
+        perms |= AV_PERM_NEG_LINESIZES;
+
     if(pic->buffer_hints & FF_BUFFER_HINTS_VALID) {
         if(pic->buffer_hints & FF_BUFFER_HINTS_READABLE) perms |= AV_PERM_READ;
         if(pic->buffer_hints & FF_BUFFER_HINTS_PRESERVE) perms |= AV_PERM_PRESERVE;
diff --git a/libavfilter/avfilter.c b/libavfilter/avfilter.c
index 362d537..a86cdde 100644
--- a/libavfilter/avfilter.c
+++ b/libavfilter/avfilter.c
@@ -197,12 +197,13 @@ int avfilter_config_links(AVFilterContext *filter)

 char *ff_get_ref_perms_string(char *buf, size_t buf_size, int perms)
 {
-    snprintf(buf, buf_size, "%s%s%s%s%s",
+    snprintf(buf, buf_size, "%s%s%s%s%s%s",
              perms & AV_PERM_READ      ? "r" : "",
              perms & AV_PERM_WRITE     ? "w" : "",
              perms & AV_PERM_PRESERVE  ? "p" : "",
              perms & AV_PERM_REUSE     ? "u" : "",
-             perms & AV_PERM_REUSE2    ? "U" : "");
+             perms & AV_PERM_REUSE2    ? "U" : "",
+             perms & AV_PERM_NEG_LINESIZES ? "n" : "");
     return buf;
 }

@@ -360,16 +361,18 @@ void avfilter_start_frame(AVFilterLink *link, AVFilterBufferRef *picref)
 {
     void (*start_frame)(AVFilterLink *, AVFilterBufferRef *);
     AVFilterPad *dst = link->dstpad;
+    int perms = picref->perms;

     FF_DPRINTF_START(NULL, start_frame); ff_dprintf_link(NULL, link, 0); dprintf(NULL, " "); ff_dprintf_ref(NULL, picref, 1);

     if (!(start_frame = dst->start_frame))
         start_frame = avfilter_default_start_frame;

+    if (picref->linesize[0] < 0)
+        perms |= AV_PERM_NEG_LINESIZES;
     /* prepare to copy the picture if it has insufficient permissions */
-    if ((dst->min_perms & picref->perms) != dst->min_perms ||
-         dst->rej_perms & picref->perms) {
-        av_log(link->dst, AV_LOG_DEBUG,
+    if ((dst->min_perms & perms) != dst->min_perms || dst->rej_perms & perms) {
+        av_log(link->dst, AV_LOG_ERROR,
                 "frame copy needed (have perms %x, need %x, reject %x)\n",
                 picref->perms,
                 link->dstpad->min_perms, link->dstpad->rej_perms);
diff --git a/libavfilter/avfilter.h b/libavfilter/avfilter.h
index 01cda12..0a88d96 100644
--- a/libavfilter/avfilter.h
+++ b/libavfilter/avfilter.h
@@ -87,6 +87,7 @@ typedef struct AVFilterBuffer {
 #define AV_PERM_PRESERVE 0x04   ///< nobody else can overwrite the buffer
 #define AV_PERM_REUSE    0x08   ///< can output the buffer multiple times, with the same contents each time
 #define AV_PERM_REUSE2   0x10   ///< can output the buffer multiple times, modified each time
+#define AV_PERM_NEG_LINESIZES 0x20  ///< the buffer requested can have negative linesizes

 /**
  * Audio specific properties in a reference to an AVFilterBuffer. Since
diff --git a/libavfilter/vf_vflip.c b/libavfilter/vf_vflip.c
index 2ff4441..e5cede8 100644
--- a/libavfilter/vf_vflip.c
+++ b/libavfilter/vf_vflip.c
@@ -43,11 +43,13 @@ static AVFilterBufferRef *get_video_buffer(AVFilterLink *link, int perms,
                                         int w, int h)
 {
     FlipContext *flip = link->dst->priv;
+    AVFilterBufferRef *picref;
     int i;

-    AVFilterBufferRef *picref = avfilter_get_video_buffer(link->dst->outputs[0],
-                                                       perms, w, h);
+    if (!(perms & AV_PERM_NEG_LINESIZES))
+        return avfilter_default_get_video_buffer(link, perms, w, h);

+    picref = avfilter_get_video_buffer(link->dst->outputs[0], perms, w, h);
     for (i = 0; i < 4; i ++) {
         int vsub = i == 1 || i == 2 ? flip->vsub : 0;

[...]
--
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

If you really think that XML is the answer, then you definitly missunderstood
the question -- Attila Kinali
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20101227/4358faa9/attachment.pgp>