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

2025-08-03 Thread kimapr via ffmpeg-devel
On 2025/08/04 04:12, Michael Niedermayer wrote:
> can you split this in 3 patches, it sounds like these are 3 distinct changes
>
> thx

that seems a little excessive !  the 3 changes are all related and very small 
individually

also, i moved the patch to forgejo as was suggested by Lynne on IRC:  
https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/20096

further updates and discussion are there

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".


Re: [FFmpeg-devel] FFmpeg 6.1.3 and 7.0.3

2025-08-05 Thread kimapr via ffmpeg-devel
On 2025/08/05 01:02, kimapr via ffmpeg-devel wrote:

> Hi! it would be nice if my bug fix
>
>avformat/libopenmpt: fix seeking weirdness
>
> was backported to 6.1.3 and 7.0.3 (dunno about 5.1.7 as i haven't checked
> if libopenmpt even exists there). not sure how that would work from my side

I just checked, and it seems that my patch can be backported as far back
as 3.2 (it cannot be backported to 3.1 or earlier because libopenmpt demuxer
does not exist there, 3.2 code is similar enough to today's code)

On 5.0 and earlier there is a merge conflict because a line of code near my
change had changed a little bit so here's a version of the patch with conflict
solved for convenience, for later versions (including 5.1) just rebase 
ecef5f9e1f

i haven't tested the patch on top of anything older than 6.1.1 so i don't know
if the seeking problems exist there, but the patch shouldn't break anything
either as it doesn't depend on any APIs that were changed between 3.2 and latest
From 3dc81264cf6e5190c69ae35c87e0c2547b36b29b Mon Sep 17 00:00:00 2001
From: Kimapr 
Date: Mon, 28 Jul 2025 06:32:27 +0500
Subject: [PATCH] avformat/libopenmpt: fix seeking weirdness

- proper pts for packets. leaving it blank leaves it up for guessing,
  but the guess doesn't take seeking into account, causing weirdness.

- clamp to 0 when seeking to negative ts. libopenmpt docs are unclear on
  this but not doing this causes an immediate EOF when seeking backwards
  to the beginning in mpv.

- only set song duration and packet pts when they are non-negative and
  in int64 range. NaNs count as out of range. this isn't a fix for any
  specific issue but might be helpful still, and shouldn't break
  anything.
---
 libavformat/libopenmpt.c | 10 +-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/libavformat/libopenmpt.c b/libavformat/libopenmpt.c
index 8006c085df..1ed48fa900 100644
--- a/libavformat/libopenmpt.c
+++ b/libavformat/libopenmpt.c
@@ -150,7 +150,8 @@ static int read_header_openmpt(AVFormatContext *s)
 if (!st)
 return AVERROR(ENOMEM);
 avpriv_set_pts_info(st, 64, 1, AV_TIME_BASE);
-st->duration = llrint(openmpt->duration*AV_TIME_BASE);
+if (openmpt->duration >= 0 && openmpt->duration < ((double)INT64_MAX + 1) / AV_TIME_BASE)
+st->duration = llrint(openmpt->duration*AV_TIME_BASE);
 
 st->codecpar->codec_type  = AVMEDIA_TYPE_AUDIO;
 st->codecpar->codec_id= AV_NE(AV_CODEC_ID_PCM_F32BE, AV_CODEC_ID_PCM_F32LE);
@@ -171,6 +172,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);
+
 switch (openmpt->channels) {
 case 1:
 ret = openmpt_module_read_float_mono(openmpt->module, openmpt->sample_rate,
@@ -196,6 +199,9 @@ static int read_packet_openmpt(AVFormatContext *s, AVPacket *pkt)
 
 pkt->size = ret * (openmpt->channels * 4);
 
+if (pos >= 0 && pos < ((double)INT64_MAX + 1) / AV_TIME_BASE)
+pkt->pts = llrint(pos * AV_TIME_BASE);
+
 return 0;
 }
 
@@ -212,6 +218,8 @@ static int read_close_openmpt(AVFormatContext *s)
 static int read_seek_openmpt(AVFormatContext *s, int stream_idx, int64_t ts, int flags)
 {
 OpenMPTContext *openmpt = s->priv_data;
+if (ts < 0)
+ts = 0;
 openmpt_module_set_position_seconds(openmpt->module, (double)ts/AV_TIME_BASE);
 return 0;
 }
-- 
2.49.0

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".


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

2025-07-31 Thread kimapr via ffmpeg-devel
revised the patch, hopefully it's better now. i also fixed
another weirdness that i didn't fix in the original patch

On 2025/07/31 19:19, Michael Niedermayer wrote:

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

added some details!

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

no idea. openmpt docs don't mention the possibility, and
besides like returning a NaN there isn't really much the API
can do to indicate such a condition

> 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

seems unlikely! i added a check anyway though (should also catch the
unknown position if it is indicated by a NaN). rather than replacing
the value with AV_NOPTS_VALUE i just don't set it. Is that good?
From d6418b665cc80a1680afee8259a242a42c0ed2ad Mon Sep 17 00:00:00 2001
From: Kimapr 
Date: Mon, 28 Jul 2025 06:32:27 +0500
Subject: [PATCH] libopenmpt: fix seeking weirdness

- proper pts for packets. leaving it blank leaves it up for guessing,
  but the guess doesn't take seeking into account, causing weirdness.

- clamp to 0 when seeking to negative ts. libopenmpt docs are unclear on
  this but not doing this causes an immediate EOF when seeking backwards
  to the beginning in mpv.

- only set song duration and packet pts when they are non-negative and
  in int64 range. NaNs count as out of range. this isn't a fix for any
  specific issue but might be helpful still, and shouldn't break
  anything.
---
 libavformat/libopenmpt.c | 12 +++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/libavformat/libopenmpt.c b/libavformat/libopenmpt.c
index 3ca59f506f..ab61000d0a 100644
--- a/libavformat/libopenmpt.c
+++ b/libavformat/libopenmpt.c
@@ -147,7 +147,10 @@ static int read_header_openmpt(AVFormatContext *s)
 if (!st)
 return AVERROR(ENOMEM);
 avpriv_set_pts_info(st, 64, 1, AV_TIME_BASE);
-st->duration = llrint(openmpt->duration*AV_TIME_BASE);
+
+if (openmpt->duration * AV_TIME_BASE <= INT64_MAX &&
+openmpt->duration * AV_TIME_BASE >= 0)
+st->duration = llrint(openmpt->duration*AV_TIME_BASE);
 
 st->codecpar->codec_type  = AVMEDIA_TYPE_AUDIO;
 st->codecpar->codec_id= AV_NE(AV_CODEC_ID_PCM_F32BE, AV_CODEC_ID_PCM_F32LE);
@@ -170,6 +173,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) * AV_TIME_BASE;
+
 switch (openmpt->ch_layout.nb_channels) {
 case 1:
 ret = openmpt_module_read_float_mono(openmpt->module, openmpt->sample_rate,
@@ -195,6 +200,9 @@ static int read_packet_openmpt(AVFormatContext *s, AVPacket *pkt)
 
 pkt->size = ret * (openmpt->ch_layout.nb_channels * 4);
 
+if (pos >= 0 && pos <= INT64_MAX)
+pkt->pts = llrint(pos);
+
 return 0;
 }
 
@@ -211,6 +219,8 @@ static int read_close_openmpt(AVFormatContext *s)
 static int read_seek_openmpt(AVFormatContext *s, int stream_idx, int64_t ts, int flags)
 {
 OpenMPTContext *openmpt = s->priv_data;
+if (ts < 0)
+ts = 0;
 openmpt_module_set_position_seconds(openmpt->module, (double)ts/AV_TIME_BASE);
 return 0;
 }
-- 
2.49.0

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".


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

2025-07-27 Thread kimapr via ffmpeg-devel
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.From 451691febac466cee37d9b836228e30c53813d60 Mon Sep 17 00:00:00 2001
From: Kimapr 
Date: Mon, 28 Jul 2025 06:32:27 +0500
Subject: [PATCH] libopenmpt: fix seeking

---
 libavformat/libopenmpt.c | 3 +++
 1 file changed, 3 insertions(+)

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);
+
 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);
 
 return 0;
 }
-- 
2.49.0

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".


Re: [FFmpeg-devel] FFmpeg 6.1.3 and 7.0.3

2025-08-04 Thread kimapr via ffmpeg-devel
On 2025/08/04 19:40, Michael Niedermayer wrote:
>> Next will be 6.1.3 and 7.0.3
> i intend to make these ASAP, and next after that will be 5.1.7

Hi! it would be nice if my bug fix

   avformat/libopenmpt: fix seeking weirdness

was backported to 6.1.3 and 7.0.3 (dunno about 5.1.7 as i haven't checked
if libopenmpt even exists there). not sure how that would work from my side

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".