Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Seeking forward 5 seconds ends up seeking backwards #12047

Open
low-batt opened this issue Jul 29, 2023 · 16 comments
Open

Seeking forward 5 seconds ends up seeking backwards #12047

low-batt opened this issue Jul 29, 2023 · 16 comments

Comments

@low-batt
Copy link
Contributor

Important Information

Provide following Information:

  • mpv version: 0.36.0
  • macOS Version: 13.5
  • Source of the mpv binary or bundle: stolendata
  • If known which version of mpv introduced the problem: 0.34.1 works
  • Possible screenshot or video of visual glitches: N/A

A new version of IINA has recently been released with upgraded mpv and FFmpeg libraries. This problem was reported against the new IINA version in issue iina/iina#4502

Reproduction steps

Play the linked video in the sample files section below and try seeking forward 5 seconds.

To reproduce it like I did first make sure socat and jq are installed on your Mac. When invoking mpv enable the IPC interface by adding --input-ipc-server=/tmp/mpv-socket to the command line. Then while mpv is playing the video run the attached script in another terminal window. This is what the script looks like:

seek-test.sh:
#!/bin/bash

echo '{ "command": ["get_property", "time-pos"] }' | socat - /tmp/mpv-socket | jq '.data | strftime("%T")'
echo '{ "command": ["get_property", "playback-time"] }' | socat - /tmp/mpv-socket | jq '.data'

echo '{ "command": ["seek", "5"] }' | socat - /tmp/mpv-socket

echo '{ "command": ["get_property", "time-pos"] }' | socat - /tmp/mpv-socket | jq '.data | strftime("%T")'
echo '{ "command": ["get_property", "playback-time"] }' | socat - /tmp/mpv-socket | jq '.data'

sleep 1

echo '{ "command": ["get_property", "time-pos"] }' | socat - /tmp/mpv-socket | jq '.data | strftime("%T")'
echo '{ "command": ["get_property", "playback-time"] }' | socat - /tmp/mpv-socket | jq '.data'

The script uses the IPC interface to:

  • Get the playback position
  • Tell mpv to seek 5 seconds forward
  • Get the playback position again
  • Then it waits for a second
  • Get the playback position again

seek-test.sh.txt

Expected behavior

  • Playback seeks forward by some number of seconds (I say some number because this is a relative seek)
  • Seeking behaves the same way it did in mpv 0.34.1
  • mpv does not seek backwards

Actual behavior

Playback restarts at an earlier time.

Running the test script shows the time-pos property initially advancing by 5 seconds as expected and one second later time-pos reports the playback position is earlier than when the seek command was submitted:

low-batt@gag ~$ ./seek-test.sh 
"00:00:03"
3.1
{"data":null,"request_id":0,"error":"success"}
"00:00:08"
0.033333
"00:00:01"
1.033333
low-batt@gag ~$ 

This is how I invoked mpv:

Invoking mpv 0.36.0:
low-batt@gag ~$ /Applications/mpv.app/Contents/MacOS/mpv --log-file=mpv.log --no-config --input-ipc-server=/tmp/mpv-socket ~/Movies/FullHD30fps.mov
 (+) Video --vid=1 (*) (hevc 1920x1080 30.000fps)
 (+) Audio --aid=1 (*) (aac 2ch 44100Hz)
AO: [coreaudio] 44100Hz stereo 2ch floatp
VO: [libmpv] 1920x1080 yuv420p10
AV: 00:00:18 / 00:00:18 (99%) A-V:  0.000

Exiting... (End of file)
low-batt@gag ~$ 

These versions exhibited the problem:

  • master, my own build today
  • 0.36.0, stolendata
  • 0.35.1, brew installed

This is the output when running the stolendata 0.34.1 build:

low-batt@gag ~$ ./seek-test.sh 
"00:00:02"
2.666667
{"data":null,"request_id":0,"error":"success"}
"00:00:07"
7.666667
"00:00:09"
9.533333
low-batt@gag ~$ 

Log file

mpv.log

Sample files

I obtained the file that exhibits the problem from here

That file is one of the samples in Photography Blog's Apple iPhone 13 Review. Other samples from that review also exhibited the problem.

@Traneptora
Copy link
Member

Traneptora commented Aug 3, 2023

Do you have --hr-seek enabled? Without it, relative seeks are inexact.

@low-batt
Copy link
Contributor Author

low-batt commented Aug 3, 2023

This issue is about the change in behavior of relative seeks from mpv 0.34.1 where it is working fine. As this is about the default (relative) behavior of seeks changing, the hr-seek option is not being used.

With a relative seek is it expected that mpv would seek backwards when asked to seek 5 seconds forwards?

Shouldn't a relative forward seek restart playback at one of the keyframes after the current position?

Keyframes:
low-batt@gag ~$ ffprobe -loglevel error -select_streams v:0 -show_entries packet=pts_time,flags -of csv=print_section=0 ~/Movies/FullHD30fps.mov | awk -F',' '/K/ {print $1}'
0.000000
1.066667
2.133333
3.200000
4.266667
5.333333
6.400000
7.466667
8.533333
9.600000
10.666667
11.733333
12.800000
13.866667
14.933333
16.000000
17.066667
18.133333
low-batt@gag ~$ 

