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

mpv crashes with gpu-next and interpolation when speed is ≥150% #9438

Closed
v-fox opened this issue Nov 10, 2021 · 20 comments
Closed

mpv crashes with gpu-next and interpolation when speed is ≥150% #9438

v-fox opened this issue Nov 10, 2021 · 20 comments

Comments

@v-fox
Copy link

v-fox commented Nov 10, 2021

Important Information

Provide following Information:

  • 0.34.0+29~git20211110.2b9d8ae8b1
  • openSUSE Tumbleweed
  • Source of the mpv binary
  • After merge of gpu-next
  • KDE5 under X11
  • RX580 with amdgpu/Mesa

Reproduction steps

  • Using smplayer with mpv backend and gpu-next vo
  • Seeking back with keyboard
  • Getting black screen with audio
  • Either seeking forward and resetting to normal or waiting 2 second for crash

Can't find what may cause different behaviour when under smplayer.

Expected behavior

Not blackscreen and crash

Actual behavior

Blackscreen and crash

Log file

Log ends with:

[vo/gpu-next/vulkan/libplacebo] Backwards source PTS jump 2749.614014 -> 2749.597900, discarding frame...
mpv: ../src/utils/frame_queue.c:683: interpolate: Проверочное утверждение «mix->num_frames» не выполнено.

Sample files

Should work on any but right now I'm testing on a Twitch rip with youtube-dl.

@v-fox v-fox added the os:linux label Nov 10, 2021
@garoto
Copy link
Contributor

garoto commented Nov 10, 2021

Can you reproduce your issue when running a mpv binary on its own? If not, then you're in the wrong issue tracker.

@v-fox
Copy link
Author

v-fox commented Nov 11, 2021

Can you reproduce your issue when running a mpv binary on its own? If not, then you're in the wrong issue tracker.

I'm pretty sure smplayer dev is not writing mpv's VOs, @haasn is. If you have nothing of substance to add then you're in the wrong issue.

@Akemi
Copy link
Member

Akemi commented Nov 11, 2021

garoto has a valid point though. no one here is going to test with smplayer. there is no need to talk back to him in the way you did.

please try to reproduce your crash with mpv and --no-config like the issue template asks you to. if you can't reproduce you have to find out whatever setting in smplayer is causing it and what the corresponding setting is in mpv.

@v-fox
Copy link
Author

v-fox commented Nov 11, 2021

garoto has a valid point though. no one here is going to test with smplayer.

Too bad most users will.

there is no need to talk back to him in the way you did.

There is no need for "get out of here, infidel, with your GUI heresy !!1" knee-jerk reaction either. Watch your tone before you accuse others.

please try to reproduce your crash with mpv and --no-config like the issue template asks you to. if you can't reproduce you have to find out whatever setting in smplayer is causing it and what the corresponding setting is in mpv.

I've provided full vo-trace log for those with a clue about vo's to look for. And looking at offending line in source would likely make it more obvious, for those with knowledge and interest. But you're seem to be more interested in drama and some self-satisfaction.

In today's testing I figured that it's increased speed that's what triggers it. Pretty common thing to use, don't you think ? But I guess I just should keep silent because of some anti-GUI witch-hunt.

smplayer just has feature of saving per-file settings which restores speed on opening and triggers it instantly. It also has nebulous "enable postprocessing by default" setting in prominent place in 'video settings' which appears to be forcing vf add lavfi=[pp] which exacerbates the problem on higher speeds and gets injected in per-file settings when they are opened while global default is active. Not sure why 'pp' without any subcommands leads to massive dropouts on high speed without even reaching high CPU and GPU load but does the same with vo=gpu too, just without crashing. lavfi=[pp] is not required for crashing with vo=gpu-next though, just having interpolation enabled and speed increased.

@v-fox v-fox changed the title mpv crashes with gpu-next when under smplayer mpv crashes with gpu-next and interpolation when speed is ≥150% Nov 11, 2021
@haasn
Copy link
Member

haasn commented Nov 11, 2021

Having a hard time reproducing on my end.

Can you run it through gdb and, after the crash, info locals, print *params, print *p, print *p->queue.elem@p->queue.num?

@LaserEyess
Copy link
Contributor

Too bad most users will.

