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

[Michael Niedermayer]

On Fri, Dec 03, 2010 at 04:00:59PM +0100, Stefano Sabatini wrote:
> On date Friday 2010-12-03 00:03:15 +0100, Michael Niedermayer encoded:
> > On Sat, Nov 27, 2010 at 04:53:51PM +0100, Stefano Sabatini wrote:
> > > On date Saturday 2010-11-06 18:21:55 +0100, Stefano Sabatini encoded:
> > > > On date Saturday 2010-11-06 18:10:04 +0100, Stefano Sabatini encoded:
> > > > > Hi,
> > > > > 
> > > > > as you may know the command:
> > > > > ffplay INPUT -vf vflip
> > > > > 
> > > > > crashes with many video codecs. This is a regression introduced by the
> > > > > direct rendering feature, since the codec request the frame to use for
> > > > > putting the decoded frame, it gets a frame with negative linesizes and
> > > > > crash
> > > > > 
> > > > > (BTW the smacker regression also seems to depend on diect
> > > > > rendering, and precisely with the way the palette is initialized in
> > > > > avfilter_default_get_buffer, which doesn't use ff_systematic_pal()).
> > > > > 
> > > > > A possible first step would be to define a CODEC_CAP_NEG_LINESIZES
> > > > > capability (suggest a better name), and set it in all the codecs which
> > > > > currently support this feature (I have no idea which of them, do
> > > > > you?).
> > > > > 
> > > > > At this point I see two solutions. One solution would be to change
> > > > > get_video_buffer(), and make it invert the buffer when it detects the
> > > > > negative linesizes && the NEG_LINESIZES capability is not
> > > > > supported, *or* auto-add another filter just before the ffplay source.
> > > > > 
> > > > > Such a filter (vflipfix - suggest better name) would work as a null
> > > > > filter if the frame is not inverted, and would readjust the frame if
> > > > > the linesizes are inverted.
> > > > > 
> > > > > The second solution seems simpler and cleaner.
> > > > 
> > > > To make it even more useful, we may add a capability to the filters,
> > > > and auto-add the vflip-fix filter when building the filterchain, and
> > > > fix all the filters which doesn't support negative linesizes.
> > > 
> > > Patchset attached.
> > > -- 
> > > FFmpeg = Frenzy and Fiendish Magic Plastic Ermetic Game
> > 
> > >  avcodec.h |    5 ++++-
> > >  1 file changed, 4 insertions(+), 1 deletion(-)
> > > 634953e7072d997016e9be769086497234f7ec8c  0001-Add-CODEC_CAP_NEG_LINESIZES.patch
> > > From da3bf16f9054f619966ddec6fb27a8aa375ad423 Mon Sep 17 00:00:00 2001
> > > From: Stefano Sabatini <stefano.sabatini-lala at poste.it>
> > > Date: Sat, 6 Nov 2010 17:05:02 +0100
> > > Subject: [PATCH] Add CODEC_CAP_NEG_LINESIZES.
> > 
> > lgtm
> > 
> > 
> > 
> > [...]
> > >  Makefile      |    1 
> > >  allfilters.c  |    1 
> > >  vf_vflipfix.c |  132 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> > >  3 files changed, 134 insertions(+)
> > > 7cbd3ac2ed85f675c6a14bdeaec9af60fd0e5d89  0003-Add-vflipfix-filter.patch
> > > From 4ef5f32725492b2ea4149e6f4aba497fad544739 Mon Sep 17 00:00:00 2001
> > > From: Stefano Sabatini <stefano.sabatini-lala at poste.it>
> > > Date: Sat, 6 Nov 2010 20:45:38 +0100
> > > Subject: [PATCH] Add vflipfix filter.
> > 
> > isnt it easier to handle (lack) of neg linesize support by using the premission
> > system?
> > like "i need a readable buffer" vs. "i need a buffer with positive linesize"
> 
> If this information is not stored in the filtergraph structure then
> the framework has to know where the flipping needs to be done and do
> it rather than delegating this to some auto-inserted filter, which is
> bad imo since this is complicating even further the framework.

the auto inserted filter approuch is insane
we have code in place that copies a frame if its not writeable (for example).
negative linesize needs exactly this existing copy code.
Thats like having to add 1 line of code vs 130+ lines for the filter + i dont
know what for the autoinsert


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

When the tyrant has disposed of foreign enemies by conquest or treaty, and
there is nothing more to fear from them, then he is always stirring up
some war or other, in order that the people may require a leader. -- Plato
-------------- 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/20101203/f3a7b44d/attachment.pgp>