Re: [FFmpeg-devel] [PATCH] libopenmpt: fix seeking

[Michael Niedermayer <michael@niedermayer.cc>]

[-- Attachment #1.1: Type: text/plain, Size: 3319 bytes --]

Hi

On Mon, Jul 28, 2025 at 07:58:18AM +0500, kimapr via ffmpeg-devel wrote:
> This patch fixes strange seeking behavior i have observed in mpv when using the libopenmpt demuxer, caused by mismatch in position state between the demuxer and underlying libopenmpt library.  Not setting the presentation timestamp field of the AVPacket caused it to be guessed by libavformat, but the guess didn't take seeking into account, so after seeking libopenmpt produced packets at the new seeked position but they were then presented as if they belong to the old position!  A quick check for this behavior is to open a tracker tracker module that is, for example 2 minutes and a few seconds long in mpv, and then press "up" 2 times (which would seek 2 minutes forward): buggy demuxer causes mpv to exit immediately after seek because of an EOF,  with non-buggy demuxer mpv will play the last few seconds of the song and then exit.
> 
> I found the bug when writing my own demuxer for an obscure audio format (which i'm not quite sure if i should submit too) as i was using the libopenmpt one as a reference and copied the bug too.  Testing was done on 6.1.1, but libopenmpt demuxer's code didn't really change since then so it should be relevant for the latest one too (unless the default value for pkt->pts changed to take seeking into account), and either way it makes more sense to expose information that is very much available to the demuxer rather than leave it to be guessed.

>  libopenmpt.c |    3 +++
>  1 file changed, 3 insertions(+)
> fd581b2b26ac22fc5bdcac053c98d9b541052fb2  0001-libopenmpt-fix-seeking.patch
> From 451691febac466cee37d9b836228e30c53813d60 Mon Sep 17 00:00:00 2001
> From: Kimapr <kimapr.fr@gmail.com>
> Date: Mon, 28 Jul 2025 06:32:27 +0500
> Subject: [PATCH] libopenmpt: fix seeking
> 
> ---
>  libavformat/libopenmpt.c | 3 +++
>  1 file changed, 3 insertions(+)

while your email contains quite some details, the commit message of
just "libopenmpt: fix seeking" is too terse


> 
> diff --git a/libavformat/libopenmpt.c b/libavformat/libopenmpt.c
> index c270a60cb2..d383d65ad8 100644
> --- a/libavformat/libopenmpt.c
> +++ b/libavformat/libopenmpt.c
> @@ -171,6 +171,8 @@ static int read_packet_openmpt(AVFormatContext *s, AVPacket *pkt)
>      if ((ret = av_new_packet(pkt, AUDIO_PKT_SIZE)) < 0)
>          return ret;
>  
> +    double pos = openmpt_module_get_position_seconds(openmpt->module);

is there any possibility of a "unknown" "position" ?
if not its fine otherwise a check may be needed to handle that



> +
>      switch (openmpt->ch_layout.nb_channels) {
>      case 1:
>          ret = openmpt_module_read_float_mono(openmpt->module, openmpt->sample_rate,
> @@ -195,6 +197,7 @@ static int read_packet_openmpt(AVFormatContext *s, AVPacket *pkt)
>      }
>  
>      pkt->size = ret * (openmpt->ch_layout.nb_channels * 4);
> +    pkt->pts = llrint(pos * AV_TIME_BASE);

is it possible that this exceeds the int64_t range ?
if so these out of range values should probably be replaced by AV_NOPTS_VALUE

thx

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

I have never wished to cater to the crowd; for what I know they do not
approve, and what they approve I do not know. -- Epicurus

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
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".