[FFmpeg-devel] [PATCH] avfilter/vf_stereo3d: implement auto detection by using frame side data

[Nicolas George]

Michael Niedermayer (2017-12-01):
> Do you think this comment will fix any issue or improve anything ?

This comment had one goal: prevent somebody from pushing incorrect
changes before I had time to make my point.

> First i do not even know what you speak of exactly and as this is

I posted the exact reference in my previous mail in this thread, which
is not very long:

https://ffmpeg.org/pipermail/ffmpeg-devel/2017-December/221480.html

> supposedly about code i wrote, i doubt others will have a better idea.
> 
> And there is possibly a difference in goals. Reinitializing filters
> would loose filter state, so avoiding reinit unless really needed was
> a big goal. Last but not least what i had envissioned is just in my head,
> i never implemeted it. (the bikeshedding made me loose
> interrest back then as i already mentioned) what is in git is a small
> part of it, that alone certainly is not that great.
> and "too simple" yes my vission of this was and is simple

I agree that avoiding resets is a worthy goal. I do not know if what you
had in your head was correct or not. But what you committed in
9225513242 (and related commits 5d2b885074, 6c702c3c63, 5d859e5980) was
just wrong. And one of the reason we can be sure it was wrong is that it
caused assert failures, that you did not fix but just hid with more of
the same.

But it is even worse than that: with the last of these commits, the
change can reach buffersink, and therefore the application. The
application will therefore get from buffersink frames with parameters
that do not match the parameters obtained from buffersink itself, it can
be considered a severe bug all by itself. It could even open security
issues for some applications.

What Paul proposes does not have that problem, since it changes the
parameters in buffersink, but it has never been documented that they
could change, and indeed our own source code is written as if they do
not. Therefore, it is not acceptable as is.

You are overly optimistic when saying it is simple. It will involve
quite a bit of work: checking filters to see if their code is safe to
call several times; probably use some kind of flag to see which one are
safe; inserting scale if necessary; providing an API to enable/disable
parameters changes on buffersink; providing an API to detect parameters
changes conveniently.

Not overwhelmingly difficult, but not an easy task either.

And this is only speaking of changes in resolution and other shallow
parameters. If you want to allow changes in pixel format (which your
commits do), you need to adapt the whole format negotiation.

Regards,

-- 
  Nicolas George
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20171203/1050f47a/attachment.sig>