From patchwork Sun Apr 9 13:27:45 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Anton Khirnov X-Patchwork-Id: 41041 Delivered-To: ffmpegpatchwork2@gmail.com Received: by 2002:a05:6a20:4645:b0:e3:3194:9d20 with SMTP id eb5csp1572058pzb; Sun, 9 Apr 2023 06:29:37 -0700 (PDT) X-Google-Smtp-Source: AKy350bE0gSr94ptXAWTo4t9mHKPn4qEOMFs0gD3+oXN0b82rnKIky3eVxEGQ2R/SFEIw9jHBwE8 X-Received: by 2002:a17:906:2790:b0:8f1:937c:f450 with SMTP id j16-20020a170906279000b008f1937cf450mr4867587ejc.13.1681046977328; Sun, 09 Apr 2023 06:29:37 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1681046977; cv=none; d=google.com; s=arc-20160816; b=TfHVh9gR5f6I64C2EKKUZlkuz9SNinyvWDRD72RsEHrSqsSrjv8joudMVaSzyLgObs f/VplwS0ys6PlTQZvneQ257hKbVWgTy/823d7Z+INFcOGv0hVzphp9c4dqTIrhK38UNo n7uIvT11pFrvMnfevjXf0YPT7f19FU7b7zjM9+v4nOgyMUu87mlrBhyMkwj+SrWBysXL D/0jEBWJHg+QaVC6PtxaQL9+Rz7L0HleFJMjaScPoqWiK5b8a8u/JK9dhBesIpfj6/Jf C6Jl2wjt6GoI1Q+xYAR9qtjn9HCe0BKLYbmtccrCtvaYUlPpCAlxXHF6F4GWR/I122HR w5gg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=sender:errors-to:content-transfer-encoding:reply-to:list-subscribe :list-help:list-post:list-archive:list-unsubscribe:list-id :precedence:subject:mime-version:references:in-reply-to:message-id :date:to:from:delivered-to; bh=B47MyMLzUKGQjqgxNVmaZCFXHv9Xn+7My1rS47AyfiQ=; b=kB0l1/OF72T7WJWNiofgfJoSRmuYBuAJucoPUUFuJ3bZhc8f7lIxpv+cuCsO3qfSP2 pklI9TmXgk7rXS+aPqtxzL1ea0C5BgBtV2hZS6tly8HPfNVEjGl/BNpP3odIkCwS6gg0 etUZA+d6Ytx6m2Uzo0mn2zRWMhVhAhG8S19rMfM8jHMMnE59FvR+J1bTGD8yB4PuwXGL MQ7JgaUS5RgEjRZEGTiUoQh//JeCFcw8mPmaKkj9rq6vDqkd29zOBry0HcygCkf3RqgU zvLbIUBEgMZnw5fijIuUGlyNMXHV9kWFTvi2581iGAMleEiXbO+PvEjEPzzuBjs3ToJ/ ez4Q== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: domain of ffmpeg-devel-bounces@ffmpeg.org designates 79.124.17.100 as permitted sender) smtp.mailfrom=ffmpeg-devel-bounces@ffmpeg.org Return-Path: Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org. [79.124.17.100]) by mx.google.com with ESMTP id ho20-20020a1709070e9400b0094a5a281168si2638851ejc.801.2023.04.09.06.29.37; Sun, 09 Apr 2023 06:29:37 -0700 (PDT) Received-SPF: pass (google.com: domain of ffmpeg-devel-bounces@ffmpeg.org designates 79.124.17.100 as permitted sender) client-ip=79.124.17.100; Authentication-Results: mx.google.com; spf=pass (google.com: domain of ffmpeg-devel-bounces@ffmpeg.org designates 79.124.17.100 as permitted sender) smtp.mailfrom=ffmpeg-devel-bounces@ffmpeg.org Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 386F068BBFD; Sun, 9 Apr 2023 16:28:56 +0300 (EEST) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mail0.khirnov.net (red.khirnov.net [176.97.15.12]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id D634A68BBAF for ; Sun, 9 Apr 2023 16:28:49 +0300 (EEST) Received: from localhost (localhost [IPv6:::1]) by mail0.khirnov.net (Postfix) with ESMTP id A0F24240591 for ; Sun, 9 Apr 2023 15:28:45 +0200 (CEST) Received: from mail0.khirnov.net ([IPv6:::1]) by localhost (mail0.khirnov.net [IPv6:::1]) (amavisd-new, port 10024) with ESMTP id EXr7GuzQRRFW for ; Sun, 9 Apr 2023 15:28:44 +0200 (CEST) Received: from libav.khirnov.net (libav.khirnov.net [IPv6:2a00:c500:561:201::7]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256 client-signature RSA-PSS (2048 bits) client-digest SHA256) (Client CN "libav.khirnov.net", Issuer "smtp.khirnov.net SMTP CA" (verified OK)) by mail0.khirnov.net (Postfix) with ESMTPS id 7FA092405B5 for ; Sun, 9 Apr 2023 15:28:43 +0200 (CEST) Received: from libav.khirnov.net (libav.khirnov.net [IPv6:::1]) by libav.khirnov.net (Postfix) with ESMTP id 37D1C3A0567 for ; Sun, 9 Apr 2023 15:28:37 +0200 (CEST) From: Anton Khirnov To: ffmpeg-devel@ffmpeg.org Date: Sun, 9 Apr 2023 15:27:45 +0200 Message-Id: <20230409132746.19756-6-anton@khirnov.net> X-Mailer: git-send-email 2.39.1 In-Reply-To: <20230409132746.19756-1-anton@khirnov.net> References: <20230409132746.19756-1-anton@khirnov.net> MIME-Version: 1.0 Subject: [FFmpeg-devel] [PATCH 6/7] fftools/ffmpeg: change video decoding timestamp generation X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.29 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" X-TUID: eeJ9P0UeeqvW When no timestamps are available from the container, the video decoding code will currently use fake dts values - generated in process_input_packet() based on a combination of information from the decoder and the parser (obtained via the demuxer) - to generate timestamps during decoder flushing. This is fragile, hard to follow, and unnecessarily convoluted, since more reliable information can be obtained directly from post-decoding values. The new code keeps track of the last decoded frame pts and estimates its duration based on a number of heuristics. Timestamps generated when both pts and pkt_dts are missing are then simple pts+duration of the last frame. The heuristics are somewhat complicated by the fact that lavf insists on making up packet timestamps based on its highly incomplete information. That should be removed in the future, allowing to further simplify this code. The results of the following tests change: * h264-3386 now requires -fps_mode passthrough to avoid dropping frames at the end; this is a pathology of the interaction of the new and old code, and the fact that the sample switches from field to frame coding in the last packet, and will be fixed in following commits * hevc-conformance-DELTAQP_A_BRCM_4 stops inventing an arbitrary timestamp gap at the end * hevc-small422chroma - the single frame output by this test now has a timestamp of 0, rather than an arbitrary 7 --- fftools/ffmpeg.c | 79 ++++++++++++++----- fftools/ffmpeg.h | 9 ++- fftools/ffmpeg_demux.c | 3 +- tests/fate/h264.mak | 2 +- tests/ref/fate/h264-3386 | 6 +- .../fate/hevc-conformance-DELTAQP_A_BRCM_4 | 2 +- tests/ref/fate/hevc-small422chroma | 2 +- 7 files changed, 75 insertions(+), 28 deletions(-) diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c index d38054be2f1..eaaa4ff7970 100644 --- a/fftools/ffmpeg.c +++ b/fftools/ffmpeg.c @@ -2204,11 +2204,63 @@ static int decode_audio(InputStream *ist, AVPacket *pkt, int *got_output, return err < 0 ? err : ret; } +static int64_t video_duration_estimate(const InputStream *ist, const AVFrame *frame) +{ + const InputFile *ifile = input_files[ist->file_index]; + const int container_nots = !!(ifile->ctx->iformat->flags & AVFMT_NOTIMESTAMPS); + int64_t codec_duration = 0; + + // XXX lavf currently makes up frame durations when they are not provided by + // the container. As there is no way to reliably distinguish real container + // durations from the fake made-up ones, we use heuristics based on whether + // the container has timestamps. Eventually lavf should stop making up + // durations, then this should be simplified. + + // prefer frame duration for containers with timestamps + if (frame->duration > 0 && !container_nots) + return frame->duration; + + if (ist->dec_ctx->framerate.den && ist->dec_ctx->framerate.num) { + int ticks = frame->repeat_pict >= 0 ? + frame->repeat_pict + 1 : + ist->dec_ctx->ticks_per_frame; + codec_duration = av_rescale_q(ticks, av_inv_q(ist->dec_ctx->framerate), + ist->st->time_base); + } + + // prefer codec-layer duration for containers without timestamps + if (codec_duration > 0 && container_nots) + return codec_duration; + + // when timestamps are available, repeat last frame's actual duration + // (i.e. pts difference between this and last frame) + if (frame->pts != AV_NOPTS_VALUE && ist->last_frame_pts != AV_NOPTS_VALUE && + frame->pts > ist->last_frame_pts) + return frame->pts - ist->last_frame_pts; + + // try frame/codec duration + if (frame->duration > 0) + return frame->duration; + if (codec_duration > 0) + return codec_duration; + + // try average framerate + if (ist->st->avg_frame_rate.num && ist->st->avg_frame_rate.den) { + int64_t d = av_rescale_q(1, av_inv_q(ist->st->avg_frame_rate), + ist->st->time_base); + if (d > 0) + return d; + } + + // last resort is last frame's estimated duration, and 1 + return FFMAX(ist->last_frame_duration_est, 1); +} + static int decode_video(InputStream *ist, AVPacket *pkt, int *got_output, int64_t *duration_pts, int eof, int *decode_failed) { AVFrame *decoded_frame = ist->decoded_frame; - int i, ret = 0, err = 0; + int ret = 0, err = 0; int64_t best_effort_timestamp; int64_t dts = AV_NOPTS_VALUE; @@ -2224,16 +2276,6 @@ static int decode_video(InputStream *ist, AVPacket *pkt, int *got_output, int64_ pkt->dts = dts; // ffmpeg.c probably shouldn't do this } - // The old code used to set dts on the drain packet, which does not work - // with the new API anymore. - if (eof) { - void *new = av_realloc_array(ist->dts_buffer, ist->nb_dts_buffer + 1, sizeof(ist->dts_buffer[0])); - if (!new) - return AVERROR(ENOMEM); - ist->dts_buffer = new; - ist->dts_buffer[ist->nb_dts_buffer++] = dts; - } - update_benchmark(NULL); ret = decode(ist, ist->dec_ctx, decoded_frame, got_output, pkt); update_benchmark("decode_video %d.%d", ist->file_index, ist->st->index); @@ -2292,13 +2334,10 @@ static int decode_video(InputStream *ist, AVPacket *pkt, int *got_output, int64_ if (ist->framerate.num) best_effort_timestamp = ist->cfr_next_pts++; - if (eof && best_effort_timestamp == AV_NOPTS_VALUE && ist->nb_dts_buffer > 0) { - best_effort_timestamp = ist->dts_buffer[0]; - - for (i = 0; i < ist->nb_dts_buffer - 1; i++) - ist->dts_buffer[i] = ist->dts_buffer[i + 1]; - ist->nb_dts_buffer--; - } + // no timestamp available - extrapolate from previous frame duration + if (best_effort_timestamp == AV_NOPTS_VALUE && + ist->last_frame_pts != AV_NOPTS_VALUE) + best_effort_timestamp = ist->last_frame_pts + ist->last_frame_duration_est; if (best_effort_timestamp == AV_NOPTS_VALUE) best_effort_timestamp = av_rescale_q(ist->pts, AV_TIME_BASE_Q, ist->st->time_base); @@ -2310,6 +2349,10 @@ static int decode_video(InputStream *ist, AVPacket *pkt, int *got_output, int64_ ist->next_pts = ist->pts = ts; } + // update timestamp history + ist->last_frame_duration_est = video_duration_estimate(ist, decoded_frame); + ist->last_frame_pts = decoded_frame->pts; + if (debug_ts) { av_log(ist, AV_LOG_INFO, "decoder -> pts:%s pts_time:%s " diff --git a/fftools/ffmpeg.h b/fftools/ffmpeg.h index 823218e2142..81586c3e45c 100644 --- a/fftools/ffmpeg.h +++ b/fftools/ffmpeg.h @@ -369,6 +369,12 @@ typedef struct InputStream { int64_t next_pts; ///< synthetic pts for the next decode frame (in AV_TIME_BASE units) int64_t pts; ///< current pts of the decoded frame (in AV_TIME_BASE units) + + // pts/estimated duration of the last decoded video frame + // in decoder timebase + int64_t last_frame_pts; + int64_t last_frame_duration_est; + int wrap_correction_done; // the value of AVCodecParserContext.repeat_pict from the AVStream parser @@ -433,9 +439,6 @@ typedef struct InputStream { uint64_t frames_decoded; uint64_t samples_decoded; - int64_t *dts_buffer; - int nb_dts_buffer; - int got_output; } InputStream; diff --git a/fftools/ffmpeg_demux.c b/fftools/ffmpeg_demux.c index db05ddb8e90..7be7789b3ea 100644 --- a/fftools/ffmpeg_demux.c +++ b/fftools/ffmpeg_demux.c @@ -494,7 +494,6 @@ static void ist_free(InputStream **pist) av_frame_free(&ist->sub2video.frame); av_freep(&ist->filters); av_freep(&ist->hwaccel_device); - av_freep(&ist->dts_buffer); avcodec_free_context(&ist->dec_ctx); avcodec_parameters_free(&ist->par); @@ -829,6 +828,8 @@ static void add_input_streams(const OptionsContext *o, Demuxer *d) ist->framerate_guessed = av_guess_frame_rate(ic, st, NULL); + ist->last_frame_pts = AV_NOPTS_VALUE; + break; case AVMEDIA_TYPE_AUDIO: { int guess_layout_max = INT_MAX; diff --git a/tests/fate/h264.mak b/tests/fate/h264.mak index c8ed23955ac..7998879ed31 100644 --- a/tests/fate/h264.mak +++ b/tests/fate/h264.mak @@ -444,7 +444,7 @@ fate-h264-mixed-nal-coding: CMD = framecrc -i $(TARGET_SAM fate-h264-ref-pic-mod-overflow: CMD = framecrc -i $(TARGET_SAMPLES)/h264/ref-pic-mod-overflow.h264 fate-h264-twofields-packet: CMD = framecrc -i $(TARGET_SAMPLES)/h264/twofields_packet.mp4 -an -frames 30 fate-h264-unescaped-extradata: CMD = framecrc -i $(TARGET_SAMPLES)/h264/unescaped_extradata.mp4 -an -frames 10 -fate-h264-3386: CMD = framecrc -i $(TARGET_SAMPLES)/h264/bbc2.sample.h264 +fate-h264-3386: CMD = framecrc -i $(TARGET_SAMPLES)/h264/bbc2.sample.h264 -fps_mode passthrough fate-h264-missing-frame: CMD = framecrc -i $(TARGET_SAMPLES)/h264/nondeterministic_cut.h264 fate-h264-timecode: CMD = framecrc -i $(TARGET_SAMPLES)/h264/crew_cif_timecode-2.h264 diff --git a/tests/ref/fate/h264-3386 b/tests/ref/fate/h264-3386 index 421581f8c63..f1a25a2dc04 100644 --- a/tests/ref/fate/h264-3386 +++ b/tests/ref/fate/h264-3386 @@ -47,6 +47,6 @@ 0, 45, 45, 1, 2332800, 0x80b968d3 0, 46, 46, 1, 2332800, 0xa45f4e6f 0, 47, 47, 1, 2332800, 0xe0fcbade -0, 48, 48, 1, 2332800, 0x19568f4d -0, 49, 49, 1, 2332800, 0x33c53f59 -0, 50, 50, 1, 2332800, 0xbdbe8fbf +0, 47, 47, 1, 2332800, 0x19568f4d +0, 48, 48, 1, 2332800, 0x33c53f59 +0, 49, 49, 1, 2332800, 0xbdbe8fbf diff --git a/tests/ref/fate/hevc-conformance-DELTAQP_A_BRCM_4 b/tests/ref/fate/hevc-conformance-DELTAQP_A_BRCM_4 index 5ca3faf58fb..2abd58b64be 100644 --- a/tests/ref/fate/hevc-conformance-DELTAQP_A_BRCM_4 +++ b/tests/ref/fate/hevc-conformance-DELTAQP_A_BRCM_4 @@ -98,4 +98,4 @@ 0, 92, 92, 1, 3133440, 0x761571be 0, 93, 93, 1, 3133440, 0x34dc14a1 0, 94, 94, 1, 3133440, 0xbb94c2d4 -0, 96, 96, 1, 3133440, 0x5300e459 +0, 95, 95, 1, 3133440, 0x5300e459 diff --git a/tests/ref/fate/hevc-small422chroma b/tests/ref/fate/hevc-small422chroma index de0ea46c31a..dca2af0a1a3 100644 --- a/tests/ref/fate/hevc-small422chroma +++ b/tests/ref/fate/hevc-small422chroma @@ -3,4 +3,4 @@ #codec_id 0: rawvideo #dimensions 0: 3840x2160 #sar 0: 1/1 -0, 7, 7, 1, 33177600, 0x53015e18 +0, 0, 0, 1, 33177600, 0x53015e18