diff mbox

[FFmpeg-devel] ffmpeg: add -drop_deviant_frames option

Message ID 7d764fe7-1752-404c-a8e8-3e5dc5a5f41f@gyani.pro
State Superseded
Headers show

Commit Message

Gyan Doshi March 27, 2019, 2:56 p.m. UTC
Weird. Attached corrected patch.

Thanks.

On 27-03-2019 08:19 PM, Moritz Barsnick wrote:
> On Wed, Mar 27, 2019 at 18:05:32 +0530, Gyan wrote:
>> +@item -drop_deviant_frames (@emph{input,per-stream})
>> +Allows discarding decoded frames whose parameters differ from initialized
>> +stream parameters. May be useful in
>> +
> It looks like this was truncated.
>
> Moritz
> Cheers,
> _______________________________________________
> 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".
From f39e3d44e1cb8108592d2241e2251d8e1506e031 Mon Sep 17 00:00:00 2001
From: Gyan Doshi <ffmpeg@gyani.pro>
Date: Mon, 25 Mar 2019 22:01:07 +0530
Subject: [PATCH] ffmpeg: add -drop_deviant_frames option

Decoded frames with changed parameters in inputs expected to be uniform
may indicate malformed/corrupted packets. Demuxer with flag discardcorrupt
set may not catch all such packets and neither may decoders with err_detect
flags set.

Upon parameter change, AVFilterLink of any filters receiving such frames has
to be reconfigured, destroying existing filter context. In case of a
change in audio sample rate, timestamps can go haywire, potentially
leaving the output stream to be "truncated".

Added option allows users to avoid sending such deviant frames onwards.
Default behaviour remains unchanged.
---
 doc/ffmpeg.texi      |  5 +++++
 fftools/ffmpeg.c     | 47 ++++++++++++++++++++++++++++++++++++++++++--
 fftools/ffmpeg.h     |  3 +++
 fftools/ffmpeg_opt.c |  5 +++++
 4 files changed, 58 insertions(+), 2 deletions(-)

Comments

Gyan Doshi April 1, 2019, 4:53 a.m. UTC | #1
On 27-03-2019 08:26 PM, Gyan wrote:
> Weird. Attached corrected patch.
>
> Thanks.

Ping.

Gyan
Gyan Doshi April 2, 2019, 4:26 a.m. UTC | #2
On 01-04-2019 10:23 AM, Gyan wrote:
>
>
> On 27-03-2019 08:26 PM, Gyan wrote:
>> Weird. Attached corrected patch.
>>
>> Thanks.
>
> Ping.

Plan to push tomorrow evening.

Gyan
Gyan Doshi April 3, 2019, 1:54 p.m. UTC | #3
On 02-04-2019 09:56 AM, Gyan wrote:
>
>
> On 01-04-2019 10:23 AM, Gyan wrote:
>>
>>
>> On 27-03-2019 08:26 PM, Gyan wrote:
>>> Weird. Attached corrected patch.
>>>
>>> Thanks.
>>
>> Ping.
>
> Plan to push tomorrow evening.

Plan to push in ~2 hours.

Gyan
Paul B Mahol April 3, 2019, 3:12 p.m. UTC | #4
On 4/3/19, Gyan <ffmpeg@gyani.pro> wrote:
>
>
> On 02-04-2019 09:56 AM, Gyan wrote:
>>
>>
>> On 01-04-2019 10:23 AM, Gyan wrote:
>>>
>>>
>>> On 27-03-2019 08:26 PM, Gyan wrote:
>>>> Weird. Attached corrected patch.
>>>>
>>>> Thanks.
>>>
>>> Ping.
>>
>> Plan to push tomorrow evening.
>
> Plan to push in ~2 hours.

Please delay it.
Michael Niedermayer April 3, 2019, 9:07 p.m. UTC | #5
On Wed, Mar 27, 2019 at 08:26:36PM +0530, Gyan wrote:
> Weird. Attached corrected patch.
> 
> Thanks.
> 
> On 27-03-2019 08:19 PM, Moritz Barsnick wrote:
> >On Wed, Mar 27, 2019 at 18:05:32 +0530, Gyan wrote:
> >>+@item -drop_deviant_frames (@emph{input,per-stream})
> >>+Allows discarding decoded frames whose parameters differ from initialized
> >>+stream parameters. May be useful in
> >>+
> >It looks like this was truncated.
> >
> >Moritz
> >Cheers,
> >_______________________________________________
> >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".
> 

