| Message ID | 7a283959-c7c1-8887-bddd-2c347127b52e@gyani.pro |
|---|---|
| State | Accepted |
| Commit | b5b6f6ad59ef9bd7af7c5ff3cf94ca3d6abd29ae |
| Headers | show |
2019-01-25 13:35 GMT+01:00, Gyan <ffmpeg@gyani.pro>: > + av_log(s, AV_LOG_WARNING, "Changing video frame > properties on the fly is not supported by all filters.\n");\ In which situations is this new warning shown? Carl Eugen
On 25-01-2019 06:11 PM, Carl Eugen Hoyos wrote: > 2019-01-25 13:35 GMT+01:00, Gyan <ffmpeg@gyani.pro>: > >> + av_log(s, AV_LOG_WARNING, "Changing video frame >> properties on the fly is not supported by all filters.\n");\ > In which situations is this new warning shown? e.g. -reinit_filter 0 -i ChangingResolutions.mp4 -vf somefilter ... or -reinit_filter 0 -i satellite_dump.ts -af aresample= ... Gyan
2019-01-25 13:52 GMT+01:00, Gyan <ffmpeg@gyani.pro>: > > > On 25-01-2019 06:11 PM, Carl Eugen Hoyos wrote: >> 2019-01-25 13:35 GMT+01:00, Gyan <ffmpeg@gyani.pro>: >> >>> + av_log(s, AV_LOG_WARNING, "Changing video frame >>> properties on the fly is not supported by all filters.\n");\ >> >> In which situations is this new warning shown? > > e.g. > -reinit_filter 0 -i ChangingResolutions.mp4 -vf somefilter ... Is it only shown if the filter does not support re-initialization? Carl Eugen
On 25-01-2019 06:25 PM, Carl Eugen Hoyos wrote: > 2019-01-25 13:52 GMT+01:00, Gyan <ffmpeg@gyani.pro>: >> >> On 25-01-2019 06:11 PM, Carl Eugen Hoyos wrote: >>> 2019-01-25 13:35 GMT+01:00, Gyan <ffmpeg@gyani.pro>: >>> >>>> + av_log(s, AV_LOG_WARNING, "Changing video frame >>>> properties on the fly is not supported by all filters.\n");\ >>> In which situations is this new warning shown? >> e.g. >> -reinit_filter 0 -i ChangingResolutions.mp4 -vf somefilter ... > Is it only shown if the filter does not support re-initialization? > No. As the existing msgs indicate, it is always printed when _some_ frame properties have changed. The new addition allows the user to discover which properties those are, with timestamp. They can then choose to continue, allow reinit or partition the input (if viable) as appropriate. Gyan
2019-01-25 14:02 GMT+01:00, Gyan <ffmpeg@gyani.pro>: > > On 25-01-2019 06:25 PM, Carl Eugen Hoyos wrote: >> 2019-01-25 13:52 GMT+01:00, Gyan <ffmpeg@gyani.pro>: >>> >>> On 25-01-2019 06:11 PM, Carl Eugen Hoyos wrote: >>>> 2019-01-25 13:35 GMT+01:00, Gyan <ffmpeg@gyani.pro>: >>>> >>>>> + av_log(s, AV_LOG_WARNING, "Changing video frame >>>>> properties on the fly is not supported by all filters.\n");\ >>>> >>>> In which situations is this new warning shown? >>> >>> e.g. >>> -reinit_filter 0 -i ChangingResolutions.mp4 -vf somefilter ... >> >> Is it only shown if the filter does not support re-initialization? >> > No. As the existing msgs indicate, it is always printed when > _some_ frame properties have changed. I may misunderstand but I believe that no warning should be shown in this case. > The new addition allows the user to discover which properties > those are, with timestamp. They can then choose to continue, > allow reinit or partition the input (if viable) as appropriate. It appears to me that the only thing the message would do is to confuse users that do not understand the warning (but see above). Maybe you could provide a more concrete example of when this warning helps to help me understand the issue? Carl Eugen
On 26-01-2019 01:17 AM, Carl Eugen Hoyos wrote: > 2019-01-25 14:02 GMT+01:00, Gyan <ffmpeg@gyani.pro>: >> On 25-01-2019 06:25 PM, Carl Eugen Hoyos wrote: >>> 2019-01-25 13:52 GMT+01:00, Gyan <ffmpeg@gyani.pro>: >>>> On 25-01-2019 06:11 PM, Carl Eugen Hoyos wrote: >>>>> 2019-01-25 13:35 GMT+01:00, Gyan <ffmpeg@gyani.pro>: >>>>> >>>>>> + av_log(s, AV_LOG_WARNING, "Changing video frame >>>>>> properties on the fly is not supported by all filters.\n");\ >>>>> In which situations is this new warning shown? >>>> e.g. >>>> -reinit_filter 0 -i ChangingResolutions.mp4 -vf somefilter ... >>> Is it only shown if the filter does not support re-initialization? >>> >> No. As the existing msgs indicate, it is always printed when >> _some_ frame properties have changed. > I may misunderstand but I believe that no warning should be > shown in this case. Are you objecting to the loglevel? Because a message was _always_ shown - I just added a line with metadata and changed the loglevel of the existing message to warning, which the character of the message warrants. >> The new addition allows the user to discover which properties >> those are, with timestamp. They can then choose to continue, >> allow reinit or partition the input (if viable) as appropriate. > It appears to me that the only thing the message would do is > to confuse users that do not understand the warning (but see > above). Most casual users don't understand most of the warnings. Either users grasp the meaning and act upon it themselves, or they can show it to others who do. I see no benefit to fatal or unusual changes remaining opaque. This hardly even adds to the verbosity. Gyan
2019-01-26 5:42 GMT+01:00, Gyan <ffmpeg@gyani.pro>: > > > On 26-01-2019 01:17 AM, Carl Eugen Hoyos wrote: >> 2019-01-25 14:02 GMT+01:00, Gyan <ffmpeg@gyani.pro>: >>> On 25-01-2019 06:25 PM, Carl Eugen Hoyos wrote: >>>> 2019-01-25 13:52 GMT+01:00, Gyan <ffmpeg@gyani.pro>: >>>>> On 25-01-2019 06:11 PM, Carl Eugen Hoyos wrote: >>>>>> 2019-01-25 13:35 GMT+01:00, Gyan <ffmpeg@gyani.pro>: >>>>>> >>>>>>> + av_log(s, AV_LOG_WARNING, "Changing video frame >>>>>>> properties on the fly is not supported by all filters.\n");\ >>>>>> In which situations is this new warning shown? >>>>> e.g. >>>>> -reinit_filter 0 -i ChangingResolutions.mp4 -vf somefilter ... >>>> Is it only shown if the filter does not support re-initialization? >>>> >>> No. As the existing msgs indicate, it is always printed when >>> _some_ frame properties have changed. >> I may misunderstand but I believe that no warning should be >> shown in this case. > > Are you objecting to the loglevel? Because a message was > _always_ shown - I just added a line with metadata and changed > the loglevel of the existing message to warning, which the > character of the message warrants. Do we agree that without your patch no warning is shown while with your patch a warning is shown? >>> The new addition allows the user to discover which properties >>> those are, with timestamp. They can then choose to continue, >>> allow reinit or partition the input (if viable) as appropriate. >> It appears to me that the only thing the message would do is >> to confuse users that do not understand the warning (but see >> above). > > Most casual users don't understand most of the warnings. This seems to support my objections to the patch. > Either users grasp the meaning and act upon it themselves, > or they can show it to others who do. Interesting approach... > I see no benefit to fatal or unusual changes remaining opaque. > This hardly even adds to the verbosity. Let me rephrase my original questions: Are there command lines for which the warning is shown although the (video) filter chain will work as expected? Carl Eugen
On 26-01-2019 06:40 PM, Carl Eugen Hoyos wrote: > 2019-01-26 5:42 GMT+01:00, Gyan <ffmpeg@gyani.pro>: >> >> On 26-01-2019 01:17 AM, Carl Eugen Hoyos wrote: >>> 2019-01-25 14:02 GMT+01:00, Gyan <ffmpeg@gyani.pro>: >>>> On 25-01-2019 06:25 PM, Carl Eugen Hoyos wrote: >>>>> 2019-01-25 13:52 GMT+01:00, Gyan <ffmpeg@gyani.pro>: >>>>>> On 25-01-2019 06:11 PM, Carl Eugen Hoyos wrote: >>>>>>> 2019-01-25 13:35 GMT+01:00, Gyan <ffmpeg@gyani.pro>: >>>>>>> >>>>>>>> + av_log(s, AV_LOG_WARNING, "Changing video frame >>>>>>>> properties on the fly is not supported by all filters.\n");\ >>>>>>> In which situations is this new warning shown? >>>>>> e.g. >>>>>> -reinit_filter 0 -i ChangingResolutions.mp4 -vf somefilter ... >>>>> Is it only shown if the filter does not support re-initialization? >>>>> >>>> No. As the existing msgs indicate, it is always printed when >>>> _some_ frame properties have changed. >>> I may misunderstand but I believe that no warning should be >>> shown in this case. >> Are you objecting to the loglevel? Because a message was >> _always_ shown - I just added a line with metadata and changed >> the loglevel of the existing message to warning, which the >> character of the message warrants. > Do we agree that without your patch no warning is shown > while with your patch a warning is shown? Yes, so is your answer to my Q above, yes? >>>> The new addition allows the user to discover which properties >>>> those are, with timestamp. They can then choose to continue, >>>> allow reinit or partition the input (if viable) as appropriate. >>> It appears to me that the only thing the message would do is >>> to confuse users that do not understand the warning (but see >>> above). >> Most casual users don't understand most of the warnings. > This seems to support my objections to the patch. Casual users won't see this message as info or warning as it requires an undocumented option to be set. >> Either users grasp the meaning and act upon it themselves, >> or they can show it to others who do. > Interesting approach... If the entire log had to be understandable by all users, it would be very short. The user isn't meant to be the only reader of the log. As many posters to ffmpeg-user or trac have realized when you post "complete, uncut /console/ output /missing/" >> I see no benefit to fatal or unusual changes remaining opaque. >> This hardly even adds to the verbosity. > Let me rephrase my original questions: > Are there command lines for which the warning is shown > although the (video) filter chain will work as expected? Yes, by happenstance. buffersrc can't predict in advance, and there may not be further messages from the filterchain even when output is unexpected, hence the warning (and link/frame info). Try feeding a few images of different sizes to the crop filter with reinit disabled, where the crop size is smaller than the first image but larger than some of the others. Then examine the output frames. Only the existing info level message is shown, with no warnings from downstream. Gyan
2019-01-26 15:10 GMT+01:00, Gyan <ffmpeg@gyani.pro>: > Try feeding a few images of different sizes to the crop > filter with reinit disabled Sorry, not sure if I understand: The warning you want to add is only shown if the filter reinit was disabled by the user? Carl Eugen
On 26-01-2019 07:49 PM, Carl Eugen Hoyos wrote: > 2019-01-26 15:10 GMT+01:00, Gyan <ffmpeg@gyani.pro>: > >> Try feeding a few images of different sizes to the crop >> filter with reinit disabled > Sorry, not sure if I understand: > The warning you want to add is only shown if the filter reinit > was disabled by the user? Yes, reinit filter is enabled by default so if frame props change, ffmpeg reconfigures the graph. If successful, no warning is shown. Gyan
2019-01-26 15:42 GMT+01:00, Gyan <ffmpeg@gyani.pro>: > > > On 26-01-2019 07:49 PM, Carl Eugen Hoyos wrote: >> 2019-01-26 15:10 GMT+01:00, Gyan <ffmpeg@gyani.pro>: >> >>> Try feeding a few images of different sizes to the crop >>> filter with reinit disabled >> Sorry, not sure if I understand: >> The warning you want to add is only shown if the filter reinit >> was disabled by the user? > Yes, reinit filter is enabled by default so if frame props change, > ffmpeg reconfigures the graph. If successful, no warning is shown. Thank you for the explanation, no more comments from me. Carl Eugen
On 26-01-2019 08:23 PM, Carl Eugen Hoyos wrote: > 2019-01-26 15:42 GMT+01:00, Gyan <ffmpeg@gyani.pro>: >> >> On 26-01-2019 07:49 PM, Carl Eugen Hoyos wrote: >>> 2019-01-26 15:10 GMT+01:00, Gyan <ffmpeg@gyani.pro>: >>> >>>> Try feeding a few images of different sizes to the crop >>>> filter with reinit disabled >>> Sorry, not sure if I understand: >>> The warning you want to add is only shown if the filter reinit >>> was disabled by the user? >> Yes, reinit filter is enabled by default so if frame props change, >> ffmpeg reconfigures the graph. If successful, no warning is shown. > Thank you for the explanation, no more comments from me. Thanks. Plan to push tonight. Gyan
On 27-01-2019 09:57 AM, Gyan wrote: > > > On 26-01-2019 08:23 PM, Carl Eugen Hoyos wrote: >> 2019-01-26 15:42 GMT+01:00, Gyan <ffmpeg@gyani.pro>: >>> >>> On 26-01-2019 07:49 PM, Carl Eugen Hoyos wrote: >>>> 2019-01-26 15:10 GMT+01:00, Gyan <ffmpeg@gyani.pro>: >>>> >>>>> Try feeding a few images of different sizes to the crop >>>>> filter with reinit disabled >>>> Sorry, not sure if I understand: >>>> The warning you want to add is only shown if the filter reinit >>>> was disabled by the user? >>> Yes, reinit filter is enabled by default so if frame props change, >>> ffmpeg reconfigures the graph. If successful, no warning is shown. >> Thank you for the explanation, no more comments from me. > Thanks. Plan to push tonight. Pushed as b5b6f6ad59ef9bd7af7c5ff3cf94ca3d6abd29ae Gyan
diff --git a/libavfilter/buffersrc.c b/libavfilter/buffersrc.c index 0c12650e1e..e0ff7e4dd8 100644 --- a/libavfilter/buffersrc.c +++ b/libavfilter/buffersrc.c @@ -33,6 +33,7 @@ #include "libavutil/internal.h" #include "libavutil/opt.h" #include "libavutil/samplefmt.h" +#include "libavutil/timestamp.h" #include "audio.h" #include "avfilter.h" #include "buffersrc.h" @@ -67,15 +68,20 @@ typedef struct BufferSourceContext { int eof; } BufferSourceContext; -#define CHECK_VIDEO_PARAM_CHANGE(s, c, width, height, format)\ +#define CHECK_VIDEO_PARAM_CHANGE(s, c, width, height, format, pts)\ if (c->w != width || c->h != height || c->pix_fmt != format) {\ - av_log(s, AV_LOG_INFO, "Changing frame properties on the fly is not supported by all filters.\n");\ + av_log(s, AV_LOG_INFO, "filter context - w: %d h: %d fmt: %d, incoming frame - w: %d h: %d fmt: %d pts_time: %s\n",\ + c->w, c->h, c->pix_fmt, width, height, format, av_ts2timestr(pts, &s->outputs[0]->time_base));\ + av_log(s, AV_LOG_WARNING, "Changing video frame properties on the fly is not supported by all filters.\n");\ } -#define CHECK_AUDIO_PARAM_CHANGE(s, c, srate, ch_layout, ch_count, format)\ +#define CHECK_AUDIO_PARAM_CHANGE(s, c, srate, ch_layout, ch_count, format, pts)\ if (c->sample_fmt != format || c->sample_rate != srate ||\ c->channel_layout != ch_layout || c->channels != ch_count) {\ - av_log(s, AV_LOG_ERROR, "Changing frame properties on the fly is not supported.\n");\ + av_log(s, AV_LOG_INFO, "filter context - fmt: %s r: %d layout: %"PRIX64" ch: %d, incoming frame - fmt: %s r: %d layout: %"PRIX64" ch: %d pts_time: %s\n",\ + av_get_sample_fmt_name(c->sample_fmt), c->sample_rate, c->channel_layout, c->channels,\ + av_get_sample_fmt_name(format), srate, ch_layout, ch_count, av_ts2timestr(pts, &s->outputs[0]->time_base));\ + av_log(s, AV_LOG_ERROR, "Changing audio frame properties on the fly is not supported.\n");\ return AVERROR(EINVAL);\ } @@ -208,14 +214,14 @@ static int av_buffersrc_add_frame_internal(AVFilterContext *ctx, switch (ctx->outputs[0]->type) { case AVMEDIA_TYPE_VIDEO: CHECK_VIDEO_PARAM_CHANGE(ctx, s, frame->width, frame->height, - frame->format); + frame->format, frame->pts); break; case AVMEDIA_TYPE_AUDIO: /* For layouts unknown on input but known on link after negotiation. */ if (!frame->channel_layout) frame->channel_layout = s->channel_layout; CHECK_AUDIO_PARAM_CHANGE(ctx, s, frame->sample_rate, frame->channel_layout, - frame->channels, frame->format); + frame->channels, frame->format, frame->pts); break; default: return AVERROR(EINVAL);
Helpful in knowing what has changed over a filter link. From a24490e8f6e7b3d5429a049d581c538491d5f6d2 Mon Sep 17 00:00:00 2001 From: Gyan Doshi <ffmpeg@gyani.pro> Date: Fri, 25 Jan 2019 17:56:11 +0530 Subject: [PATCH] avfilter/buffersrc: print relevant info when skipping filter reinit The timestamp of the changed input frame as well as its relevant properties can be examined by the user. Only applicable when reinit_filter is disabled on the input stream. --- libavfilter/buffersrc.c | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-)