| Message ID | 0f859e7c-aa7f-8c1b-7ea2-7aa4029726be@gyani.pro |
|---|---|
| State | Superseded |
| Headers | show |
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
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
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
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
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.
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
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
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, \
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(-)