From patchwork Fri Apr 10 23:47:43 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Philip Langdale X-Patchwork-Id: 18831 Return-Path: X-Original-To: patchwork@ffaux-bg.ffmpeg.org Delivered-To: patchwork@ffaux-bg.ffmpeg.org Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org [79.124.17.100]) by ffaux.localdomain (Postfix) with ESMTP id 0C45344A642 for ; Sat, 11 Apr 2020 02:47:54 +0300 (EEST) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id E116768B206; Sat, 11 Apr 2020 02:47:53 +0300 (EEST) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mail.overt.org (mail.overt.org [157.230.92.47]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 0D49668AD72 for ; Sat, 11 Apr 2020 02:47:47 +0300 (EEST) Received: from authenticated-user (mail.overt.org [157.230.92.47]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) (No client certificate requested) by mail.overt.org (Postfix) with ESMTPSA id 42FC73F1E7; Fri, 10 Apr 2020 18:47:46 -0500 (CDT) DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=overt.org; s=mail; t=1586562466; bh=ghni6sc4hjj/u4IY+7IMjTPTPIFyZrmNdXveh+CvXGE=; h=From:To:Cc:Subject:Date:From; b=mylocN6bSN5FhkqHG/MAHqAk6xqDVLTw3Ij+0GYArWW9inwq/D4/i29j6nnR+TlBC lqTqNeUgaQva3tEZ3OX5Tgl+7yA3D4UP0p3LnAvsM5va+HBgm3qJ8DfR1DnKc/28im +56llmQBsQAS4PAGvRnBQBHYNGe4toYlCncUBo9uvt69F2UIYHIDPrh7W1UzLZo3Pw mN8KBLxwLH1LIDuOWDZ2Qz2jP3DOJYE87AQSIVMNPnXtkVVO5CSBUnzF5Pielw6vbu a8vagUmo4RZM5FULZF+uJELsPEGn/00Bj3qkYFqmzx7oHTj04dywezon3ZgNuhkkDL A5BFOUnJeefKg== From: Philip Langdale To: ffmpeg-devel@ffmpeg.org Date: Fri, 10 Apr 2020 16:47:43 -0700 Message-Id: <20200410234743.13873-1-philipl@overt.org> MIME-Version: 1.0 Subject: [FFmpeg-devel] [PATCH v3] avcodec: Add explicit capability flag for encoder flushing X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: FFmpeg development discussions and patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: FFmpeg development discussions and patches Cc: Philip Langdale Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" We've been in this fuzzy situation where maybe you could call avcodec_flush_buffers() on an encoder but there weren't any encoders that supported it except maybe audiotoolboxenc. Then we added flush support to nvenc very intentionally, and it worked, but that was more a coincidence than anything else. And if you call avcodec_flush_buffers() on an encoder that doesn't support it, it'll leave the encoder in an undefined state, so that's not great. As part of cleaning this up, this change introduces a formal capability flag for encoders that support flushing and ensures a flush call is a no-op for any other encoder. This allows client code to check if it is meaningful to call flush on an encoder before actually doing it. I have not attempted to separate the steps taken inside avcodec_flush_buffers() because it's not doing anything that's wrong for an encoder. But I did add a sanity check to reject attempts to flush a frame threaded encoder because I couldn't wrap my head around whether that code path was actually safe or not. As this combination doesn't exist today, we'll deal with it if it ever comes up. Signed-off-by: Philip Langdale --- doc/APIchanges | 6 ++++++ libavcodec/audiotoolboxenc.c | 3 ++- libavcodec/avcodec.h | 21 ++++++++++++++++----- libavcodec/decode.c | 15 +++++++++++++++ libavcodec/nvenc_h264.c | 6 ++++-- libavcodec/nvenc_hevc.c | 6 ++++-- libavcodec/version.h | 4 ++-- 7 files changed, 49 insertions(+), 12 deletions(-) diff --git a/doc/APIchanges b/doc/APIchanges index 4cc2367e69..938ccf3aaa 100644 --- a/doc/APIchanges +++ b/doc/APIchanges @@ -15,6 +15,12 @@ libavutil: 2017-10-21 API changes, most recent first: +2020-04-10 - xxxxxxxxxx - lavc 58.79.100 - avcodec.h + Add formal support for calling avcodec_flush_buffers() on encoders. + Encoders that set the cap AV_CODEC_CAP_ENCODER_FLUSH will be flushed. + For all other encoders, the call is now a no-op rather than undefined + behaviour. + 2020-xx-xx - xxxxxxxxxx - lavc 58.78.100 - avcodec.h codec_desc.h codec_id.h packet.h Move AVCodecDesc-related public API to new header codec_desc.h. Move AVCodecID enum to new header codec_id.h. diff --git a/libavcodec/audiotoolboxenc.c b/libavcodec/audiotoolboxenc.c index 2c1891693e..27632decf5 100644 --- a/libavcodec/audiotoolboxenc.c +++ b/libavcodec/audiotoolboxenc.c @@ -627,7 +627,8 @@ static const AVOption options[] = { .encode2 = ffat_encode, \ .flush = ffat_encode_flush, \ .priv_class = &ffat_##NAME##_enc_class, \ - .capabilities = AV_CODEC_CAP_DR1 | AV_CODEC_CAP_DELAY __VA_ARGS__, \ + .capabilities = AV_CODEC_CAP_DR1 | AV_CODEC_CAP_DELAY | \ + AV_CODEC_CAP_ENCODER_FLUSH __VA_ARGS__, \ .sample_fmts = (const enum AVSampleFormat[]) { \ AV_SAMPLE_FMT_S16, \ AV_SAMPLE_FMT_U8, AV_SAMPLE_FMT_NONE \ diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h index 55151a0b71..5efe5583a1 100644 --- a/libavcodec/avcodec.h +++ b/libavcodec/avcodec.h @@ -513,6 +513,13 @@ typedef struct RcOverride{ */ #define AV_CODEC_CAP_ENCODER_REORDERED_OPAQUE (1 << 20) +/** + * This encoder can be flushed using avcodec_flush_buffers(). If this flag is + * not set, the encoder must be closed and reopened to ensure that no frames + * remain pending. + */ +#define AV_CODEC_CAP_ENCODER_FLUSH (1 << 21) + /* Exported side data. These flags can be passed in AVCodecContext.export_side_data before initialization. */ @@ -4473,13 +4480,17 @@ int avcodec_fill_audio_frame(AVFrame *frame, int nb_channels, int buf_size, int align); /** - * Reset the internal decoder state / flush internal buffers. Should be called + * Reset the internal codec state / flush internal buffers. Should be called * e.g. when seeking or when switching to a different stream. * - * @note when refcounted frames are not used (i.e. avctx->refcounted_frames is 0), - * this invalidates the frames previously returned from the decoder. When - * refcounted frames are used, the decoder just releases any references it might - * keep internally, but the caller's reference remains valid. + * @note for decoders, when refcounted frames are not used + * (i.e. avctx->refcounted_frames is 0), this invalidates the frames previously + * returned from the decoder. When refcounted frames are used, the decoder just + * releases any references it might keep internally, but the caller's reference + * remains valid. + * + * @note for encoders, this function will only do something if the encoder + * declares support for AV_CODEC_CAP_ENCODER_FLUSH. */ void avcodec_flush_buffers(AVCodecContext *avctx); diff --git a/libavcodec/decode.c b/libavcodec/decode.c index b7ae1fbb84..d4bdb9b1c0 100644 --- a/libavcodec/decode.c +++ b/libavcodec/decode.c @@ -2084,6 +2084,21 @@ void avcodec_flush_buffers(AVCodecContext *avctx) { AVCodecInternal *avci = avctx->internal; + if (av_codec_is_encoder(avctx->codec)) { + int caps = avctx->codec->capabilities; + + if (!(caps & AV_CODEC_CAP_ENCODER_FLUSH)) { + // Only encoders that explicitly declare support for it can be + // flushed. Otherwise, this is a no-op. + av_log(avctx, AV_LOG_WARNING, "Ignoring attempt to flush encoder " + "that doesn't support it\n"); + return; + } + + // We haven't implemented flushing for frame-threaded encoders. + av_assert0(!(caps & AV_CODEC_CAP_FRAME_THREADS)); + } + avci->draining = 0; avci->draining_done = 0; avci->nb_draining_errors = 0; diff --git a/libavcodec/nvenc_h264.c b/libavcodec/nvenc_h264.c index 479155fe15..02392db46a 100644 --- a/libavcodec/nvenc_h264.c +++ b/libavcodec/nvenc_h264.c @@ -214,7 +214,8 @@ AVCodec ff_nvenc_h264_encoder = { .priv_data_size = sizeof(NvencContext), .priv_class = &nvenc_h264_class, .defaults = defaults, - .capabilities = AV_CODEC_CAP_DELAY | AV_CODEC_CAP_HARDWARE, + .capabilities = AV_CODEC_CAP_DELAY | AV_CODEC_CAP_HARDWARE | + AV_CODEC_CAP_ENCODER_FLUSH, .caps_internal = FF_CODEC_CAP_INIT_CLEANUP, .pix_fmts = ff_nvenc_pix_fmts, .wrapper_name = "nvenc", @@ -244,7 +245,8 @@ AVCodec ff_h264_nvenc_encoder = { .priv_data_size = sizeof(NvencContext), .priv_class = &h264_nvenc_class, .defaults = defaults, - .capabilities = AV_CODEC_CAP_DELAY | AV_CODEC_CAP_HARDWARE, + .capabilities = AV_CODEC_CAP_DELAY | AV_CODEC_CAP_HARDWARE | + AV_CODEC_CAP_ENCODER_FLUSH, .caps_internal = FF_CODEC_CAP_INIT_CLEANUP, .pix_fmts = ff_nvenc_pix_fmts, .wrapper_name = "nvenc", diff --git a/libavcodec/nvenc_hevc.c b/libavcodec/nvenc_hevc.c index 7c9b3848f1..ea337a514f 100644 --- a/libavcodec/nvenc_hevc.c +++ b/libavcodec/nvenc_hevc.c @@ -174,7 +174,8 @@ AVCodec ff_nvenc_hevc_encoder = { .priv_class = &nvenc_hevc_class, .defaults = defaults, .pix_fmts = ff_nvenc_pix_fmts, - .capabilities = AV_CODEC_CAP_DELAY | AV_CODEC_CAP_HARDWARE, + .capabilities = AV_CODEC_CAP_DELAY | AV_CODEC_CAP_HARDWARE | + AV_CODEC_CAP_ENCODER_FLUSH, .caps_internal = FF_CODEC_CAP_INIT_CLEANUP, .wrapper_name = "nvenc", }; @@ -203,7 +204,8 @@ AVCodec ff_hevc_nvenc_encoder = { .priv_class = &hevc_nvenc_class, .defaults = defaults, .pix_fmts = ff_nvenc_pix_fmts, - .capabilities = AV_CODEC_CAP_DELAY | AV_CODEC_CAP_HARDWARE, + .capabilities = AV_CODEC_CAP_DELAY | AV_CODEC_CAP_HARDWARE | + AV_CODEC_CAP_ENCODER_FLUSH, .caps_internal = FF_CODEC_CAP_INIT_CLEANUP, .wrapper_name = "nvenc", }; diff --git a/libavcodec/version.h b/libavcodec/version.h index 278f6be0cf..a4eb83124c 100644 --- a/libavcodec/version.h +++ b/libavcodec/version.h @@ -28,8 +28,8 @@ #include "libavutil/version.h" #define LIBAVCODEC_VERSION_MAJOR 58 -#define LIBAVCODEC_VERSION_MINOR 78 -#define LIBAVCODEC_VERSION_MICRO 101 +#define LIBAVCODEC_VERSION_MINOR 79 +#define LIBAVCODEC_VERSION_MICRO 100 #define LIBAVCODEC_VERSION_INT AV_VERSION_INT(LIBAVCODEC_VERSION_MAJOR, \ LIBAVCODEC_VERSION_MINOR, \