>  doc/ffmpeg.texi      |    5 +++++
>  fftools/ffmpeg.c     |   47 +++++++++++++++++++++++++++++++++++++++++++++--
>  fftools/ffmpeg.h     |    3 +++
>  fftools/ffmpeg_opt.c |    5 +++++
>  4 files changed, 58 insertions(+), 2 deletions(-)
> 71a87fe4d8f2efa73a3d2d9845e7aabfb75edce5  0001-ffmpeg-add-drop_deviant_frames-option.patch
> From f39e3d44e1cb8108592d2241e2251d8e1506e031 Mon Sep 17 00:00:00 2001
> From: Gyan Doshi <ffmpeg@gyani.pro>
> Date: Mon, 25 Mar 2019 22:01:07 +0530
> Subject: [PATCH] ffmpeg: add -drop_deviant_frames option
> 
> Decoded frames with changed parameters in inputs expected to be uniform
> may indicate malformed/corrupted packets. Demuxer with flag discardcorrupt
> set may not catch all such packets and neither may decoders with err_detect
> flags set.
> 
> Upon parameter change, AVFilterLink of any filters receiving such frames has
> to be reconfigured, destroying existing filter context. In case of a
> change in audio sample rate, timestamps can go haywire, potentially
> leaving the output stream to be "truncated".
> 
> Added option allows users to avoid sending such deviant frames onwards.
> Default behaviour remains unchanged.
> ---
>  doc/ffmpeg.texi      |  5 +++++
>  fftools/ffmpeg.c     | 47 ++++++++++++++++++++++++++++++++++++++++++--
>  fftools/ffmpeg.h     |  3 +++
>  fftools/ffmpeg_opt.c |  5 +++++
>  4 files changed, 58 insertions(+), 2 deletions(-)

Would it make sense to make this feature available to other applications ?
The awnser here has an effect of the design because
For example if this is implemented in 
libavcodec or libavfilter then other applications could
use such a feature. While if its in ffmpeg then other applications could
not. 

Thanks

[...]
Gyan Doshi April 4, 2019, 12:24 p.m. UTC | #6
On 04-04-2019 02:37 AM, Michael Niedermayer wrote:
>>   doc/ffmpeg.texi      |    5 +++++
>>   fftools/ffmpeg.c     |   47 +++++++++++++++++++++++++++++++++++++++++++++--
>>   fftools/ffmpeg.h     |    3 +++
>>   fftools/ffmpeg_opt.c |    5 +++++
>>   4 files changed, 58 insertions(+), 2 deletions(-)
>> 71a87fe4d8f2efa73a3d2d9845e7aabfb75edce5  0001-ffmpeg-add-drop_deviant_frames-option.patch
>>  From f39e3d44e1cb8108592d2241e2251d8e1506e031 Mon Sep 17 00:00:00 2001
>> From: Gyan Doshi <ffmpeg@gyani.pro>
>> Date: Mon, 25 Mar 2019 22:01:07 +0530
>> Subject: [PATCH] ffmpeg: add -drop_deviant_frames option
>>
>> Decoded frames with changed parameters in inputs expected to be uniform
>> may indicate malformed/corrupted packets. Demuxer with flag discardcorrupt
>> set may not catch all such packets and neither may decoders with err_detect
>> flags set.
>>
>> Upon parameter change, AVFilterLink of any filters receiving such frames has
>> to be reconfigured, destroying existing filter context. In case of a
>> change in audio sample rate, timestamps can go haywire, potentially
>> leaving the output stream to be "truncated".
>>
>> Added option allows users to avoid sending such deviant frames onwards.
>> Default behaviour remains unchanged.
>> ---
>>   doc/ffmpeg.texi      |  5 +++++
>>   fftools/ffmpeg.c     | 47 ++++++++++++++++++++++++++++++++++++++++++--
>>   fftools/ffmpeg.h     |  3 +++
>>   fftools/ffmpeg_opt.c |  5 +++++
>>   4 files changed, 58 insertions(+), 2 deletions(-)
> Would it make sense to make this feature available to other applications ?
> The awnser here has an effect of the design because
> For example if this is implemented in
> libavcodec or libavfilter then other applications could
> use such a feature. While if its in ffmpeg then other applications could
> not.
Could be a useful addition to libavcodec, since not all users may onward 
process using libavfilter.

