| Message ID | 20171130221146.15471-1-onemda@gmail.com |
|---|---|
| State | New |
| Headers | show |
Paul B Mahol (2017-11-30): > +static int reset_links(AVFilterContext *filter) > +{ > + int i, ret; > + > + if (!filter) > + return 0; > + > + for (i = 0; i < filter->nb_outputs; i++) { > + AVFilterLink *link = filter->outputs[i]; > + > + link->init_state = AVLINK_UNINIT; > + link->frame_rate.num = link->frame_rate.den = 0; > + link->w = link->h = 0; > + > + ret = reset_links(link->dst); > + if (ret < 0) > + return ret; > + } > + > + if (!i) > + return avfilter_config_links(filter); > + > + return 0; > +} Sorry, but no. All filters are currently written with the assumption that config_props is called only once. Not all of them actually rely on this assumption, but some do, and expect the fields of the private context to be in their initial state. Violating that assumption would result in a lot of memory leaks and probably a few crashes, some of them security relevant. Plus, the public API of buffersink does not document params changes and does not provide a good way of reacting to it, so enabling params changes like that would have problematic results on applications that expect frames with all the same size. (By the way, I am quite worried to see that Michael weakened the protections for that last point in 5d859e59809.) Filters reconfiguration has been wanted for a lot of time. If it was that simple, it would have been done already. If you want to implement it, please go ahead, but it will not be an easy task. Regards,
On 12/1/17, Nicolas George <george@nsup.org> wrote: > Paul B Mahol (2017-11-30): >> +static int reset_links(AVFilterContext *filter) >> +{ >> + int i, ret; >> + >> + if (!filter) >> + return 0; >> + >> + for (i = 0; i < filter->nb_outputs; i++) { >> + AVFilterLink *link = filter->outputs[i]; >> + >> + link->init_state = AVLINK_UNINIT; >> + link->frame_rate.num = link->frame_rate.den = 0; >> + link->w = link->h = 0; >> + >> + ret = reset_links(link->dst); >> + if (ret < 0) >> + return ret; >> + } >> + >> + if (!i) >> + return avfilter_config_links(filter); >> + >> + return 0; >> +} > > Sorry, but no. All filters are currently written with the assumption > that config_props is called only once. Not all of them actually rely on > this assumption, but some do, and expect the fields of the private > context to be in their initial state. Violating that assumption would > result in a lot of memory leaks and probably a few crashes, some of them > security relevant. Not all filters are written with such assumption. And I would like to fix those who rely on this assumption. > > Plus, the public API of buffersink does not document params changes and > does not provide a good way of reacting to it, so enabling params > changes like that would have problematic results on applications that > expect frames with all the same size. Well for that point one can always autoinsert scale filter if user does not set some flag when calling buffersink function. > > (By the way, I am quite worried to see that Michael weakened the > protections for that last point in 5d859e59809.) > > Filters reconfiguration has been wanted for a lot of time. If it was > that simple, it would have been done already. If you want to implement > it, please go ahead, but it will not be an easy task. I would like to proceed but I can't guess what is in your mind.
On Fri, 1 Dec 2017 10:35:34 +0100 Paul B Mahol <onemda@gmail.com> wrote: > On 12/1/17, Nicolas George <george@nsup.org> wrote: > > Paul B Mahol (2017-11-30): > >> +static int reset_links(AVFilterContext *filter) > >> +{ > >> + int i, ret; > >> + > >> + if (!filter) > >> + return 0; > >> + > >> + for (i = 0; i < filter->nb_outputs; i++) { > >> + AVFilterLink *link = filter->outputs[i]; > >> + > >> + link->init_state = AVLINK_UNINIT; > >> + link->frame_rate.num = link->frame_rate.den = 0; > >> + link->w = link->h = 0; > >> + > >> + ret = reset_links(link->dst); > >> + if (ret < 0) > >> + return ret; > >> + } > >> + > >> + if (!i) > >> + return avfilter_config_links(filter); > >> + > >> + return 0; > >> +} > > > > Sorry, but no. All filters are currently written with the assumption > > that config_props is called only once. Not all of them actually rely on > > this assumption, but some do, and expect the fields of the private > > context to be in their initial state. Violating that assumption would > > result in a lot of memory leaks and probably a few crashes, some of them > > security relevant. > > Not all filters are written with such assumption. And I would like to fix those > who rely on this assumption. SGTM > > > > Plus, the public API of buffersink does not document params changes and > > does not provide a good way of reacting to it, so enabling params > > changes like that would have problematic results on applications that > > expect frames with all the same size. > > Well for that point one can always autoinsert scale filter if user does > not set some flag when calling buffersink function. Also SGTM.
On Fri, Dec 01, 2017 at 10:01:42AM +0100, Nicolas George wrote: > Paul B Mahol (2017-11-30): > > +static int reset_links(AVFilterContext *filter) > > +{ > > + int i, ret; > > + > > + if (!filter) > > + return 0; > > + > > + for (i = 0; i < filter->nb_outputs; i++) { > > + AVFilterLink *link = filter->outputs[i]; > > + > > + link->init_state = AVLINK_UNINIT; > > + link->frame_rate.num = link->frame_rate.den = 0; > > + link->w = link->h = 0; > > + > > + ret = reset_links(link->dst); > > + if (ret < 0) > > + return ret; > > + } > > + > > + if (!i) > > + return avfilter_config_links(filter); > > + > > + return 0; > > +} > > Sorry, but no. All filters are currently written with the assumption > that config_props is called only once. Not all of them actually rely on > this assumption, but some do, and expect the fields of the private > context to be in their initial state. Violating that assumption would > result in a lot of memory leaks and probably a few crashes, some of them > security relevant. > > Plus, the public API of buffersink does not document params changes and > does not provide a good way of reacting to it, so enabling params > changes like that would have problematic results on applications that > expect frames with all the same size. > > (By the way, I am quite worried to see that Michael weakened the > protections for that last point in 5d859e59809.) > > Filters reconfiguration has been wanted for a lot of time. If it was > that simple, it would have been done already. If you want to implement > it, please go ahead, but it will not be an easy task. As the one who tried implementing changing frame parameters like dimension long time ago, the only real problem i encountered was bikeshedding, i dont rememer a technical problem. The bikeshedding in fact resolved itself but by the time i had lost interrest ... And in fact many filters should just be able to handle changing frame parameters as they are or with minimal changes. [...]
On Fri, 1 Dec 2017 18:02:52 +0100 Michael Niedermayer <michael@niedermayer.cc> wrote: > On Fri, Dec 01, 2017 at 10:01:42AM +0100, Nicolas George wrote: > > Paul B Mahol (2017-11-30): > > > +static int reset_links(AVFilterContext *filter) > > > +{ > > > + int i, ret; > > > + > > > + if (!filter) > > > + return 0; > > > + > > > + for (i = 0; i < filter->nb_outputs; i++) { > > > + AVFilterLink *link = filter->outputs[i]; > > > + > > > + link->init_state = AVLINK_UNINIT; > > > + link->frame_rate.num = link->frame_rate.den = 0; > > > + link->w = link->h = 0; > > > + > > > + ret = reset_links(link->dst); > > > + if (ret < 0) > > > + return ret; > > > + } > > > + > > > + if (!i) > > > + return avfilter_config_links(filter); > > > + > > > + return 0; > > > +} > > > > Sorry, but no. All filters are currently written with the assumption > > that config_props is called only once. Not all of them actually rely on > > this assumption, but some do, and expect the fields of the private > > context to be in their initial state. Violating that assumption would > > result in a lot of memory leaks and probably a few crashes, some of them > > security relevant. > > > > Plus, the public API of buffersink does not document params changes and > > does not provide a good way of reacting to it, so enabling params > > changes like that would have problematic results on applications that > > expect frames with all the same size. > > > > (By the way, I am quite worried to see that Michael weakened the > > protections for that last point in 5d859e59809.) > > > > > Filters reconfiguration has been wanted for a lot of time. If it was > > that simple, it would have been done already. If you want to implement > > it, please go ahead, but it will not be an easy task. > > As the one who tried implementing changing frame parameters like > dimension long time ago, the only real problem i encountered was > bikeshedding, i dont rememer a technical problem. The bikeshedding in > fact resolved itself but by the time i had lost interrest ... > > And in fact many filters should just be able to handle changing frame > parameters as they are or with minimal changes. Well, the result is that at least the vf_scale filter appears to blatantly violate the API and crashes my software.
On 12/1/17, wm4 <nfxjfg@googlemail.com> wrote: > On Fri, 1 Dec 2017 18:02:52 +0100 > Michael Niedermayer <michael@niedermayer.cc> wrote: > >> On Fri, Dec 01, 2017 at 10:01:42AM +0100, Nicolas George wrote: >> > Paul B Mahol (2017-11-30): >> > > +static int reset_links(AVFilterContext *filter) >> > > +{ >> > > + int i, ret; >> > > + >> > > + if (!filter) >> > > + return 0; >> > > + >> > > + for (i = 0; i < filter->nb_outputs; i++) { >> > > + AVFilterLink *link = filter->outputs[i]; >> > > + >> > > + link->init_state = AVLINK_UNINIT; >> > > + link->frame_rate.num = link->frame_rate.den = 0; >> > > + link->w = link->h = 0; >> > > + >> > > + ret = reset_links(link->dst); >> > > + if (ret < 0) >> > > + return ret; >> > > + } >> > > + >> > > + if (!i) >> > > + return avfilter_config_links(filter); >> > > + >> > > + return 0; >> > > +} >> > >> > Sorry, but no. All filters are currently written with the assumption >> > that config_props is called only once. Not all of them actually rely on >> > this assumption, but some do, and expect the fields of the private >> > context to be in their initial state. Violating that assumption would >> > result in a lot of memory leaks and probably a few crashes, some of them >> > security relevant. >> > >> > Plus, the public API of buffersink does not document params changes and >> > does not provide a good way of reacting to it, so enabling params >> > changes like that would have problematic results on applications that >> > expect frames with all the same size. >> > >> > (By the way, I am quite worried to see that Michael weakened the >> > protections for that last point in 5d859e59809.) >> > >> >> > Filters reconfiguration has been wanted for a lot of time. If it was >> > that simple, it would have been done already. If you want to implement >> > it, please go ahead, but it will not be an easy task. >> >> As the one who tried implementing changing frame parameters like >> dimension long time ago, the only real problem i encountered was >> bikeshedding, i dont rememer a technical problem. The bikeshedding in >> fact resolved itself but by the time i had lost interrest ... >> >> And in fact many filters should just be able to handle changing frame >> parameters as they are or with minimal changes. > > Well, the result is that at least the vf_scale filter appears to > blatantly violate the API and crashes my software. How? The current "support" works only with some filters and when input itself have parameter changes, using other filters in same filtergraph will just crash.
On Fri, Dec 01, 2017 at 06:10:02PM +0100, wm4 wrote: > On Fri, 1 Dec 2017 18:02:52 +0100 > Michael Niedermayer <michael@niedermayer.cc> wrote: > > > On Fri, Dec 01, 2017 at 10:01:42AM +0100, Nicolas George wrote: > > > Paul B Mahol (2017-11-30): > > > > +static int reset_links(AVFilterContext *filter) > > > > +{ > > > > + int i, ret; > > > > + > > > > + if (!filter) > > > > + return 0; > > > > + > > > > + for (i = 0; i < filter->nb_outputs; i++) { > > > > + AVFilterLink *link = filter->outputs[i]; > > > > + > > > > + link->init_state = AVLINK_UNINIT; > > > > + link->frame_rate.num = link->frame_rate.den = 0; > > > > + link->w = link->h = 0; > > > > + > > > > + ret = reset_links(link->dst); > > > > + if (ret < 0) > > > > + return ret; > > > > + } > > > > + > > > > + if (!i) > > > > + return avfilter_config_links(filter); > > > > + > > > > + return 0; > > > > +} > > > > > > Sorry, but no. All filters are currently written with the assumption > > > that config_props is called only once. Not all of them actually rely on > > > this assumption, but some do, and expect the fields of the private > > > context to be in their initial state. Violating that assumption would > > > result in a lot of memory leaks and probably a few crashes, some of them > > > security relevant. > > > > > > Plus, the public API of buffersink does not document params changes and > > > does not provide a good way of reacting to it, so enabling params > > > changes like that would have problematic results on applications that > > > expect frames with all the same size. > > > > > > (By the way, I am quite worried to see that Michael weakened the > > > protections for that last point in 5d859e59809.) > > > > > > > > Filters reconfiguration has been wanted for a lot of time. If it was > > > that simple, it would have been done already. If you want to implement > > > it, please go ahead, but it will not be an easy task. > > > > As the one who tried implementing changing frame parameters like > > dimension long time ago, the only real problem i encountered was > > bikeshedding, i dont rememer a technical problem. The bikeshedding in > > fact resolved itself but by the time i had lost interrest ... > > > > And in fact many filters should just be able to handle changing frame > > parameters as they are or with minimal changes. > > Well, the result is that at least the vf_scale filter appears to > blatantly violate the API and crashes my software. This sounds unlikely but Which ticket on trac is that ? And what part of which API do you speak about ? [...]
Michael Niedermayer (2017-12-01): > As the one who tried implementing changing frame parameters like > dimension long time ago, the only real problem i encountered was > bikeshedding, i dont rememer a technical problem. "not easy" != "tremendously hard". I said it would not be easy, i.e. it will require work and careful testing, but it is doable. But what you did was wrong, too simple, and results in our libraries outputting inconsistent data. No time to say more today. Regards,
On 12/1/17, Nicolas George <george@nsup.org> wrote: > Michael Niedermayer (2017-12-01): >> As the one who tried implementing changing frame parameters like >> dimension long time ago, the only real problem i encountered was >> bikeshedding, i dont rememer a technical problem. > > "not easy" != "tremendously hard". I said it would not be easy, i.e. it > will require work and careful testing, but it is doable. > > But what you did was wrong, too simple, and results in our libraries > outputting inconsistent data. > > No time to say more today. If you do not have "time" to speak than do not speak at all.
On Fri, Dec 01, 2017 at 07:23:00PM +0100, Nicolas George wrote: > Michael Niedermayer (2017-12-01): > > As the one who tried implementing changing frame parameters like > > dimension long time ago, the only real problem i encountered was > > bikeshedding, i dont rememer a technical problem. > > "not easy" != "tremendously hard". I said it would not be easy, i.e. it > will require work and careful testing, but it is doable. > > But what you did was wrong, too simple, and results in our libraries > outputting inconsistent data. Do you think this comment will fix any issue or improve anything ? First i do not even know what you speak of exactly and as this is 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 > > No time to say more today. > > Regards, > > -- > Nicolas George > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
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,
On Sun, Dec 03, 2017 at 08:21:50PM +0100, Nicolas George wrote: > 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. I think i may be missing something here The way i remember this, is that all the changes do never get triggered unless the user application passes frames with changing parameters into the filter graph. That is something not supported before these commits. So an application first has to introduce changes to get changes out or to get any of these failure modes. While before all this should have failed [...] > > 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. I think our difference here is entirely linguistic its a lot of work, and indeed as there are more filters now its more work than what it would have been in the past. But it is simple work (as in alot but it does not seem to be complicated work) > > 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. iam not sure naively the same approuch (flag & insert scale or reinit or do nothing) should work too. I do not think we need to rebuild parts of the filter graph totally reoptimizing for changed pixel formats but we could of course if thats something someone wants to implement [...]
Michael Niedermayer (2017-12-03): > I think i may be missing something here > > The way i remember this, is that all the changes do never get > triggered unless the user application passes frames with changing > parameters into the filter graph. > That is something not supported before these commits. So an application > first has to introduce changes to get changes out or to get any of > these failure modes. While before all this should have failed Yes, you are right. The problem is not as severe as I made it. But it is still a worry. These checks are there to protect from bugs. Or more precisely to protect from the consequences of bugs, and these changes weaken that protection. And there are bugs. Our very own src_movie does not check for formats changes in decoded frames. An application trusting it could write outside the frame data, and cause security issues. It would be better if it crashed on an assert failure, because assert failures, at the very least, are not exploitable. > iam not sure > naively the same approuch (flag & insert scale or reinit or do nothing) > should work too. > I do not think we need to rebuild parts of the filter graph totally > reoptimizing for changed pixel formats but we could of course if thats > something someone wants to implement That would be very suboptimal, but as long as somebody is willing to do the work properly, why not. Regards,
diff --git a/doc/filters.texi b/doc/filters.texi index 4a4efc70c8..e8da5e21a6 100644 --- a/doc/filters.texi +++ b/doc/filters.texi @@ -14057,6 +14057,10 @@ Set stereoscopic image format of input. Available values for input image formats are: @table @samp +@item auto +set input format using frame side data +If such data is not available filter will return unchanged input. + @item sbsl side by side parallel (left eye left, right eye right) diff --git a/libavfilter/avfilter.c b/libavfilter/avfilter.c index b98b32bacb..0d4bfd2671 100644 --- a/libavfilter/avfilter.c +++ b/libavfilter/avfilter.c @@ -378,6 +378,36 @@ int avfilter_config_links(AVFilterContext *filter) return 0; } +static int reset_links(AVFilterContext *filter) +{ + int i, ret; + + if (!filter) + return 0; + + for (i = 0; i < filter->nb_outputs; i++) { + AVFilterLink *link = filter->outputs[i]; + + link->init_state = AVLINK_UNINIT; + link->frame_rate.num = link->frame_rate.den = 0; + link->w = link->h = 0; + + ret = reset_links(link->dst); + if (ret < 0) + return ret; + } + + if (!i) + return avfilter_config_links(filter); + + return 0; +} + +int ff_reconfig_links(AVFilterContext *filter) +{ + return reset_links(filter); +} + void ff_tlog_link(void *ctx, AVFilterLink *link, int end) { if (link->type == AVMEDIA_TYPE_VIDEO) { diff --git a/libavfilter/internal.h b/libavfilter/internal.h index f9679ed1d7..5062bb7296 100644 --- a/libavfilter/internal.h +++ b/libavfilter/internal.h @@ -411,4 +411,6 @@ static inline int ff_norm_qscale(int qscale, int type) */ int ff_filter_get_nb_threads(AVFilterContext *ctx); +int ff_reconfig_links(AVFilterContext *ctx); + #endif /* AVFILTER_INTERNAL_H */ diff --git a/libavfilter/vf_stereo3d.c b/libavfilter/vf_stereo3d.c index 8b22f880ca..6665a2631b 100644 --- a/libavfilter/vf_stereo3d.c +++ b/libavfilter/vf_stereo3d.c @@ -25,6 +25,7 @@ #include "libavutil/opt.h" #include "libavutil/parseutils.h" #include "libavutil/pixdesc.h" +#include "libavutil/stereo3d.h" #include "avfilter.h" #include "drawutils.h" #include "formats.h" @@ -47,8 +48,8 @@ enum StereoCode { ANAGLYPH_YB_DUBOIS, // anaglyph yellow/blue dubois ANAGLYPH_RB_GRAY, // anaglyph red/blue gray ANAGLYPH_RG_GRAY, // anaglyph red/green gray - MONO_L, // mono output for debugging (left eye only) - MONO_R, // mono output for debugging (right eye only) + MONO_L, // mono output (left eye only) + MONO_R, // mono output (right eye only) INTERLEAVE_ROWS_LR, // row-interleave (left eye has top row) INTERLEAVE_ROWS_RL, // row-interleave (right eye has top row) SIDE_BY_SIDE_LR, // side by side parallel (left eye left, right eye right) @@ -66,7 +67,8 @@ enum StereoCode { INTERLEAVE_COLS_LR, // column-interleave (left eye first, right eye second) INTERLEAVE_COLS_RL, // column-interleave (right eye first, left eye second) HDMI, // HDMI frame pack (left eye first, right eye second) - STEREO_CODE_COUNT // TODO: needs autodetection + AUTO, // guess input format using frame's metadata. + STEREO_CODE_COUNT }; typedef struct StereoComponent { @@ -173,6 +175,7 @@ static const AVOption stereo3d_options[] = { { "irr", "interleave rows right first", 0, AV_OPT_TYPE_CONST, {.i64=INTERLEAVE_ROWS_RL}, 0, 0, FLAGS, "in" }, { "icl", "interleave columns left first", 0, AV_OPT_TYPE_CONST, {.i64=INTERLEAVE_COLS_LR}, 0, 0, FLAGS, "in" }, { "icr", "interleave columns right first", 0, AV_OPT_TYPE_CONST, {.i64=INTERLEAVE_COLS_RL}, 0, 0, FLAGS, "in" }, + { "auto", "guess using frame metadata", 0, AV_OPT_TYPE_CONST, {.i64=AUTO}, 0, 0, FLAGS, "in" }, { "out", "set output format", OFFSET(out.format), AV_OPT_TYPE_INT, {.i64=ANAGLYPH_RC_DUBOIS}, 0, STEREO_CODE_COUNT-1, FLAGS, "out"}, { "ab2l", "above below half height left first", 0, AV_OPT_TYPE_CONST, {.i64=ABOVE_BELOW_2_LR}, 0, 0, FLAGS, "out" }, { "ab2r", "above below half height right first", 0, AV_OPT_TYPE_CONST, {.i64=ABOVE_BELOW_2_RL}, 0, 0, FLAGS, "out" }, @@ -366,6 +369,9 @@ static int config_output(AVFilterLink *outlink) int ret; s->aspect = inlink->sample_aspect_ratio; + if (s->in.format == AUTO || (s->in.format == s->out.format)) + return 0; + switch (s->in.format) { case INTERLEAVE_COLS_LR: case INTERLEAVE_COLS_RL: @@ -670,6 +676,31 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *inpicref) int out_off_left[4], out_off_right[4]; int i, ret; + if (s->in.format == AUTO) { + AVFrameSideData *sd = av_frame_get_side_data(inpicref, AV_FRAME_DATA_STEREO3D); + AVFilterLink *link; + + if (sd) { + AVStereo3D *s3d = (AVStereo3D *)sd->data; + + switch (s3d->type) { + case AV_STEREO3D_SIDEBYSIDE: s->in.format = SIDE_BY_SIDE_LR; break; + case AV_STEREO3D_SIDEBYSIDE_QUINCUNX: s->in.format = SIDE_BY_SIDE_LR; break; + case AV_STEREO3D_TOPBOTTOM: s->in.format = ABOVE_BELOW_LR; break; + case AV_STEREO3D_CHECKERBOARD: s->in.format = CHECKERBOARD_LR; break; + case AV_STEREO3D_FRAMESEQUENCE: s->in.format = ALTERNATING_LR; break; + case AV_STEREO3D_LINES: s->in.format = INTERLEAVE_ROWS_LR; break; + case AV_STEREO3D_COLUMNS: s->in.format = INTERLEAVE_COLS_LR; break; + default: s->in.format = s->out.format = MONO_L; + }; + } else { + s->in.format = s->out.format = MONO_L; + } + + if ((ret = ff_reconfig_links(ctx)) < 0) + return ret; + } + if (s->in.format == s->out.format) return ff_filter_frame(outlink, inpicref);
Signed-off-by: Paul B Mahol <onemda@gmail.com> --- doc/filters.texi | 4 ++++ libavfilter/avfilter.c | 30 ++++++++++++++++++++++++++++++ libavfilter/internal.h | 2 ++ libavfilter/vf_stereo3d.c | 37 ++++++++++++++++++++++++++++++++++--- 4 files changed, 70 insertions(+), 3 deletions(-)