From patchwork Tue Aug 18 19:53:32 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: James Almer X-Patchwork-Id: 21714 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 8FBFC44A88C for ; Tue, 18 Aug 2020 23:00:55 +0300 (EEST) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 659826880C8; Tue, 18 Aug 2020 23:00:55 +0300 (EEST) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mail-pl1-f196.google.com (mail-pl1-f196.google.com [209.85.214.196]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id DFEE9680A1D for ; Tue, 18 Aug 2020 23:00:48 +0300 (EEST) Received: by mail-pl1-f196.google.com with SMTP id y10so8154706plr.11 for ; Tue, 18 Aug 2020 13:00:48 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:subject:date:message-id:in-reply-to:references:mime-version :content-transfer-encoding; bh=VvKU5SYxUpRxlkfE/yQ4UsoNHQ5vnvlQED0rEGlrnJc=; b=L+I/gA78rBcbcsttmr4OZOt4pXDvnJzLQ7IL/PoiMV+SroI+qcDA8NmfaqIGGCAxgW IZ9fcR4r0hqslhCVZwnpJ+3X7HmN7RNHIPSOf+WqoJqQFOJ+yL4wpu1IehXjgDFSklMp PiM/uW4CLN/N7AEn7cYuEW0vbZNxRP4UsKuwvprvYJvEDPhhkfCv19SwVl9kGWlwEKos /3QOh9bBkGtQqr5N9Yl+HS64910gC0rRyL++o6HntpTPeDLIcbVRshH9of9f1tqcPYuf WEsE6CMbw6NNREFOsF+740wLfQuotxJgPB60dG9jOzniP/hF3NTr/9Tko7SHgq6+tZLf 2npg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=VvKU5SYxUpRxlkfE/yQ4UsoNHQ5vnvlQED0rEGlrnJc=; b=dZx92m9IuZ71xxhZ54NSzM06EnKlOulWVvyc44ujWb9VbUbmCabs0OtrW2dgYg6U3p V3/D50U0d+yKhhr75/FSn3AjLzSX+IcTlU7A131llMrDmzXiJ/b/cgXZuAoqmL08A1g0 5zpoTVlnQSiK/9g3YBqPNaIx7Z5BuzTJCNcosugayIsMvgtbCYVWOciW/5hDFfVDy1g2 Dmi5vAZg6gepPLCrUW+zyuaocz6T993uxJlyKsAmIi6Mh4jfUrxZMZwYoePel5f7IhHe RXbaQqk34nhnUu4prz3MtpQe6D0L0grZSrp/Bxm85E5EU2yIjKE3vPI1WpmZzqJVgsJN vMMA== X-Gm-Message-State: AOAM532M1iZ9iaz/UyBjgS3z5Wj1Ug1a6pJkNBQYkIA+jD81eGZehvMT Y0zhvM7E4g6hH68lwjfD9Ypw/lGZ5OY= X-Google-Smtp-Source: ABdhPJzU1uaJE9KQBlxb82Gn1KHtHP/jyPBLsg1gEL0VPYm93o0MFK066SxFWfeYoFKChV28ysqaLw== X-Received: by 2002:a0c:fa50:: with SMTP id k16mr19806527qvo.132.1597780465148; Tue, 18 Aug 2020 12:54:25 -0700 (PDT) Received: from localhost.localdomain ([191.83.214.65]) by smtp.gmail.com with ESMTPSA id 2sm21566453qki.46.2020.08.18.12.54.23 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 18 Aug 2020 12:54:24 -0700 (PDT) From: James Almer To: ffmpeg-devel@ffmpeg.org Date: Tue, 18 Aug 2020 16:53:32 -0300 Message-Id: <20200818195332.9442-3-jamrial@gmail.com> X-Mailer: git-send-email 2.27.0 In-Reply-To: <20200818195332.9442-1-jamrial@gmail.com> References: <20200818195332.9442-1-jamrial@gmail.com> MIME-Version: 1.0 Subject: [FFmpeg-devel] [PATCH 2/2] avcodec/decode: use a packet list to store packet properties 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 Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Keeping only the latest packet fed to the decoder works only for decoders that return a frame immediately after every consumed packet. Decoders that consume several packets before they return a frame will fill said frame with properties taken from the last consumed packet instead of the earliest. Signed-off-by: James Almer --- This essentially enables decoders implementing the AVCodec.receive_frame API to not require having to deal with timestamps and such properties internally, and letting ff_decode_frame_props() do all the work instead (See libdav1d, which i'll adapt once this goes in). As an example, this fixes the timestamps for the audio track in bink/binkaudio_rdft.bik from the FATE suite, where the decoder wrongly assumed ff_decode_frame_props() alone would handle them right. The reason the relevant fate test is unaffected is because it just compares the decoded output with a reference pcm file. Haven't looked too much into it, but i assume this change could simplify some AV_CODEC_CAP_DELAY decoders that keep track of timestamps, even those using the old AVCodec.decode() API. libavcodec/decode.c | 34 +++++++++++++++++++++++++--------- libavcodec/internal.h | 2 ++ libavcodec/utils.c | 3 +++ 3 files changed, 30 insertions(+), 9 deletions(-) diff --git a/libavcodec/decode.c b/libavcodec/decode.c index da587ac1f6..de36e28d3c 100644 --- a/libavcodec/decode.c +++ b/libavcodec/decode.c @@ -43,6 +43,7 @@ #include "decode.h" #include "hwconfig.h" #include "internal.h" +#include "packet_internal.h" #include "thread.h" typedef struct FramePool { @@ -142,15 +143,22 @@ fail2: return 0; } -static int extract_packet_props(AVCodecInternal *avci, const AVPacket *pkt) +#define IS_EMPTY(pkt) (!(pkt)->data && !(pkt)->side_data_elems) + +static int extract_packet_props(AVCodecInternal *avci, AVPacket *pkt) { int ret = 0; - av_packet_unref(avci->last_pkt_props); - if (pkt) { - ret = av_packet_copy_props(avci->last_pkt_props, pkt); - if (!ret) - avci->last_pkt_props->size = pkt->size; // HACK: Needed for ff_decode_frame_props(). + ret = avpriv_packet_list_put(&avci->pkt_props, &avci->pkt_props_tail, pkt, + FF_PACKETLIST_FLAG_REF_PACKET); + if (ret < 0) + return ret; + + if (IS_EMPTY(avci->last_pkt_props)) { + ret = avpriv_packet_list_get(&avci->pkt_props, + &avci->pkt_props_tail, + avci->last_pkt_props); + av_assert0(ret != AVERROR(EAGAIN)); } return ret; } @@ -512,6 +520,7 @@ FF_ENABLE_DEPRECATION_WARNINGS if (ret >= pkt->size || ret < 0) { av_packet_unref(pkt); + av_packet_unref(avci->last_pkt_props); } else { int consumed = ret; @@ -550,9 +559,11 @@ static int decode_receive_frame_internal(AVCodecContext *avctx, AVFrame *frame) av_assert0(!frame->buf[0]); - if (avctx->codec->receive_frame) + if (avctx->codec->receive_frame) { ret = avctx->codec->receive_frame(avctx, frame); - else + if (ret != AVERROR(EAGAIN)) + av_packet_unref(avci->last_pkt_props); + } else ret = decode_simple_receive_frame(avctx, frame); if (ret == AVERROR_EOF) @@ -1683,7 +1694,7 @@ static int add_metadata_from_side_data(const AVPacket *avpkt, AVFrame *frame) int ff_decode_frame_props(AVCodecContext *avctx, AVFrame *frame) { - const AVPacket *pkt = avctx->internal->last_pkt_props; + AVPacket *pkt = avctx->internal->last_pkt_props; int i; static const struct { enum AVPacketSideDataType packet; @@ -1701,6 +1712,11 @@ int ff_decode_frame_props(AVCodecContext *avctx, AVFrame *frame) { AV_PKT_DATA_S12M_TIMECODE, AV_FRAME_DATA_S12M_TIMECODE }, }; + if (IS_EMPTY(pkt)) + avpriv_packet_list_get(&avctx->internal->pkt_props, + &avctx->internal->pkt_props_tail, + pkt); + if (pkt) { frame->pts = pkt->pts; #if FF_API_PKT_PTS diff --git a/libavcodec/internal.h b/libavcodec/internal.h index 5d0e6e7831..ce4dbbc2b9 100644 --- a/libavcodec/internal.h +++ b/libavcodec/internal.h @@ -145,6 +145,8 @@ typedef struct AVCodecInternal { * for decoding. */ AVPacket *last_pkt_props; + AVPacketList *pkt_props; + AVPacketList *pkt_props_tail; /** * temporary buffer used for encoders to store their bitstream diff --git a/libavcodec/utils.c b/libavcodec/utils.c index 14cb5cf1aa..4a180a1500 100644 --- a/libavcodec/utils.c +++ b/libavcodec/utils.c @@ -50,6 +50,7 @@ #include "thread.h" #include "frame_thread_encoder.h" #include "internal.h" +#include "packet_internal.h" #include "put_bits.h" #include "raw.h" #include "bytestream.h" @@ -1149,6 +1150,8 @@ av_cold int avcodec_close(AVCodecContext *avctx) av_packet_free(&avctx->internal->compat_encode_packet); av_packet_free(&avctx->internal->buffer_pkt); av_packet_free(&avctx->internal->last_pkt_props); + avpriv_packet_list_free(&avctx->internal->pkt_props, + &avctx->internal->pkt_props_tail); av_packet_free(&avctx->internal->ds.in_pkt); av_frame_free(&avctx->internal->es.in_frame);