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

[Michael Niedermayer]

On Thu, Dec 02, 2010 at 05:13:51AM -0800, Jason Garrett-Glaser wrote:
> On Thu, Dec 2, 2010 at 4:53 AM, Michael Niedermayer <michaelni at gmx.at> wrote:
> > On Wed, Dec 01, 2010 at 08:41:38PM -0800, Baptiste Coudurier wrote:
> >> On 12/1/10 4:53 PM, Jason Garrett-Glaser wrote:
> >> > On Tue, Nov 30, 2010 at 2:26 PM, Stefano Sabatini
> >> > <stefano.sabatini-lala at poste.it> wrote:
> >> >> On date Saturday 2010-11-27 16:53:51 +0100, Stefano Sabatini encoded:
> >> >>> 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.
> >> >>
> >> >> Ping.
> >> >
> >> > I don't think this extra complexity is a good idea. ?I'd rather just
> >> > modify vflip to do things the slow way (it's NOT an important filter)
> >> > and just officially drop support for negative linesizes.
> >> >
> >> > This extra complexity would be worth it if it affected any use-case
> >> > that matters. ?It doesn't.
> >>
> >> Well, I tend to agree that I like the idea of dropping negative
> >> linesizes, it has always caused more problems than it fixed IMHO.
> >
> > libmpcodecs supports negative linesize
> > and code crashing with negative linesize is a bug even if we dont support
> > negative linesizes
> > also the bugs that have been mentioned above are said to be in codecs.
> > droping negative linesize in codecs requires a major bump and changes in
> > applications (mplayer at least).
> >
> > also while iam just mildly against droping negative linesizes iam very strongly
> > against brushing bugs under the carpet without understanding the bugs first
> > aka id like to know which codec crashes and id like to have backtraces and know
> > why before droping a random feature that someone claims is the cause of the
> > crash
> 
> Much assembly code doesn't support negative linesizes on 64-bit
> because it doesn't do a movsxd on the stride.

ok, that is an argument

though even if we drop negative linesize support some specific parts of code
will need to continue to support them, like the rotate/flip/transpose code or
some codecs which store images upside down.

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

it is not once nor twice but times without number that the same ideas make
their appearance in the world. -- Aristotle
-------------- 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/20101202/38db4a85/attachment.pgp>