[FFmpeg-devel] avcodec: add drop_changed_frames

Submitted by Gyan on April 14, 2019, 4:50 p.m.

Details

Message ID 0f859e7c-aa7f-8c1b-7ea2-7aa4029726be@gyani.pro
State New
Headers show

Commit Message

Gyan April 14, 2019, 4:50 p.m.
Implemented this patch

http://www.ffmpeg.org/pipermail/ffmpeg-devel/2019-March/241733.html

  in libavcodec as suggested by Michael

Gyan

P.S. will add doc/APIchanges entry after I have the hash.
From bbc8ac18da560a59cd252bbb5c34dfa1fcc575b8 Mon Sep 17 00:00:00 2001
From: Gyan Doshi <ffmpeg@gyani.pro>
Date: Sun, 14 Apr 2019 22:12:25 +0530
Subject: [PATCH] avcodec: add drop_changed_frames

Discard decoded frames which differ from first decoded frame in stream.
---
 doc/codecs.texi            |  3 +++
 libavcodec/avcodec.h       | 18 ++++++++++++++
 libavcodec/decode.c        | 48 +++++++++++++++++++++++++++++++++++++-
 libavcodec/options_table.h |  1 +
 libavcodec/version.h       |  2 +-
 5 files changed, 70 insertions(+), 2 deletions(-)

Comments

Marton Balint April 14, 2019, 4:59 p.m.
On Sun, 14 Apr 2019, Gyan wrote:

> Implemented this patch
>
> http://www.ffmpeg.org/pipermail/ffmpeg-devel/2019-March/241733.html
>
>  in libavcodec as suggested by Michael
>

Docs should mention that you are not only dropping the frames but 
returning error as well. (I agree that error should be returned).

Thanks,
Marton
Gyan April 14, 2019, 5:36 p.m.
On 14-04-2019 10:29 PM, Marton Balint wrote:
>
>
> On Sun, 14 Apr 2019, Gyan wrote:
>
>> Implemented this patch
>>
>> http://www.ffmpeg.org/pipermail/ffmpeg-devel/2019-March/241733.html
>>
>>  in libavcodec as suggested by Michael
>>
>
> Docs should mention that you are not only dropping the frames but 
> returning error as well. (I agree that error should be returned).

Updated doxy for avcodec_receive_frame. Do you want a note in the option 
doc entry as well?

Gyan
Marton Balint April 14, 2019, 5:39 p.m.
On Sun, 14 Apr 2019, Gyan wrote:

>
>
> On 14-04-2019 10:29 PM, Marton Balint wrote:
>>
>>
>> On Sun, 14 Apr 2019, Gyan wrote:
>>
>>> Implemented this patch
>>>
>>> http://www.ffmpeg.org/pipermail/ffmpeg-devel/2019-March/241733.html
>>>
>>>  in libavcodec as suggested by Michael
>>>
>>
>> Docs should mention that you are not only dropping the frames but 
>> returning error as well. (I agree that error should be returned).
>
> Updated doxy for avcodec_receive_frame. Do you want a note in the option
> doc entry as well?

I think it is better if we put a note there as well.

Thanks,
Marton
Hendrik Leppkes April 14, 2019, 6:29 p.m.
On Sun, Apr 14, 2019 at 6:50 PM Gyan <ffmpeg@gyani.pro> wrote:
>
> Implemented this patch
> http://www.ffmpeg.org/pipermail/ffmpeg-devel/2019-March/241733.html
>
>
>   in libavcodec as suggested by Michael
>

This sure adds a lot of additional fields to the main struct for a
rather specialized feature, that I personally rather see in the hands
of the user of avcodec, not avcodec itself.

In any case, can't we do this without any new public fields at all?
Put the initial_* state fields into an internal struct (ie.
AVCodecInternal), and expose enabling this through
AVCodecContext->flags or flags2?
That would make me feel much less dirty looking at this patch, personally.

- Hendrik
James Almer April 14, 2019, 6:47 p.m.
On 4/14/2019 3:29 PM, Hendrik Leppkes wrote:
> On Sun, Apr 14, 2019 at 6:50 PM Gyan <ffmpeg@gyani.pro> wrote:
>>
>> Implemented this patch
>> http://www.ffmpeg.org/pipermail/ffmpeg-devel/2019-March/241733.html
>>
>>
>>   in libavcodec as suggested by Michael
>>
> 
> This sure adds a lot of additional fields to the main struct for a
> rather specialized feature, that I personally rather see in the hands
> of the user of avcodec, not avcodec itself.
> 
> In any case, can't we do this without any new public fields at all?
> Put the initial_* state fields into an internal struct (ie.
> AVCodecInternal), and expose enabling this through
> AVCodecContext->flags or flags2?
> That would make me feel much less dirty looking at this patch, personally.

+1

