diff mbox series

[FFmpeg-devel] avcodec: Vorbis decode: don't use a flag to determine if frames have been output

Message ID 20220908113632.1056-2-jyrkive@nekonyansoft.com
State Accepted
Commit 8fc2dedfe6e8fcc58dd052bf3b85cd4754133b17
Headers show
Series [FFmpeg-devel] avcodec: Vorbis decode: don't use a flag to determine if frames have been output | expand

Checks

Context Check Description
andriy/commit_msg_x86 warning Please wrap lines in the body of the commit message between 60 and 72 characters.
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished

Commit Message

jyrkive@nekonyansoft.com Sept. 8, 2022, 11:36 a.m. UTC
From: Jyrki Vesterinen <jyrkive@nekonyansoft.com>

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 records the presentation timestamp of the first frame and
determines after that if the new frame is the first frame (possible after
seeking to the start) by comparing its pts to the stored pts.
---
 libavcodec/vorbisdec.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

Comments

Paul B Mahol Sept. 8, 2022, 12:11 p.m. UTC | #1
On 9/8/22, jyrkive@nekonyansoft.com <jyrkive@nekonyansoft.com> wrote:
> From: Jyrki Vesterinen <jyrkive@nekonyansoft.com>
>
> 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 records the presentation timestamp of the first frame and
> determines after that if the new frame is the first frame (possible after
> seeking to the start) by comparing its pts to the stored pts.
> ---
>  libavcodec/vorbisdec.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/libavcodec/vorbisdec.c b/libavcodec/vorbisdec.c
> index 4d03947c49..38a5367be3 100644
> --- a/libavcodec/vorbisdec.c
> +++ b/libavcodec/vorbisdec.c
> @@ -131,6 +131,7 @@ typedef struct vorbis_context_s {
>
>      FFTContext mdct[2];
>      uint8_t       first_frame;
> +    int64_t       initial_pts;
>      uint32_t      version;
>      uint8_t       audio_channels;
>      uint32_t      audio_samplerate;
> @@ -1847,6 +1848,10 @@ static int vorbis_decode_frame(AVCodecContext *avctx,
> AVFrame *frame,
>
>      if (!vc->first_frame) {
>          vc->first_frame = 1;
> +        vc->initial_pts = frame->pts;
> +    }
> +
> +    if (frame->pts == vc->initial_pts) {
>          *got_frame_ptr = 0;
>          av_frame_unref(frame);
>          return buf_size;
> @@ -1881,7 +1886,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
>


LGTM

> _______________________________________________
> 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".
>
diff mbox series

Patch

diff --git a/libavcodec/vorbisdec.c b/libavcodec/vorbisdec.c
index 4d03947c49..38a5367be3 100644
--- a/libavcodec/vorbisdec.c
+++ b/libavcodec/vorbisdec.c
@@ -131,6 +131,7 @@  typedef struct vorbis_context_s {
 
     FFTContext mdct[2];
     uint8_t       first_frame;
+    int64_t       initial_pts;
     uint32_t      version;
     uint8_t       audio_channels;
     uint32_t      audio_samplerate;
@@ -1847,6 +1848,10 @@  static int vorbis_decode_frame(AVCodecContext *avctx, AVFrame *frame,
 
     if (!vc->first_frame) {
         vc->first_frame = 1;
+        vc->initial_pts = frame->pts;
+    }
+
+    if (frame->pts == vc->initial_pts) {
         *got_frame_ptr = 0;
         av_frame_unref(frame);
         return buf_size;
@@ -1881,7 +1886,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 = {