@Traneptora
Copy link
Member

Can you reproduce this without the IPC server, using a keybind in input.conf?

@low-batt
Copy link
Contributor Author

low-batt commented Aug 3, 2023

Yes. The reason we know about this is that the default IINA key bindings tie the left and right arrow keys to seek -5/5. So this was initially noticed due to the unexpected behavior when pressing the right arrow key in IINA.

@Traneptora Traneptora changed the title Seek forward by 5 seconds resumes playback at an earlier time Seeking forward 5 seconds ends up seeking backwards Aug 3, 2023
@low-batt
Copy link
Contributor Author

low-batt commented Aug 4, 2023

Thanks for fixing the title. Yours is better than what I entered.

If you need me to run tests or anything like that, don't hesitate to ask.

The background on this is that IINA was using old versions of libraries such as libmpv (0.34.1) and the ones from FFmpeg (4) and just recently upgraded. So we are now hearing about regressions from users.

Just FYI, I did not report one of these regressions to mpv because it was reproduced directly with FFmpeg and reported to that group. Hardware decoding for VP9 locks up both IINA and mpv on Intel Macs. We are working around that one for now by removing VP9 from hwdec-codecs on Intel Macs.

@Traneptora
Copy link
Member

I'm not particularly sure where to start, and I can't reproduce this. But I also am not using macOS, which makes it harder.

@Traneptora
Copy link
Member

Update, I just checked again, and I can reproduce this with the video you linked.

@Traneptora
Copy link
Member

Traneptora commented Aug 4, 2023

I can reproduce this with the following:

curl -O 'https://img.photographyblog.com/reviews/apple_iphone_13/sample_images/FullHD30fps.mov'
mpv --no-config --pause FullHD30fps.mov

and then pushing the right arrow to seek forward 5 seconds. The seek is almost immediately reverted. The issue is also reproducible with --vo=gpu-next.

Log file: https://0x0.st/H2nu.log

@vorou
Copy link

vorou commented Aug 14, 2023

might be related #10876

@low-batt
Copy link
Contributor Author

Yes, looks like the same problem.

The change from 0.34.1 that has introduced this behavior needs to be identified.

@llyyr
Copy link
Contributor

llyyr commented Sep 22, 2023

This is a ffmpeg regression, you can reproduce the same issue with ffplay on ffmpeg-5 but not with ffplay on ffmpeg-4.4.4. Most likely same with #10876

To be specific, this is a regression between the 5.1 and 5.0 releases. I'll try to bisect further

@low-batt
Copy link
Contributor Author

Reproduced for me as well using FFplay built from FFmpeg master just now.

@llyyr
Copy link
Contributor

llyyr commented Sep 23, 2023

I bisected it to FFmpeg/FFmpeg@ab77b87 I'll see if I can figure out a fix for this, but you should report it to ffmpeg instead

@llyyr
Copy link
Contributor

llyyr commented Sep 23, 2023

diff --git a/libavformat/mov.c b/libavformat/mov.c
index 1996e0028c..cb57c4c9ea 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -9108,7 +9108,7 @@ static int mov_seek_stream(AVFormatContext *s, AVStream *st, int64_t timestamp,
 {
     MOVStreamContext *sc = st->priv_data;
     FFStream *const sti = ffstream(st);
-    int sample, time_sample, ret;
+    int sample, time_sample, ret, offset, requested_sample;
     unsigned int i;
 
     // Here we consider timestamp to be PTS, hence try to offset it so that we
@@ -9129,7 +9129,17 @@ static int mov_seek_stream(AVFormatContext *s, AVStream *st, int64_t timestamp,
 
         if (!sample || can_seek_to_key_sample(st, sample, timestamp))
             break;
-        timestamp -= FFMAX(sc->min_sample_duration, 1);
+
+        offset = FFMAX(sc->min_sample_duration, 1);
+        requested_sample = av_index_search_timestamp(st, timestamp-offset, flags);
+
+        // If we can't seek to the next pts either and it's a different sample,
+        // give up trying to find a good pts to seek to, otherwise we'll end up
+        // seeking back to sample 0 on every seek.
+        if (!can_seek_to_key_sample(st, requested_sample, timestamp-offset) && sample != requested_sample)
+            break;
+
+        timestamp -= offset;
     }
 
     mov_current_sample_set(sc, sample);

I'm not convinced this is the right approach but you can apply this patch to lavf to fix this. I'll see if upstream agrees this is the right fix.

@low-batt
Copy link
Contributor Author

Thanks for investigating this problem!

I have entered FFmpeg Ticket 10585.

With the patch, if I let FFplay play to the end and then start pressing the arrow keys I can sometimes generate this error:

FullHD30fps.mov: error while seekingKB vq=    0KB sq=    0B f=0/0

Without the patch I was not able to generate that error. With both versions I can trigger this message:

[hevc @ 0x12cf2e6c0] Multiple Dolby Vision RPUs found in one AU. Skipping previous.

I've included this information in the FFmpeg ticket.

@llyyr
Copy link
Contributor

llyyr commented Sep 23, 2023

[hevc @ 0x12cf2e6c0] Multiple Dolby Vision RPUs found in one AU. Skipping previous.

This is unrelated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants