[FFmpeg-devel] [RFC] How to fix DR+lavfi+vflip crash
Michael Niedermayer
michaelni
Sat Dec 18 14:07:47 CET 2010
On Sat, Dec 18, 2010 at 11:46:16AM +0100, Stefano Sabatini wrote:
> On date Saturday 2010-12-18 02:29:04 +0100, Michael Niedermayer encoded:
> > 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
>
> I could say the same thing, that is I still can't try to understand
> what's your problem with my implementation...
To me it looks like its bigger than needed and messy if not buggy
>
> > > 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
> ^^^^^^^^
> Sorry I meant -> negative
ok, so your mail starts to make more sense to me
>
> > > 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
>
> yes, as implemented in posls
>
> > >
> > > 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
> ^^^^^^^^
>
> Here I meant positive (I messed up something after the many re-edits).
>
> > > 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
>
> Yes but with the current scheme you can't make a filter reject a
> buffer with positive linesizes (that is you cannot implement negls),
> and again I don't think this is a feature that we want to implement.
yes, so far so good, makes sense and i agree, we dont want that
>
> > 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.
>
> Let's consider this scenario:
>
> pos_src ----> vflip
>
> pos_src requires a buffer with positive linesizes. When a buffer is
> requested to vflip it won't reverse the linesizes and will allocate a
> new buffer and which will have positive linesizes (as in general when
> requesting a buffer to the filterchain you have no guarantee that the
> returned buffer will have positive linesizes).
ok so far
>
> Then pos_src invokes start_frame(). Now vflip has to somehow knows if
> it can revert the linesizes, or if the buffer was the same one
> previously allocated by the vflip filter itself.
No, vflip flips the linesize.
This fliped buffer is passed on by start_frame to the next filter and if that
doesnt support it the existing code copies it
The previous filter is not affected by start_frame messing with line sizes,
It cant be affected because that would be a bug. A filter can have a static
buffer that it passes on and it can alraedy have rendered into this buffer
theres no way start_frame could tell the previous filter render fliped at that
time. Only though get_video_buffer() can teh previous filter be brought to
render upside down.
Thus if the previous filter uses get_video_buffer() it receives frames with
negative linesize (if supported) and start_frame then also receives these
buffers with negative linesize and it changes them to positive by fliping
linesize and vissual image
If OTOH the previous filter does not use get_video_buffer() or negative
linesizes arent supported somewhere along the path then start_frame gets
positive linesize frames and flips them to negative and (its the identical
operation as previous) if now a downstream filter cant handle negative linesizes
it gets copied before in that filters start_frame
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
I am the wisest man alive, for I know one thing, and that is that I know
nothing. -- Socrates
-------------- 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/ee6a0496/attachment.pgp>
More information about the ffmpeg-devel
mailing list