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

[Michael Niedermayer]

On Fri, Dec 17, 2010 at 12:07:50PM +0100, Stefano Sabatini wrote:
> On date Wednesday 2010-12-15 13:22:24 +0100, Michael Niedermayer encoded:
> > On Tue, Dec 14, 2010 at 11:17:47PM +0100, Stefano Sabatini wrote:
> [...]
> > > Have another look at my last patch. You see that most code is in the
> > > vflip draw_slice() function. Now avfilter_draw_slice() has similar
> > > code which copies the input slice to the output slice when this is
> > > required for permission reasons.
> > > 
> > > Now we could try to move the vflipping code to
> > > avfilter_draw_slice(). Now this is a problem because this way
> > > avfilter_draw_slice() has to know *when* to vflip the frame, which is
> > > not required when the code is in the vflip filter.
> > 
> > the permission code in start_frame needs 1 line to be added for the current
> > copying code in draw_slice to copy the frame.
> > 
> > i dont understand the rest of your convoluted ideas.
> > 
> > 1. We need copying code if buffer permissions differ, this exists and i assume
> >    it works, if it doesnt work it needs to be fixed
> > 2. We need to copy if we have a negative linesize and a filter not supporting
> >    it
> > i see nothing in your mails which would even hint on why the permission copy
> > could not handle the case where linesizes differ
> 
> You're confusing different problems.

If so iam not seeing it, and i fail to be able to see your problem


>
> Here there is a list of some of the lavfi features related to the
> linesize sign invertion:
> 
> 1) having a filter only accept a buffer with non-negative linesizes in
>    get_video_buffer() (can be done if you don't set
>    AV_PERM_NEG_LINESIZES in perms, check ffplay.c)


> 
> 2) having a filter reject a buffer with positive linesizes in
>    start_frame(), and copy it to a newly allocated buffer (can be done
>    using again the permission system, check the posls test filter)

rejecting positive linesize, we dont want to do this, posls rejects
negative linesizes though


> 
> 3) having a filter explicitely request a buffer with negative
>    linesizes in get_video_buffer() (cannot be done, it may be done
>    adding another AV_PERM_POS_LINESIZES flag and changing the
>    semantics of AV_PERM_NEG_LINESIZES, but I don't think we want this
>    feature)

not wanted, agree


> 
> 4) having a filter reject a buffer with negative linesizes in
>    start_frame(), and copy it to a newly allocated buffer (cannot be
>    done, see above)

What are you talking about?
There may be a problem but you can copy a buffer whatever linesize in whatever
function


a few more comments about your code below:
[...]

> @@ -63,25 +68,65 @@ static AVFilterBufferRef *get_video_buffer(AVFilterLink *link, int perms,
>  static void start_frame(AVFilterLink *link, AVFilterBufferRef *picref)
>  {
>      FlipContext *flip = link->dst->priv;
> +    AVFilterBufferRef *picref2 = avfilter_ref_buffer(picref, ~0);
>      int i;
>  
> +    if (!(picref->perms & AV_PERM_NEG_LINESIZES)) {
> +        picref2->perms |= AV_PERM_NEG_LINESIZES;
> +        avfilter_default_start_frame(link, picref2);
> +        return;
> +    }
> +
>      for (i = 0; i < 4; i ++) {
>          int vsub = i == 1 || i == 2 ? flip->vsub : 0;
>  
> -        if (picref->data[i]) {
> -            picref->data[i] += ((link->h >> vsub)-1) * picref->linesize[i];
> -            picref->linesize[i] = -picref->linesize[i];
> +        if (picref2->data[i]) {
> +            picref2->data[i] += ((link->h >> vsub)-1) * picref->linesize[i];
> +            picref2->linesize[i] = -picref->linesize[i];
>          }
>      }
>  
> -    avfilter_start_frame(link->dst->outputs[0], picref);
> +    avfilter_start_frame(link->dst->outputs[0], picref2);
>  }

setting AVFilterBufferRef.perm to or not to AV_PERM_NEG_LINESIZES makes no
sense, its a piece of memory it can always be referenced both ways.


>  
> -static void draw_slice(AVFilterLink *link, int y, int h, int slice_dir)
> +static void draw_slice(AVFilterLink *inlink, int y, int h, int slice_dir)
>  {
> -    AVFilterContext *ctx = link->dst;
> +    AVFilterContext *ctx = inlink->dst;
> +    FlipContext *flip = ctx->priv;
> +    AVFilterLink *outlink = ctx->outputs[0];
> +
> +    if (!(inlink->cur_buf->perms & AV_PERM_NEG_LINESIZES)) {
> +        AVFilterBufferRef *inpic  = inlink ->cur_buf;
> +        AVFilterBufferRef *outpic = outlink->out_buf;
> +        uint8_t *inrow, *outrow;
> +        int plane, i;
> +
> +        /* vflip slice */
> +        for (plane = 0; plane < 4 && inpic->data[plane]; plane++) {
> +            int frame_h = plane == 1 || plane == 2 ? (inlink->h)>>flip->vsub : inlink->h;
> +            int y1      = plane == 1 || plane == 2 ? y>>flip->vsub : y;
> +            int h1      = plane == 1 || plane == 2 ? h>>flip->vsub : h;
> +
> +            inrow  = inpic ->data[plane] + inpic ->linesize[plane] * y1;
> +            outrow = outpic->data[plane] + outpic->linesize[plane] * (frame_h -y1-1);
> +
> +            for (i = 0; i < h1; i++) {
> +                memcpy(outrow, inrow, flip->linesizes[plane]);
> +                inrow  += inpic ->linesize[plane];
> +                outrow -= outpic->linesize[plane];
> +            }
> +        }
> +    }
> +
> +    avfilter_draw_slice(outlink, inlink->h - (y+h), h, -1 * slice_dir);
> +}

I dont know what you are trying to do here
Again, a buffer can be read only it can be write only but it cant be positve
linesizes only. one can always reference memory with negative linesizes



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

Those who are best at talking, realize last or never when they are wrong.
-------------- 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/20101218/da397bbf/attachment.pgp>