Looks like avcodec_receive_frame is the place to add this gate.

Gyan
diff mbox

Patch

diff --git a/doc/ffmpeg.texi b/doc/ffmpeg.texi
index cd35eb49c8..b388942859 100644
--- a/doc/ffmpeg.texi
+++ b/doc/ffmpeg.texi
@@ -1664,6 +1664,11 @@  Discard all frames excepts keyframes.
 Discard all frames.
 @end table
 
+@item -drop_deviant_frames (@emph{input,per-stream})
+Allows discarding decoded frames whose parameters differ from initialized
+stream parameters. May be useful when an input with uniform frames is received
+over unreliable transport.
+
 @item -abort_on @var{flags} (@emph{global})
 Stop and abort on various conditions. The following flags are available:
 
diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c
index 544f1a1cef..5da75306e3 100644
--- a/fftools/ffmpeg.c
+++ b/fftools/ffmpeg.c
@@ -2283,7 +2283,7 @@  static int decode_audio(InputStream *ist, AVPacket *pkt, int *got_output,
 {
     AVFrame *decoded_frame;
     AVCodecContext *avctx = ist->dec_ctx;
-    int ret, err = 0;
+    int ret, err = 0, deviant = 0;
     AVRational decoded_frame_tb;
 
     if (!ist->decoded_frame && !(ist->decoded_frame = av_frame_alloc()))
@@ -2303,12 +2303,36 @@  static int decode_audio(InputStream *ist, AVPacket *pkt, int *got_output,
         ret = AVERROR_INVALIDDATA;
     }
 
+    if (*got_output && ret >= 0 && ist->frames_decoded && ist->nb_filters &&
+        (ist->filters[0]->sample_rate    != decoded_frame->sample_rate ||
+         ist->filters[0]->channels       != decoded_frame->channels ||
+         ist->filters[0]->channel_layout != decoded_frame->channel_layout ||
+         ist->filters[0]->format         != decoded_frame->format ||
+         ist->filters[0]->sample_rate    != avctx->sample_rate)) {
+        av_log(NULL, ist->drop_deviant_frames ? AV_LOG_INFO : AV_LOG_VERBOSE,
+               "Parameter change in decoded audio frame from stream %d.%d pts %s \n"
+               "filter rate:%d fmt:%s ch:%d layout:%"PRIX64" != frame rate:%d fmt:%s ch:%d layout:%"PRIX64", decoder context rate: %d \n",
+               ist->file_index, ist->st->index, av_ts2timestr(decoded_frame->pts, &ist->st->time_base),
+               ist->filters[0]->sample_rate, av_get_sample_fmt_name(ist->filters[0]->format),
+               ist->filters[0]->channels, ist->filters[0]->channel_layout,
+               decoded_frame->sample_rate, av_get_sample_fmt_name(decoded_frame->format),
+               decoded_frame->channels, decoded_frame->channel_layout,
+               avctx->sample_rate);
+        if (ist->drop_deviant_frames) {
+            deviant = 1;
+            av_log(NULL, AV_LOG_WARNING, "Dropping frame.\n");
+        }
+    }
+
     if (ret != AVERROR_EOF)
         check_decode_result(ist, got_output, ret);
 
     if (!*got_output || ret < 0)
         return ret;
 
+    if (deviant)
+        goto fail;
+
     ist->samples_decoded += decoded_frame->nb_samples;
     ist->frames_decoded++;
 
@@ -2335,6 +2359,7 @@  static int decode_audio(InputStream *ist, AVPacket *pkt, int *got_output,
     ist->nb_samples = decoded_frame->nb_samples;
     err = send_frame_to_filters(ist, decoded_frame);
 
+fail:
     av_frame_unref(ist->filter_frame);
     av_frame_unref(decoded_frame);
     return err < 0 ? err : ret;
@@ -2344,7 +2369,7 @@  static int decode_video(InputStream *ist, AVPacket *pkt, int *got_output, int64_
                         int *decode_failed)
 {
     AVFrame *decoded_frame;
-    int i, ret = 0, err = 0;
+    int i, ret = 0, err = 0, deviant = 0;
     int64_t best_effort_timestamp;
     int64_t dts = AV_NOPTS_VALUE;
     AVPacket avpkt;
@@ -2413,11 +2438,29 @@  static int decode_video(InputStream *ist, AVPacket *pkt, int *got_output, int64_
                 ist->dec_ctx->height,
                 ist->dec_ctx->pix_fmt);
         }
+        if (ist->nb_filters && ist->frames_decoded &&
+            (ist->filters[0]->width    != decoded_frame->width ||
+             ist->filters[0]->height   != decoded_frame->height ||
+             ist->filters[0]->format   != decoded_frame->format)) {
+            av_log(NULL, ist->drop_deviant_frames ? AV_LOG_INFO : AV_LOG_VERBOSE,
+               "Parameter change in decoded video frame from stream %d.%d pts %s \n"
+               "filter width:%d height:%d fmt:%s != frame width:%d height:%d fmt:%s \n",
+               ist->file_index, ist->st->index, av_ts2timestr(decoded_frame->pts, &ist->st->time_base),
+               ist->filters[0]->width, ist->filters[0]->height, av_get_pix_fmt_name(ist->filters[0]->format),
+               decoded_frame->width, decoded_frame->height, av_get_pix_fmt_name(decoded_frame->format));
+            if (ist->drop_deviant_frames) {
+                deviant = 1;
+                av_log(NULL, AV_LOG_WARNING, "Dropping frame.\n");
+            }
+        }
     }
 
     if (!*got_output || ret < 0)
         return ret;
 
+    if (deviant)
+        goto fail;
+
     if(ist->top_field_first>=0)
         decoded_frame->top_field_first = ist->top_field_first;
 
diff --git a/fftools/ffmpeg.h b/fftools/ffmpeg.h
index eb1eaf6363..b0a71ff8c7 100644
--- a/fftools/ffmpeg.h
+++ b/fftools/ffmpeg.h
@@ -133,6 +133,8 @@  typedef struct OptionsContext {
     int        nb_hwaccel_output_formats;
     SpecifierOpt *autorotate;
     int        nb_autorotate;
+    SpecifierOpt *drop_deviant_frames;
+    int        nb_drop_deviant_frames;
 
     /* output options */
     StreamMap *stream_maps;
@@ -388,6 +390,7 @@  typedef struct InputStream {
     int nb_dts_buffer;
 
     int got_output;
+    int drop_deviant_frames;    /* drop decoded frames whose parameters differ from first filter's inlink parameters */
 } InputStream;
 
 typedef struct InputFile {
diff --git a/fftools/ffmpeg_opt.c b/fftools/ffmpeg_opt.c
index 53d688b764..932a995439 100644
--- a/fftools/ffmpeg_opt.c
+++ b/fftools/ffmpeg_opt.c
@@ -773,6 +773,9 @@  static void add_input_streams(OptionsContext *o, AVFormatContext *ic)
             exit_program(1);
         }
 
+        ist->drop_deviant_frames = 0;
+        MATCH_PER_STREAM_OPT(drop_deviant_frames, i, ist->drop_deviant_frames, ic ,st);
+
         ist->filter_in_rescale_delta_last = AV_NOPTS_VALUE;
 
         ist->dec_ctx = avcodec_alloc_context3(ist->dec);
@@ -3532,6 +3535,8 @@  const OptionDef options[] = {
     { "discard",        OPT_STRING | HAS_ARG | OPT_SPEC |
                         OPT_INPUT,                                   { .off = OFFSET(discard) },
         "discard", "" },
+    { "drop_deviant_frames",    OPT_BOOL | HAS_ARG | OPT_EXPERT | OPT_SPEC | OPT_INPUT,   { .off = OFFSET(drop_deviant_frames) },
+        "discard decoded frames whose parameters differ from initial stream parameters" },
     { "disposition",    OPT_STRING | HAS_ARG | OPT_SPEC |
                         OPT_OUTPUT,                                  { .off = OFFSET(disposition) },
         "disposition", "" },