Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
 help / color / mirror / Atom feed
From: Gyan Doshi <ffmpeg@gyani.pro>
To: ffmpeg-devel@ffmpeg.org
Subject: Re: [FFmpeg-devel] [PATCH] ffmpeg: add per-stream input option drop_changed
Date: Sun, 16 Mar 2025 12:52:11 +0530
Message-ID: <c1ac6511-2c79-4413-ac2c-56bd5d78e75b@gyani.pro> (raw)
In-Reply-To: <b6528a6f-fd16-5fed-cc07-2343627e1948@passwd.hu>



On 2025-03-16 01:21 am, Marton Balint wrote:
>
>
> On Sat, 15 Mar 2025, Gyan Doshi wrote:
>
>> This is a replacement in ffmpeg for the deprecated avcodec flag 
>> AV_CODEC_FLAG_DROPCHANGED.
>>
>> This option is meant to be used when the filtergraph should not be
>> reinited upon input parameter changes as that leads to loss of state
>> in the filtergraph potentially leading to broken or aborted output,
>> e.g. inserting of silence with first_pts specified in aresample.
>>
>> Generally useful to avoid corrupted yet decodable packets in live
>> streaming inputs.
>>
>> This option when enabled takes precedence over reinit_filters
>> ---
>> doc/ffmpeg.texi         |  6 ++++++
>> fftools/ffmpeg.h        |  2 ++
>> fftools/ffmpeg_demux.c  | 13 ++++++++++++-
>> fftools/ffmpeg_filter.c | 10 ++++++++++
>> fftools/ffmpeg_opt.c    |  3 +++
>> 5 files changed, 33 insertions(+), 1 deletion(-)
>>
>> diff --git a/doc/ffmpeg.texi b/doc/ffmpeg.texi
>> index fca220a334..a73db79f94 100644
>> --- a/doc/ffmpeg.texi
>> +++ b/doc/ffmpeg.texi
>> @@ -1373,6 +1373,12 @@ The properties where a change triggers 
>> reinitialization are,
>> for video, frame resolution or pixel format;
>> for audio, sample format, sample rate, channel count or channel layout.
>>
>> +@item -drop_changed[:@var{stream_specifier}] @var{integer} 
>> (@emph{input,per-stream})
>> +This boolean option determines whether a frame with differing frame 
>> parameters mid-stream
>> +gets dropped instead of leading to filtergraph reinitialization, as 
>> that would lead to loss
>> +of filter state. Generally useful to avoid corrupted yet decodable 
>> packets in live streaming
>> +inputs. Default is false.
>> +
>> @item -filter_threads @var{nb_threads} (@emph{global})
>> Defines how many threads are used to process a filter pipeline. Each 
>> pipeline
>> will produce a thread pool with this many threads available for 
>> parallel processing.
>> diff --git a/fftools/ffmpeg.h b/fftools/ffmpeg.h
>> index 86a3e10c6b..5869979214 100644
>> --- a/fftools/ffmpeg.h
>> +++ b/fftools/ffmpeg.h
>> @@ -232,6 +232,7 @@ typedef struct OptionsContext {
>>     SpecifierOptList filter_scripts;
>> #endif
>>     SpecifierOptList reinit_filters;
>> +    SpecifierOptList drop_changed;
>>     SpecifierOptList fix_sub_duration;
>>     SpecifierOptList fix_sub_duration_heartbeat;
>>     SpecifierOptList canvas_sizes;
>> @@ -262,6 +263,7 @@ enum IFilterFlags {
>>     IFILTER_FLAG_REINIT         = (1 << 1),
>>     IFILTER_FLAG_CFR            = (1 << 2),
>>     IFILTER_FLAG_CROP           = (1 << 3),
>> +    IFILTER_FLAG_DROPCHANGED    = (1 << 4),
>> };
>>
>> typedef struct InputFilterOptions {
>> diff --git a/fftools/ffmpeg_demux.c b/fftools/ffmpeg_demux.c
>> index b8171e336e..292b718f14 100644
>> --- a/fftools/ffmpeg_demux.c
>> +++ b/fftools/ffmpeg_demux.c
>> @@ -67,6 +67,7 @@ typedef struct DemuxStream {
>>     int                      reinit_filters;
>>     int                      autorotate;
>>     int                      apply_cropping;
>> +    int                      drop_changed;
>>
>>
>>     int                      wrap_correction_done;
>> @@ -1099,7 +1100,8 @@ int ist_filter_add(InputStream *ist, 
>> InputFilter *ifilter, int is_simple,
>>         return AVERROR(ENOMEM);
>>
>>     opts->flags |= IFILTER_FLAG_AUTOROTATE * !!(ds->autorotate) |
>> -                   IFILTER_FLAG_REINIT     * !!(ds->reinit_filters);
>> +                   IFILTER_FLAG_REINIT     * !!(ds->reinit_filters) |
>> +                   IFILTER_FLAG_DROPCHANGED* !!(ds->drop_changed);
>>
>>     return 0;
>> }
>> @@ -1410,6 +1412,15 @@ static int ist_add(const OptionsContext *o, 
>> Demuxer *d, AVStream *st, AVDictiona
>>     ds->reinit_filters = -1;
>>     opt_match_per_stream_int(ist, &o->reinit_filters, ic, st, 
>> &ds->reinit_filters);
>>
>> +    ds->drop_changed = 0;
>> +    opt_match_per_stream_int(ist, &o->drop_changed, ic, st, 
>> &ds->drop_changed);
>> +
>> +    if (ds->drop_changed && ds->reinit_filters) {
>> +        if (ds->reinit_filters > 0)
>> +            av_log(ist, AV_LOG_ERROR, "drop_changed and 
>> reinit_filters both set, which is contradictory; ignoring 
>> reinit_filters.\n");
>
> I'd rather simply return an error, instead of continuing, unless there 
> is a very good reason to allow conflicting options and fall back to 
> one or the other.

Done
>
>> +        ds->reinit_filters = 0;
>> +    }
>> +
>>     ist->user_set_discard = AVDISCARD_NONE;
>>
>>     if ((o->video_disable && ist->st->codecpar->codec_type == 
>> AVMEDIA_TYPE_VIDEO) ||
>> diff --git a/fftools/ffmpeg_filter.c b/fftools/ffmpeg_filter.c
>> index ef35c3988f..fddfbceec1 100644
>> --- a/fftools/ffmpeg_filter.c
>> +++ b/fftools/ffmpeg_filter.c
>> @@ -126,6 +126,7 @@ typedef struct InputFilterPriv {
>>
>>     int                 eof;
>>     int                 bound;
>> +    uint64_t            nb_dropped;
>>
>>     // parameters configured for this input
>>     int                 format;
>> @@ -2898,6 +2899,13 @@ static int send_frame(FilterGraph *fg, 
>> FilterGraphThread *fgt,
>>     } else if (ifp->downmixinfo_present)
>>         need_reinit |= DOWNMIX_CHANGED;
>>
>> +    if (need_reinit && (ifp->opts.flags & IFILTER_FLAG_DROPCHANGED)) {
>> +            ifp->nb_dropped++;
>> +            av_log(fg, AV_LOG_DEBUG, "Avoiding reinit; dropping 
>> frame pts: %s bound for %s\n", av_ts2str(frame->pts), ifilter->name);
>
> Maybe it would make sense to use AV_LOG_WARNING for the first dropped 
> frame.

Done.

>
>> +            av_frame_unref(frame);
>> +            return 0;
>> +    }
>> +
>>     if (!(ifp->opts.flags & IFILTER_FLAG_REINIT) && fgt->graph)
>>         need_reinit = 0;
>>
>> @@ -3140,6 +3148,8 @@ read_frames:
>>         ret = read_frames(fg, &fgt, fgt.frame);
>>         if (ret == AVERROR_EOF) {
>>             av_log(fg, AV_LOG_VERBOSE, "All consumers returned EOF\n");
>> +            if (ifp->opts.flags & IFILTER_FLAG_DROPCHANGED)
>> +                av_log(fg, AV_LOG_VERBOSE, "Total changed input 
>> frames dropped : %"PRId64"\n", ifp->nb_dropped);
>
> I think you should make this AV_LOG_INFO and it is enough to print if 
> the number of dropped frames is non-zero.

Level promoted. I've kept the drop count as it is a crude indicator of 
'signal quality'.

Regards,
Gyan


>
>>             break;
>>         } else if (ret < 0) {
>>             av_log(fg, AV_LOG_ERROR, "Error sending frames to 
>> consumers: %s\n",
>> diff --git a/fftools/ffmpeg_opt.c b/fftools/ffmpeg_opt.c
>> index 1f5e6050c8..6ec325f51e 100644
>> --- a/fftools/ffmpeg_opt.c
>> +++ b/fftools/ffmpeg_opt.c
>> @@ -1718,6 +1718,9 @@ const OptionDef options[] = {
>>     { "reinit_filter",          OPT_TYPE_INT, OPT_PERSTREAM | 
>> OPT_INPUT | OPT_EXPERT,
>>         { .off = OFFSET(reinit_filters) },
>>         "reinit filtergraph on input parameter changes", "" },
>> +    { "drop_changed",          OPT_TYPE_INT, OPT_PERSTREAM | 
>> OPT_INPUT | OPT_EXPERT,
>> +        { .off = OFFSET(drop_changed) },
>> +        "drop frame instead of reiniting filtergraph on input 
>> parameter changes", "" },
>>     { "filter_complex",         OPT_TYPE_FUNC, OPT_FUNC_ARG | 
>> OPT_EXPERT,
>>         { .func_arg = opt_filter_complex },
>>         "create a complex filtergraph", "graph_description" },
>> -- 
>> 2.46.1
>
> Thanks,
> Marton
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

      reply	other threads:[~2025-03-16  7:22 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-03-15 12:20 Gyan Doshi
2025-03-15 19:51 ` Marton Balint
2025-03-16  7:22   ` Gyan Doshi [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=c1ac6511-2c79-4413-ac2c-56bd5d78e75b@gyani.pro \
    --to=ffmpeg@gyani.pro \
    --cc=ffmpeg-devel@ffmpeg.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
		ffmpegdev@gitmailbox.com
	public-inbox-index ffmpegdev

Example config snippet for mirrors.


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git