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 22FF443585 for ; Mon, 17 Oct 2022 08:15:46 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 0B22768BCFE; Mon, 17 Oct 2022 11:15:43 +0300 (EEST) Received: from mail-lj1-f175.google.com (mail-lj1-f175.google.com [209.85.208.175]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 758A368BC85 for ; Mon, 17 Oct 2022 11:15:36 +0300 (EEST) Received: by mail-lj1-f175.google.com with SMTP id c20so13011309ljj.7 for ; Mon, 17 Oct 2022 01:15:36 -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=PD5ZAO8Nb6hznFkPalyXqodqo80+cWAtXR+qHRB8WiQ=; b=V8w2y/dVebl/p6pF+F8U7S/OZzmaic0LdTKsNgkfFjtWW/JuinuPg9WSJPk+DXr1vw GtHNNVfMcXqcgBKnj9+3w3RQxPRQROXpznOY6ksisyqPDJ8AG3dOxrGhG4nLJSzWbbze guNwPLsGg5MNN9PUI63hMsQxifj7C1qLjIPVqxJ5e2TcU3T85X05geXB8SlC2HBu15+Z Ih01oAk/K6In9W0nD0FSl/LzRwjNS+7CeA2zp8UyqcaHoPzMAJGFqDYXTXX67mYxusVn v+mwC5HylPNfVqO9scYQNrJ0DeO49B0TFOVjaeiOwtenqonNlwDJH6bR3da4Ag60PWlq 4xaQ== 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=PD5ZAO8Nb6hznFkPalyXqodqo80+cWAtXR+qHRB8WiQ=; b=bhmNrux7V3Ka7DKlBMQXxFNlRG5MRQONky4PquzezR0pADNeZtuvb5te8xQQQSc1PS eHQjq/8xDScIeooRbvZ+2T0asiLk+HND3jy5N1jk/NIUsc203x74loB+8JWpFUQgqYcE UUJVlsHfxivB2yUqP880Y0futP9rqsS5hLzG3wE0h+C4OI7GMWYShrY9cHPM5D727ivE gc+eMFkbdHw9PFEFQmpE4EEp5r1AZCKrei/VaLW4wt/dA3tEA0dq7vHYZMNwr1yUrnPo cNqMqqeI94Dr9AnZSV7rdfUcU1THdvmIW7mOAzwNY3Ujw1NV+9DA1GlLbons8bG02G0F Wi7Q== X-Gm-Message-State: ACrzQf2jFUHbxQcJrORHRMgPL5pSXgWBelkkk4zpPqPMZENevuZDV7QB sT+nogXBSWZEbumBAEwAGv5euMDHayXbtnKPu7Kpq295 X-Google-Smtp-Source: AMsMyM48QEVcv0F1SIk/26XNpb5HN25tr2mldPtj8fybWbphK00eFH9nGcFimsRh73qR/DGe4m11bGfpg7Abch7EW5A= X-Received: by 2002:a2e:8548:0:b0:26d:fecc:5675 with SMTP id u8-20020a2e8548000000b0026dfecc5675mr3915548ljj.515.1665994534743; Mon, 17 Oct 2022 01:15:34 -0700 (PDT) MIME-Version: 1.0 References: <20220908082505.953-1-jyrkive@nekonyansoft.com> In-Reply-To: <20220908082505.953-1-jyrkive@nekonyansoft.com> From: Hendrik Leppkes Date: Mon, 17 Oct 2022 10:15:22 +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 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? - 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".