From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org [79.124.17.100]) by master.gitmailbox.com (Postfix) with ESMTP id 59AE9435A5 for ; Mon, 17 Oct 2022 08:23:34 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id C819F68BCDF; Mon, 17 Oct 2022 11:23:32 +0300 (EEST) Received: from mail-lf1-f52.google.com (mail-lf1-f52.google.com [209.85.167.52]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 4E79768BCD0 for ; Mon, 17 Oct 2022 11:23:26 +0300 (EEST) Received: by mail-lf1-f52.google.com with SMTP id s20so16305340lfi.11 for ; Mon, 17 Oct 2022 01:23:26 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=to:subject:message-id:date:from:in-reply-to:references:mime-version :from:to:cc:subject:date:message-id:reply-to; bh=I71moePBfOWv3DAYXJ+aqNP1h1EUxv6kuXRAH3gmYHY=; b=VE5XvPwNrpnbP9Ak05xuYKfsDJj1VWB2HelC9dqzoB8FaBzXA6PszeOWhhXKDU9qpj 12ThfYHK4oRdnuWub/AIvDggNGXnOqaxZbKSpXPM9vSiGed1RiVh9NUcrURKwGLYco5U Ylor4i9NuZxdO2kHu5SWrZFIfi2+3YHWnwaxPErd27mKiYF3iF8+0M92OiPSKJbZWGHt Okfps7lzx6a3CJy3Khc6BNFZ/32ZrpdzgjpFieA12ytm16uGEGaqqwuy0Fcn3xRZty+l VLMQJPEIqB4eLsdUyLUUXFsEvfh0pipE11WC8PEb1bTt8yYPNZga74ClRN/ti/0YQK6p g8hw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=to:subject:message-id:date:from:in-reply-to:references:mime-version :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=I71moePBfOWv3DAYXJ+aqNP1h1EUxv6kuXRAH3gmYHY=; b=L585txJAAXsA3tfscbQz7NrpCMQm142tspX1/wHXUkxDHvuVKDgJHaddZez3vmh8jW Mt/ThWizotjOQQhyw5EqOKanAbdUjXuGTFSC4x/GKc901w2Kk/sPbC5/0xwOs2D34zuk oOyWwpD3G+OEEWeK0de+SDmLb9Ix1te1AUbpLW8zai1JpHXdw3yXfK3L+eRGjGHf211V IV+2bwDVJ5rH3ZNG/oYhzaDyJ8/NzbPJckpSz9fLWaZ95WbNj1EqYqtclPfC8fkjyXra xRN5m2dGyXYX1bZQ1Ptik82mj0HlFZUI7UR9AMzpmPEsip/2OCDgO69ronQfcx8Q5nmc aOfQ== X-Gm-Message-State: ACrzQf2eP7HeRYz+Sze6L8lLHimD78A7ul40LDs0SCpGSU+5v3ourIFH rxxyK5XwC5qOw1j3mapS9DbcRCHWoS5X/FwBATc/Y1pw X-Google-Smtp-Source: AMsMyM5K8VbUpzbKOveA7M+eYPWTvewVeEt/k6Y/NeK3M1h8VdRgFtu1GWPQmFabiPDLd1gK12aEfUYmrt8KjmMbJM0= X-Received: by 2002:a19:f618:0:b0:4a2:4f89:163e with SMTP id x24-20020a19f618000000b004a24f89163emr3312811lfe.684.1665995004548; Mon, 17 Oct 2022 01:23:24 -0700 (PDT) MIME-Version: 1.0 References: <20220908082505.953-1-jyrkive@nekonyansoft.com> In-Reply-To: From: Hendrik Leppkes Date: Mon, 17 Oct 2022 10:23:11 +0200 Message-ID: To: FFmpeg development discussions and patches Subject: Re: [FFmpeg-devel] [PATCH] avcodec: Vorbis decode: don't use a flag to determine if frames have been output 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 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: On Mon, Oct 17, 2022 at 10:18 AM Paul B Mahol wrote: > > On 10/17/22, Hendrik Leppkes wrote: > > On Thu, Sep 8, 2022 at 10:26 AM wrote: > >> > >> From: Jyrki Vesterinen > >> > >> If a developer using FFmpeg libraries seeks into an earlier position and > >> calls > >> avcodec_flush_buffers() afterwards as recommended, the Vorbis decoder will > >> drop > >> the next frame, since buffer flushing clears the first_frame flag. As a > >> result, > >> the audio samples the calling code receives may be ahead of the requested > >> seek > >> position, which is unacceptable in some use cases such as playing a > >> looping > >> sound effect. > >> > >> This commit removes the first_frame flag entirely and instead uses the > >> presentation timestamp to determine if it's the first frame. > >> --- > >> libavcodec/vorbisdec.c | 5 +---- > >> 1 file changed, 1 insertion(+), 4 deletions(-) > >> > >> diff --git a/libavcodec/vorbisdec.c b/libavcodec/vorbisdec.c > >> index 4d03947c49..d4b030d7b9 100644 > >> --- a/libavcodec/vorbisdec.c > >> +++ b/libavcodec/vorbisdec.c > >> @@ -130,7 +130,6 @@ typedef struct vorbis_context_s { > >> AVFloatDSPContext *fdsp; > >> > >> FFTContext mdct[2]; > >> - uint8_t first_frame; > >> uint32_t version; > >> uint8_t audio_channels; > >> uint32_t audio_samplerate; > >> @@ -1845,8 +1844,7 @@ static int vorbis_decode_frame(AVCodecContext > >> *avctx, AVFrame *frame, > >> if ((len = vorbis_parse_audio_packet(vc, channel_ptrs)) <= 0) > >> return len; > >> > >> - if (!vc->first_frame) { > >> - vc->first_frame = 1; > >> + if (frame->pts < 0) { > >> *got_frame_ptr = 0; > >> av_frame_unref(frame); > >> return buf_size; > >> @@ -1881,7 +1879,6 @@ static av_cold void > >> vorbis_decode_flush(AVCodecContext *avctx) > >> sizeof(*vc->saved)); > >> } > >> vc->previous_window = -1; > >> - vc->first_frame = 0; > >> } > >> > >> const FFCodec ff_vorbis_decoder = { > >> -- > >> 2.37.2.windows.2 > >> > > > > This change seems to be rather fragile and faulty, causing vorbis > > decoding to fail in various scenarios for a bunch of downstream > > projects. > > > > - A user may not set pts at all, resulting in all frames being dropped > > (pure audio files don't necessarily need timestamps) > > - A seek could happen before any frame is ever decoded, resulting in > > wrong drops, potentially in the middle of playback if the user seeks > > backwards after opening in the middle. > > > > In general, using timestamps to control decoder behavior is often just > > wrong, as timestamps are not reliable, and most importantly, not tied > > to the bitstream at all. > > > > Can we revert this and re-think the approach? > > Are you saying that previous solution was better than current one? > > By your own words its ever worse that current state. > At least the old solution consistently just dropped one frame after a flush, not in the middle of playback, or dropping every single frame because the user did not specify timestamps, breaking playback entirely. We already have mechanisms to properly drop padding data from the front of a stream in generic code, that should ideally be used, and not a decoder-specific hack. - Hendrik _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".