There is no need for "get out of here, infidel, with your GUI heresy !!1" knee-jerk reaction either. Watch your tone before you accuse others.

This is mpv's issue tracker, not smplayer's issue tracker. The template, which you should have read, says:

Try to reproduce your issue with --no-config first. If it isn't reproducible
with --no-config try to first find out which option or script causes your issue.

Describe the reproduction steps as precise as possible. It's very likely that
the bug you experience wasn't reproduced by the developer because the workflow
differs from your own.

You must reproduce your issue with mpv. No one is accusing you of anything except not using mpv to reproduce this issue. There is no anti-GUI conspiracy or anything, you simply need use mpv to reproduce the issue.

FYI the issue template also says:

Do not use garbage like "cloud storage", especially not Google Drive.

@Akemi
Copy link
Member

Akemi commented Nov 12, 2021

Too bad most users will.

you can believe whatever you want, but you can't expect our help if we kindly ask you to do specific things. so please play by our rules.

There is no need for "get out of here, infidel, with your GUI heresy !!1" knee-jerk reaction either. Watch your tone before you accuse others.

i don't know what you mean, my tone was perfectly fine. if i meant for you to get out of here i would have said so directly.

@v-fox
Copy link
Author

v-fox commented Nov 14, 2021

@haasn

Having a hard time reproducing on my end.

To do it required steps are:

  • working interpolation
  • speed over 150%, bigger = more chances of the blackout+crash
  • pressing <left> on keyboard for seeking back

These multiply the chance of crash:

  • --hr-seek=yes (maybe also hr-seek-framedrop=no)
  • bigger resolution: 1080p is more likely to crash each time than 720p
  • fullscreen rather than windowed
  • using shaders
  • using vapoursynth script
  • holding <left> instead of just pressing quickly
  • anything to induce [vo/gpu-next/vulkan/libplacebo] Backwards source PTS jump 1360.817017 -> 1360.784058, discarding frame... message

Can you run it through gdb and, after the crash, info locals, print *params, print *p, print *p->queue.elem@p->queue.num?

Basically it end up with:

Thread 17 "mpv/vo" received signal SIGABRT, Aborted.
[Switching to Thread 0x7fffb3fff640 (LWP 25041)]
__pthread_kill_implementation (threadid=<optimized out>, signo=signo@entry=6, no_tid=no_tid@entry=0) at pthread_kill.c:44
…
(gdb) info locals
tid = <optimized out>
ret = 0
pd = <optimized out>
old_mask = {__val = {140736099047724, 140736099047424, 140736099047724, 0, 0, 0, 0, 0, 553864505496, 1737569797626169856, 0, 140737302074560, 140736213278352, 140737309648264, 140737302074560, 140736213278432}}
ret = <optimized out>
(gdb) print *params
No symbol "params" in current context.
(gdb) print *p
No symbol "p" in current context.
(gdb) print *p->queue.elem@p->queue.num
No symbol "p" in current context.