There has been work to turn public AVCodecContext fields into internal
codec options recently as they were too specialized. This commit adds
half a dozen of such fields in one go, so it just feels wrong.
Gyan April 15, 2019, 6:16 a.m.
On 15-04-2019 12:17 AM, James Almer wrote:
> On 4/14/2019 3:29 PM, Hendrik Leppkes wrote:
>> On Sun, Apr 14, 2019 at 6:50 PM Gyan <ffmpeg@gyani.pro> wrote:
>>> Implemented this patch
>>> http://www.ffmpeg.org/pipermail/ffmpeg-devel/2019-March/241733.html
>>>
>>>
>>>    in libavcodec as suggested by Michael
>>>
>> This sure adds a lot of additional fields to the main struct for a
>> rather specialized feature, that I personally rather see in the hands
>> of the user of avcodec, not avcodec itself.
>>
>> In any case, can't we do this without any new public fields at all?
>> Put the initial_* state fields into an internal struct (ie.
>> AVCodecInternal), and expose enabling this through
>> AVCodecContext->flags or flags2?
>> That would make me feel much less dirty looking at this patch, personally.
> +1
>
> There has been work to turn public AVCodecContext fields into internal
> codec options recently as they were too specialized. This commit adds
> half a dozen of such fields in one go, so it just feels wrong.

Since, for this proposed use, the initial frame information has to be 
recorded, I thought to make them public, so any avcodec user could check 
them for other purposes. Right now, their population is contingent on 
the changed option being set, but that can be taken care of. But no firm 
preference here.

I have a soft preference to keep the first field distinct in avctx, as 
I've seen CLI users to be sloppy both with option placement and setting 
multiple flags correctly for both the same or different target streams.

Gyan
Hendrik Leppkes April 15, 2019, 8:26 a.m.
On Mon, Apr 15, 2019 at 8:17 AM Gyan <ffmpeg@gyani.pro> wrote:
>
>
>
> On 15-04-2019 12:17 AM, James Almer wrote:
> > On 4/14/2019 3:29 PM, Hendrik Leppkes wrote:
> >> On Sun, Apr 14, 2019 at 6:50 PM Gyan <ffmpeg@gyani.pro> wrote:
> >>> Implemented this patch
> >>> http://www.ffmpeg.org/pipermail/ffmpeg-devel/2019-March/241733.html
> >>>
> >>>
> >>>    in libavcodec as suggested by Michael
> >>>
> >> This sure adds a lot of additional fields to the main struct for a
> >> rather specialized feature, that I personally rather see in the hands
> >> of the user of avcodec, not avcodec itself.
> >>
> >> In any case, can't we do this without any new public fields at all?
> >> Put the initial_* state fields into an internal struct (ie.
> >> AVCodecInternal), and expose enabling this through
> >> AVCodecContext->flags or flags2?
> >> That would make me feel much less dirty looking at this patch, personally.
> > +1
> >
> > There has been work to turn public AVCodecContext fields into internal
> > codec options recently as they were too specialized. This commit adds
> > half a dozen of such fields in one go, so it just feels wrong.
>
> Since, for this proposed use, the initial frame information has to be
> recorded, I thought to make them public, so any avcodec user could check
> them for other purposes. Right now, their population is contingent on
> the changed option being set, but that can be taken care of. But no firm
> preference here.
>
> I have a soft preference to keep the first field distinct in avctx, as
> I've seen CLI users to be sloppy both with option placement and setting
> multiple flags correctly for both the same or different target streams.
>

CLI usability problems should be resolved in the CLI tools, not in the
avcodec API, imho.

- Hendrik

Patch hide | download patch | download mbox

diff --git a/doc/codecs.texi b/doc/codecs.texi
index 572e561c1a..327350453c 100644
--- a/doc/codecs.texi
+++ b/doc/codecs.texi
@@ -1249,6 +1249,9 @@  Note: The required alignment depends on if @code{AV_CODEC_FLAG_UNALIGNED} is set
 CPU. @code{AV_CODEC_FLAG_UNALIGNED} cannot be changed from the command line. Also hardware
 decoders will not apply left/top Cropping.
 
+@item drop_changed_frames @var{bool} (@emph{decoding,audio,video})
+Drop decoded frames whose parameters are different with respect to the first decoded frame.
+Default is 0 (disabled).
 
 @end table
 
diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
index 9e37466641..9fc59abc98 100644
--- a/libavcodec/avcodec.h
+++ b/libavcodec/avcodec.h
@@ -3360,6 +3360,24 @@  typedef struct AVCodecContext {
      * - encoding: unused
      */
     int discard_damaged_percentage;
+
+    /**
+     * If drop_changed_frames is set, the parameters of the first decoded frame
+     * are stored in the initial_* fields. Any subsequent frame with differing
+     * parameters is dropped. Unset by default.
+     *
+     * @note only applicable with avcodec_receive_frame.
+     *
+     * - decoding: set by user
+     * - encoding: unused
+     */
+    int drop_changed_frames;
+    int changed_frames_dropped;
+    int initial_format;
+    int initial_width, initial_height;
+    int initial_sample_rate;
+    int initial_channels;
+    uint64_t initial_channel_layout;
 } AVCodecContext;
 
 #if FF_API_CODEC_GET_SET
diff --git a/libavcodec/decode.c b/libavcodec/decode.c
index a32ff2fcd3..50baa2105e 100644
--- a/libavcodec/decode.c
+++ b/libavcodec/decode.c
@@ -740,7 +740,7 @@  static int apply_cropping(AVCodecContext *avctx, AVFrame *frame)
 int attribute_align_arg avcodec_receive_frame(AVCodecContext *avctx, AVFrame *frame)
 {
     AVCodecInternal *avci = avctx->internal;
-    int ret;
+    int ret, changed;
 
     av_frame_unref(frame);
 
@@ -765,6 +765,52 @@  int attribute_align_arg avcodec_receive_frame(AVCodecContext *avctx, AVFrame *fr
 
     avctx->frame_number++;
 
+    if (avctx->drop_changed_frames &&
+        (avctx->codec_type == AVMEDIA_TYPE_VIDEO || avctx->codec_type == AVMEDIA_TYPE_AUDIO)) {
+
+        if (avctx->frame_number == 1) {
+            avctx->initial_format = frame->format;
+            switch(avctx->codec_type) {
+            case AVMEDIA_TYPE_VIDEO:
+                avctx->initial_width  = frame->width;
+                avctx->initial_height = frame->height;
+                break;
+            case AVMEDIA_TYPE_AUDIO:
+                avctx->initial_sample_rate = frame->sample_rate ? frame->sample_rate :
+                                                                  avctx->sample_rate;
+                avctx->initial_channels       = frame->channels;
+                avctx->initial_channel_layout = frame->channel_layout;
+                break;
+            }
+        }
+
+        if (avctx->frame_number > 1) {
+            changed = avctx->initial_format != frame->format;
+
+           switch(avctx->codec_type) {
+            case AVMEDIA_TYPE_VIDEO:
+                changed |= avctx->initial_width  != frame->width ||
+                           avctx->initial_height != frame->height;
+                break;
+            case AVMEDIA_TYPE_AUDIO:
+                changed |= avctx->initial_sample_rate    != frame->sample_rate ||
+                           avctx->initial_sample_rate    != avctx->sample_rate ||
+                           avctx->initial_channels       != frame->channels ||
+                           avctx->initial_channel_layout != frame->channel_layout;
+                break;
+            }
+
+            if (changed) {
+                avctx->changed_frames_dropped++;
+                av_log(avctx, AV_LOG_INFO, "dropped changed frame #%d pts %"PRId64
+                                            " drop count: %d \n",
+                                            avctx->frame_number, frame->pts,
+                                            avctx->changed_frames_dropped);
+                av_frame_unref(frame);
+                return AVERROR_INPUT_CHANGED;
+            }
+        }
+    }
     return 0;
 }
 
diff --git a/libavcodec/options_table.h b/libavcodec/options_table.h
index a3235bcd57..96e40005b4 100644
--- a/libavcodec/options_table.h
+++ b/libavcodec/options_table.h
@@ -480,6 +480,7 @@  static const AVOption avcodec_options[] = {
 {"allow_profile_mismatch", "attempt to decode anyway if HW accelerated decoder's supported profiles do not exactly match the stream", 0, AV_OPT_TYPE_CONST, {.i64 = AV_HWACCEL_FLAG_ALLOW_PROFILE_MISMATCH }, INT_MIN, INT_MAX, V | D, "hwaccel_flags"},
 {"extra_hw_frames", "Number of extra hardware frames to allocate for the user", OFFSET(extra_hw_frames), AV_OPT_TYPE_INT, { .i64 = -1 }, -1, INT_MAX, V|D },
 {"discard_damaged_percentage", "Percentage of damaged samples to discard a frame", OFFSET(discard_damaged_percentage), AV_OPT_TYPE_INT, {.i64 = 95 }, 0, 100, V|D },
+{"drop_changed_frames", "Drop frames whose parameters differ from first decoded frame", OFFSET(drop_changed_frames), AV_OPT_TYPE_BOOL, {.i64 = 0}, 0, 1, A|V|D },
 {NULL},
 };
 
diff --git a/libavcodec/version.h b/libavcodec/version.h
index 1b60202dee..195e21bfbe 100644
--- a/libavcodec/version.h
+++ b/libavcodec/version.h
@@ -28,7 +28,7 @@ 
 #include "libavutil/version.h"
 
 #define LIBAVCODEC_VERSION_MAJOR  58
-#define LIBAVCODEC_VERSION_MINOR  51
+#define LIBAVCODEC_VERSION_MINOR  52
 #define LIBAVCODEC_VERSION_MICRO 100
 
 #define LIBAVCODEC_VERSION_INT  AV_VERSION_INT(LIBAVCODEC_VERSION_MAJOR, \