But after installing debugsymbols for mpv and libplacebo175 print *params started giving weird A syntax error in expression, near `'. message. bt says:

#0  __pthread_kill_implementation (threadid=<optimized out>, signo=signo@entry=6, no_tid=no_tid@entry=0) at pthread_kill.c:44
#1  0x00007ffff4cef8e3 in __pthread_kill_internal (signo=6, threadid=<optimized out>) at pthread_kill.c:78
#2  0x00007ffff4ca26f6 in __GI_raise (sig=sig@entry=6) at ../sysdeps/posix/raise.c:26
#3  0x00007ffff4c8c7b3 in __GI_abort () at abort.c:79
#4  0x00007ffff4c8c6db in __assert_fail_base (fmt=<optimized out>, assertion=<optimized out>, file=<optimized out>, line=<optimized out>, function=<optimized out>) at assert.c:92
#5  0x00007ffff4c9b386 in __GI___assert_fail
    (assertion=assertion@entry=0x7ffff5592e48 "mix->num_frames", file=file@entry=0x7ffff55924b8 "../src/utils/frame_queue.c", line=line@entry=683, function=function@entry=0x7ffff5593b08 <__PRETTY_FUNCTION__.3.lto_priv.9> "interpolate") at assert.c:101
#6  0x00007ffff556068a in interpolate (params=0x7fffb3ffe130, mix=0x7fffb3ffe100, p=0x7fffac20c030) at ../src/utils/frame_queue.c:683
#7  pl_queue_update (p=0x7fffac20c030, out_mix=out_mix@entry=0x7fffb3ffe100, params=params@entry=0x7fffb3ffe130) at ../src/utils/frame_queue.c:785
#8  0x000055555569592a in draw_frame (vo=0x55555595bac0, frame=0x7fffad3301f0) at ../video/out/vo_gpu_next.c:698
#9  0x000055555569a1d4 in do_redraw (vo=0x55555595bac0) at ../video/out/vo.c:1051
#10 vo_thread (ptr=0x55555595bac0) at ../video/out/vo.c:1129
#11 0x00007ffff4cedb37 in start_thread (arg=<optimized out>) at pthread_create.c:435
#12 0x00007ffff4d72640 in clone3 () at ../sysdeps/unix/sysv/linux/x86_64/clone3.S:81

By the way, now using FSRCNNX_x2_8-0-4-1.glsl shader, which works fine with gpu & opengl, hangs mpv without crash. --msg-level=vo=trace ends with:

[vo/gpu-next/vulkan/libplacebo] shaderc compile status 'success' (0 errors, 0 warnings)
[vo/gpu-next/vulkan/libplacebo] Spent 193.001 ms translating SPIR-V (slow!)
[vo/gpu-next/vulkan/libplacebo] vk->CreatePipelineCache(vk->dev, &pcinfo, PL_VK_ALLOC, &pass_vk->cache)
[vo/gpu-next/vulkan/libplacebo] vk->CreateShaderModule(vk->dev, &sinfo, PL_VK_ALLOC, &pass_vk->shader)
[vo/gpu-next/vulkan/libplacebo] Spent 0.102 ms compiling shader
[vo/gpu-next/vulkan/libplacebo] vk_recreate_pipelines(vk, pass, has_spec, NULL, pipe)

But that might be driver-related. I don't know.

@LaserEyess

This is mpv's issue tracker, not smplayer's issue tracker.
You must reproduce your issue with mpv. No one is accusing you of anything except not using mpv to reproduce this issue. There is no anti-GUI conspiracy or anything, you simply need use mpv to reproduce the issue.

Last time I checked, smplayer were just launching mpv with additional options and I'm yet to find an unreproducible issue. Do you know many ?
I said in the very post which you quoting that I already reproduced it and how to do it. Have you tried ?

FYI the issue template also says:
Do not use garbage like "cloud storage", especially not Google Drive.

Does it also say what is not a "garbage" public storage that has no miniscule filesize limit and allows to delete files later in case that they reveal some private information ? Were you intending to download and read that log either way ?

@Akemi

you can believe whatever you want, but you can't expect our help if we kindly ask you to do specific things. so please play by our rules.

That is anything but 'kindly'. Also, you wrote this long after I confirmed the issue on raw mpv. Any comments on the actual matter ?

i don't know what you mean, my tone was perfectly fine. if i meant for you to get out of here i would have said so directly.

If not, then you're in the wrong issue tracker and there is no need to talk back to him in the way you did is not "fine", especially when 3 of you came only to posture without even trying to debug anything.

@haasn
Copy link
Member

haasn commented Nov 15, 2021

Basically it end up with:s

You have to select the right stack frame first (f 6 in your backtrace).

@v-fox
Copy link
Author

v-fox commented Nov 15, 2021

You have to select the right stack frame first (f 6 in your backtrace).

Oh-yeah, there is a bunch of stuff now:

(gdb) bt
#0  0x00007ffff4ced87c in __pthread_kill_implementation () at /lib64/libc.so.6
#1  0x00007ffff4ca06f6 in raise () at /lib64/libc.so.6
#2  0x00007ffff4c8a7b3 in abort () at /lib64/libc.so.6
#3  0x00007ffff4c8a6db in _nl_load_domain.cold () at /lib64/libc.so.6
#4  0x00007ffff4c99386 in  () at /lib64/libc.so.6
#5  0x00007ffff555fcda in interpolate (params=0x7fffb7ffe130, mix=0x7fffb7ffe100, p=0x7fffac006670) at ../src/utils/frame_queue.c:683
#6  pl_queue_update(pl_queue, pl_frame_mix*, pl_queue_params const*) (p=0x7fffac006670, out_mix=out_mix@entry=0x7fffb7ffe100, params=params@entry=0x7fffb7ffe130) at ../src/utils/frame_queue.c:785
#7  0x000055555569592a in draw_frame (vo=0x55555595cc80, frame=0x7fffacaca070) at ../video/out/vo_gpu_next.c:698
#8  0x0000555555699b11 in render_frame (vo=0x55555595cc80) at ../video/out/vo.c:961
#9  vo_thread (ptr=0x55555595cc80) at ../video/out/vo.c:1099
#10 0x00007ffff4cebb37 in start_thread () at /lib64/libc.so.6
#11 0x00007ffff4d70640 in clone3 () at /lib64/libc.so.6
(gdb) f 6
#6  pl_queue_update (p=0x7fffac006670, out_mix=out_mix@entry=0x7fffb7ffe100, params=params@entry=0x7fffb7ffe130) at ../src/utils/frame_queue.c:785
785     ../src/utils/frame_queue.c: Нет такого файла или каталога.
(gdb) info locals
delta = <optimized out>
ret = <optimized out>
max_vsync = <optimized out>
min_vsync = <optimized out>
(gdb) print *params
$1 = {pts = 1326.28406, radius = 1.5, vsync_duration = 0.0136740003, frame_duration = 0.00706829317, interpolation_threshold = 0.100000001, timeout = 0, get_frame = 0x0, priv = 0x0}
(gdb) print *p
$2 = {gpu = 0x7fffac503880, log = 0x7fffac211c70, lock_strong = {__data = {__lock = 1, __count = 0, __owner = 8441, __nusers = 1, __kind = 2, __spins = 0, __elision = 0, __list = {__prev = 0x0, __next = 0x0}}, 
    __size = "\001\000\000\000\000\000\000\000\371 \000\000\001\000\000\000\002", '\000' <repeats 22 times>, __align = 1}, lock_weak = {__data = {__lock = 1, __count = 0, __owner = 8441, __nusers = 1, 
      __kind = 2, __spins = 0, __elision = 0, __list = {__prev = 0x0, __next = 0x0}}, __size = "\001\000\000\000\000\000\000\000\371 \000\000\001\000\000\000\002", '\000' <repeats 22 times>, __align = 1}, 
  wakeup = {__data = {{__wseq = 0, __wseq32 = {__low = 0, __high = 0}}, {__g1_start = 0, __g1_start32 = {__low = 0, __high = 0}}, __g_refs = {0, 0}, __g_size = {0, 0}, __g1_orig_size = 0, __wrefs = 2, 
      __g_signals = {0, 0}}, __size = '\000' <repeats 36 times>, "\002\000\000\000\000\000\000\000\000\000\000", __align = 0}, queue = {elem = 0x7fffacb47e40, num = 2}, signature = 2, threshold_frames = 0, 
  want_frame = false, eof = false, vps = {samples = {0 <repeats 32 times>}, estimate = 0.0136740003, sum = 0, idx = 0, num = 0, total = 0}, fps = {samples = {0 <repeats 32 times>}, estimate = 0.00706829317, 
    sum = 0, idx = 1, num = 1, total = 1}, reported_vps = 0, reported_fps = 0, prev_pts = 1326.28406, tmp_sig = {elem = 0x7fffac4dd320, num = 0}, tmp_ts = {elem = 0x7fffac51c250, num = 0}, tmp_frame = {
    elem = 0x7fffac52ed80, num = 0}, cache = {elem = 0x7fffacaca200, num = 14}}
(gdb) print *p->queue.elem@p->queue.num
$3 = {{cache = {tex = {0x7fffac511090, 0x7fffac510db0, 0x7fffac510ad0, 0x0}}, src = {pts = 1326.41699, frame_data = 0x7fffacc1e6f0, map = 0x5555556943f0 <map_frame>, unmap = 0x555555690bf0 <unmap_frame>, 
      discard = 0x55555568ff20 <discard_frame>}, frame = {num_planes = 0, planes = {{texture = 0x0, address_mode = PL_TEX_ADDRESS_CLAMP, components = 0, component_mapping = {0, 0, 0, 0}, shift_x = 0, 
          shift_y = 0}, {texture = 0x0, address_mode = PL_TEX_ADDRESS_CLAMP, components = 0, component_mapping = {0, 0, 0, 0}, shift_x = 0, shift_y = 0}, {texture = 0x0, address_mode = PL_TEX_ADDRESS_CLAMP, 
          components = 0, component_mapping = {0, 0, 0, 0}, shift_x = 0, shift_y = 0}, {texture = 0x0, address_mode = PL_TEX_ADDRESS_CLAMP, components = 0, component_mapping = {0, 0, 0, 0}, shift_x = 0, 
          shift_y = 0}}, repr = {sys = PL_COLOR_SYSTEM_UNKNOWN, levels = PL_COLOR_LEVELS_UNKNOWN, alpha = PL_ALPHA_UNKNOWN, bits = {sample_depth = 0, color_depth = 0, bit_shift = 0}}, color = {
        primaries = PL_COLOR_PRIM_UNKNOWN, transfer = PL_COLOR_TRC_UNKNOWN, light = PL_COLOR_LIGHT_UNKNOWN, sig_peak = 0, sig_avg = 0, sig_floor = 0, sig_scale = 0}, profile = {data = 0x0, len = 0, 
        signature = 0}, lut = 0x0, lut_type = PL_LUT_UNKNOWN, crop = {x0 = 0, y0 = 0, x1 = 0, y1 = 0}, rotation = 0, overlays = 0x0, num_overlays = 0, film_grain = {type = PL_FILM_GRAIN_NONE, seed = 0, 
        params = {av1 = {num_points_y = 0, points_y = {"\000" <repeats 14 times>}, chroma_scaling_from_luma = false, num_points_uv = {0, 0}, points_uv = {{"\000", "\000", "\000", "\000", "\000", "\000", "\000", 
                "\000", "\000", "\000"}, {"\000", "\000", "\000", "\000", "\000", "\000", "\000", "\000", "\000", "\000"}}, scaling_shift = 0, ar_coeff_lag = 0, ar_coeffs_y = '\000' <repeats 23 times>, 
            ar_coeffs_uv = {'\000' <repeats 24 times>, '\000' <repeats 24 times>}, ar_coeff_shift = 0, grain_scale_shift = 0, uv_mult = "\000", uv_mult_luma = "\000", uv_offset = {0, 0}, overlap = false}, 
          h274 = {model_id = 0, blending_mode_id = 0, log2_scale_factor = 0, component_model_present = {false, false, false}, num_intensity_intervals = {0, 0, 0}, num_model_values = "\000\000", 
            intensity_interval_lower_bound = {0x0, 0x0, 0x0}, intensity_interval_upper_bound = {0x0, 0x0, 0x0}, comp_model_value = {0x0, 0x0, 0x0}}}}, user_data = 0x0}, signature = 0, mapped = false, 
    ok = false}, {cache = {tex = {0x7fffac52fbb0, 0x7fffac530260, 0x7fffac4cce60, 0x0}}, src = {pts = 1326.41699, frame_data = 0x7fffacc23930, map = 0x5555556943f0 <map_frame>, unmap = 
    0x555555690bf0 <unmap_frame>, discard = 0x55555568ff20 <discard_frame>}, frame = {num_planes = 0, planes = {{texture = 0x0, address_mode = PL_TEX_ADDRESS_CLAMP, components = 0, component_mapping = {0, 0, 0, 
            0}, shift_x = 0, shift_y = 0}, {texture = 0x0, address_mode = PL_TEX_ADDRESS_CLAMP, components = 0, component_mapping = {0, 0, 0, 0}, shift_x = 0, shift_y = 0}, {texture = 0x0, 
          address_mode = PL_TEX_ADDRESS_CLAMP, components = 0, component_mapping = {0, 0, 0, 0}, shift_x = 0, shift_y = 0}, {texture = 0x0, address_mode = PL_TEX_ADDRESS_CLAMP, components = 0, 
          component_mapping = {0, 0, 0, 0}, shift_x = 0, shift_y = 0}}, repr = {sys = PL_COLOR_SYSTEM_UNKNOWN, levels = PL_COLOR_LEVELS_UNKNOWN, alpha = PL_ALPHA_UNKNOWN, bits = {sample_depth = 0, 
          color_depth = 0, bit_shift = 0}}, color = {primaries = PL_COLOR_PRIM_UNKNOWN, transfer = PL_COLOR_TRC_UNKNOWN, light = PL_COLOR_LIGHT_UNKNOWN, sig_peak = 0, sig_avg = 0, sig_floor = 0, sig_scale = 0}, 
      profile = {data = 0x0, len = 0, signature = 0}, lut = 0x0, lut_type = PL_LUT_UNKNOWN, crop = {x0 = 0, y0 = 0, x1 = 0, y1 = 0}, rotation = 0, overlays = 0x0, num_overlays = 0, film_grain = {
        type = PL_FILM_GRAIN_NONE, seed = 0, params = {av1 = {num_points_y = 0, points_y = {"\000" <repeats 14 times>}, chroma_scaling_from_luma = false, num_points_uv = {0, 0}, points_uv = {{"\000", "\000", 
                "\000", "\000", "\000", "\000", "\000", "\000", "\000", "\000"}, {"\000", "\000", "\000", "\000", "\000", "\000", "\000", "\000", "\000", "\000"}}, scaling_shift = 0, ar_coeff_lag = 0, 
            ar_coeffs_y = '\000' <repeats 23 times>, ar_coeffs_uv = {'\000' <repeats 24 times>, '\000' <repeats 24 times>}, ar_coeff_shift = 0, grain_scale_shift = 0, uv_mult = "\000", uv_mult_luma = "\000", 
            uv_offset = {0, 0}, overlap = false}, h274 = {model_id = 0, blending_mode_id = 0, log2_scale_factor = 0, component_model_present = {false, false, false}, num_intensity_intervals = {0, 0, 0}, 
            num_model_values = "\000\000", intensity_interval_lower_bound = {0x0, 0x0, 0x0}, intensity_interval_upper_bound = {0x0, 0x0, 0x0}, comp_model_value = {0x0, 0x0, 0x0}}}}, user_data = 0x0}, 
    signature = 1, mapped = false, ok = false}}

@jeeb
Copy link
Member

jeeb commented Nov 15, 2021

Not sure how it exactly looks in the frame of interpolate itself, but how it seems to me is that:

  1. max_pts = pts + radius * estimate => 1326.294662439755
  2. While I don't think it gets printed, and we have only the following frame (?) shown (pts = 1326.41699), I would guess that p->queue.elem[p->queue.num - 1].src.pts being > max_pts (since even with 60Hz you get 1326.28406 + 2*(1/60) -> 1326.3173933333333, which goes over). So the while does not get run.
  3. mix is not nullptr, so we do not take the PL_QUEUE_OK early exit.
  4. p->tmp_frame.num = 0 initialization takes place.
  5. loop starts, we have two entries in queue.
  6. looking at the previous state if I have understood it correctly, entry->src.pts > max_pts is most likely true, loop breaks.
  7. .num_frames = p->tmp_frame.num,, (tmp_frame.num is still zero)
  8. pl_assert(mix->num_frames); kabooms, as nothing got added to it since everything is in the future as far as it is concerned, while the loop cared only about pts <= max_pts to add into the mix , not a bit further.

If we go by the comment of

representing the current queue state, starting at the last frame before min_pts

and add into the interpretation that we probably want to go up to max_pts, then in this case we might want to early exit out if we know that there's nothing in the queue <= max_pts.

Something a la (wholly untested)?

diff --git a/src/utils/frame_queue.c b/src/utils/frame_queue.c
index fc4ffd5..0849822 100644
--- a/src/utils/frame_queue.c
+++ b/src/utils/frame_queue.c
@@ -638,6 +638,16 @@ static enum pl_queue_status interpolate(pl_queue p, struct pl_frame_mix *mix,
 
     // Keep adding new frames until we've covered the range we care about
     pl_assert(p->queue.num);
+
+    struct entry *first_entry = &p->queue.elem[0];
+    if (first_entry->src.pts > max_pts) {
+        PL_WARN(p,
+                "Frame queue contains nothing in the requested range of "
+                "pts %.4f <= max_pts %.4f (first entry pts %.4f)!",
+                params->pts, max_pts, first_entry->src.pts);
+        return PL_QUEUE_OK;
+    }
+
     while (p->queue.elem[p->queue.num - 1].src.pts < max_pts) {
         switch ((ret = get_frame(p, params))) {
         case PL_QUEUE_ERR:

Might be worth a try at least (not sure if you want PL_QUEUE_ERR or PL_QUEUE_OK in case of such a mismatch, or what to do with mix if anything). With vps->estimate = 0.0136740003 being larger than min_vsync = 1.0 / 200 and smaller than max_vsync = 1.0 / 10, it does not bail out of attempting to interpolate.

Basically, how I read is that we have had a discontinuity (which the logs show), and haven't reset the state accordingly (or made the time line continuous in some way so that libplacebo can think that there's nothing wrong) so that we can proceed to fill up or get back to the time range of the frame queue.

@v-fox

If not, then you're in the wrong issue tracker and there is no need to talk back to him in the way you did is not "fine", especially when 3 of you came only to posture without even trying to debug anything.

I'd prefer if you separate these two.

Do note how the person who specifically was provocative has stayed silent (I do agree that he was provocative, and if he continued that way that would not be good). What @Akemi did is not interpret it as provocative as you took it, and noted that your response with what could be perceived as continued escalation was not good. And that - IMHO - was still done in a civil fashion (there is no need to talk back to him in the way you did. is not derogatory in my view).

How it looks to this side, you then felt the need to defend yourself against the original provocation further, and thus ended up misfiring towards @Akemi . I would prefer it if you could refrain from this in the future. Thank you.

If someone is unnecessarily provocative, that should not be fed, and possibly reported if it continues.

As for the logs, there are multiple pastebin services but this is a rather long one, yes. I would have probably attempted to limit it to the last seek or so initially (which technically is ~496k - which seems to fit https://gist.github.com (this one you can utilize with Github credentials which you should already have as you post here), https://pastebin.com or https://paste.centos.org/). For larger things I think you can add attachments into your Github posts (which I think you can remove?), as well as utilizing various file uploaders (I don't utilize them myself so unfortunately I cannot give recommendations for such).

Anyways, I will not go further regarding this. Some issues are GUI specific, but many are not. What the goal is, is to make reproduction of issues simpler and to minimize the amount of variables at play, so that as many people can check it out with as little hardship as possible.

@jeeb
Copy link
Member

jeeb commented Nov 15, 2021

Anyways, I guess the real problem is how this doesn't hit the negative delta check in pl_queue_update, since that is supposed to handle this case, I think?

edit: D'oh, because params->pts and p->prev_pts are both the same value, 1326.28406. So delta is zero and thus the first entry does not get checked.

@jeeb
Copy link
Member

jeeb commented Nov 15, 2021

So maybe something like this?

diff --git a/src/utils/frame_queue.c b/src/utils/frame_queue.c
index fc4ffd5..abd133b 100644
--- a/src/utils/frame_queue.c
+++ b/src/utils/frame_queue.c
@@ -734,22 +734,19 @@ enum pl_queue_status pl_queue_update(pl_queue p, struct pl_frame_mix *out_mix,
     default_estimate(&p->vps, params->vsync_duration);
 
     float delta = params->pts - p->prev_pts;
-    if (delta < 0.0) {
-
+    if (delta <= 0.0 &&
+        (p->queue.num && p->queue.elem[0].src.pts > params->pts)) {
         // This is a backwards PTS jump. This is something we can handle
         // semi-gracefully, but only if we haven't culled past the current
         // frame yet.
-        if (p->queue.num && p->queue.elem[0].src.pts > params->pts) {
-            PL_ERR(p, "Requested PTS %f is lower than the oldest frame "
-                   "PTS %f. This is not supported, PTS must be monotonically "
-                   "increasing! Please use `pl_queue_reset` to reset the frame "
-                   "queue on discontinuous PTS jumps.",
-                   params->pts, p->queue.elem[0].src.pts);
-            pl_mutex_unlock(&p->lock_weak);
-            pl_mutex_unlock(&p->lock_strong);
-            return PL_QUEUE_ERR;
-        }
-
+        PL_ERR(p, "Requested PTS %f is lower than the oldest frame "
+               "PTS %f. This is not supported, PTS must be monotonically "
+               "increasing! Please use `pl_queue_reset` to reset the frame "
+               "queue on discontinuous PTS jumps.",
+               params->pts, p->queue.elem[0].src.pts);
+        pl_mutex_unlock(&p->lock_weak);
+        pl_mutex_unlock(&p->lock_strong);
+        return PL_QUEUE_ERR;
     } else if (delta > 1.0) {
 
         // A jump of more than a second is probably the result of a

but at this point if this is considered a libplacebo bug by @haasn this can be moved onto the libplacebo tracker?

Of course if such an assert is OK due to mpv's (mis)usage of libplacebo API with regards to missing pl_queue_reset, then that's still an mpv issue.

@v-fox
Copy link
Author

v-fox commented Nov 15, 2021

How it looks to this side, you then felt the need to defend yourself against the original provocation further, and thus ended up misfiring towards @Akemi . I would prefer it if you could refrain from this in the future. Thank you.

And I would prefer for that to apply to everyone and not selectively with exclusion of "authorized" few that are immune to 'talking back' but not to dishing it out.

If someone is unnecessarily provocative, that should not be fed, and possibly reported if it continues.

'Report' to whom and for what ? Same people that encourage it ? For gagging which leads to FUD in discussions ? Doesn't seem like a good idea.

As for the logs, there are multiple pastebin services but this is a rather long one, yes…

Making gists for debug logs seems like an overkill and is likely subject to same size restrictions. pastebin.com and similar have censorship filters that can trigger on random 'suspicious' symbol/word combination and forbid posting.

Anyways, I will not go further regarding this. Some issues are GUI specific, but many are not. What the goal is, is to make reproduction of issues simpler and to minimize the amount of variables at play, so that as many people can check it out with as little hardship as possible.

But that beside the point. Which is the fact that I gave enough data even then to reproduce it. And, while having exact settings for it from the get-go is nice and all, let's not pretend that launching mpv binary with some custom settings makes its crash impossible when launched standalone. Everyone here is well aware that it does not invalidate it in any way, so there is no reason in good faith for such gang-up highfalutin pedantry, if the goal is really to achieve software improvement and not abuse notoriety status (as protection from 'talking back' with 'or else…') with fast and easy posturing or something which plagues interest communities now when they turned into social media bubbles.

As for issue itself, its details are beyond me but the 'FSRCNNX_x2_8-0-4-1.glsl shader' part was indeed unrelated and caused by a bad patch of mine in Mesa.

@haasn
Copy link
Member

haasn commented Nov 19, 2021

@v-fox can you test if the linked commit does, indeed, fix this?

@v-fox
Copy link
Author

v-fox commented Nov 20, 2021

@haasn

@v-fox can you test if the linked commit does, indeed, fix this?

With libplacebo179-4.157.0+159~git20211119.4d72486 and mpv-0.34.0+67~git20211119.59898331dd it dies for me similarly but with this message:

mpv: ../src/renderer.c:2897: pl_render_image_mix: Проверочное утверждение «fidx > 0» не выполнено.

@v-fox
Copy link
Author

v-fox commented Dec 2, 2021

@haasn
The original mpv: ../src/utils/frame_queue.c:686: interpolate: Проверочное утверждение «mix->num_frames» не выполнено. started happening again with libplacebo181-4.157.0+177~git20211130.f25699c and mpv-0.34.0+92~git20211128.ac39af461d
The constant flashing black frames on backseeks also kind of distracting.

@v-fox
Copy link
Author

v-fox commented Dec 4, 2021

@haasn And it also does mpv: ../src/renderer.c:2933: pl_render_image_mix: Проверочное утверждение «fidx > 0» не выполнено. in exactly the same way.

@jeeb @Akemi
Since it was never fixed and workaround was quietly reversed after being closed without confirmation, how about opening it back up ? The button is blocked for me since I'm not in the club.

@Akemi
Copy link
Member

Akemi commented Dec 4, 2021

since it seems a libplacebo issue doesn't it make more sense to open an issue there? otherwise it's up to @haasn how he wants to proceed.

@v-fox
Copy link
Author

v-fox commented Dec 4, 2021

since it seems a libplacebo issue doesn't it make more sense to open an issue there? otherwise it's up to @haasn how he wants to proceed.

Maybe. Unless mpv is supposed to filter out such condition on its own.
And also avoid initial black frames somehow (by caching current frame until new one is rendered, probably) too. Now on back-seeking there is either a safe-but-ugly black frame or a crash.

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

6 participants