Closed Dudemanguy closed 2 weeks ago
Dudemanguy
commented
2 weeks ago Ah the approach in the last commit didn't work how I expected and has an edge case if you use lavfi-complex on the same file twice. Give me a bit.
Edit: OK it should be functional now.
github-actions[bot]
commented
2 weeks ago Download the artifacts for this pull request:
guidocella
commented
2 weeks ago mpv does not change behavior when demuxers detect an image, it is only used for minor things like not displaying the FPS. What changes the behavior is player/video.c setting vo_c->is_sparse = true when there's an EOF after the first frame, which is more reliable because it doesn't depend on a heuristic. With this patch misdetected images no longer play as video. One misdetection I know of is bmp/numbers.bmp in the FATE suite, after this patch only the first frame is played. So I think it would be safer to limit this check to jpeg_pipe streams.
Dudemanguy
commented
2 weeks ago One misdetection I know of is bmp/numbers.bmp in the FATE suite, after this patch only the first frame is played.
Isn't this PR better than the current behavior of "rapidly loop through all the frames and close"? Although I guess a problem is that this makes it impossible to see all the frames (e.g. with --pause and then frame stepping). I wonder if maybe mpv should just do an internal pause for an image although this would be a behavior change.
guidocella
commented
2 weeks ago How is it better? numbers.bmp is a video that mpv misdetects as an image.
Dudemanguy
commented
2 weeks ago I misunderstood.
Dudemanguy
commented
2 weeks ago I decided to drop the other commits and make this just the revert. Ideally this should be fixed in ffmpeg.
guidocella
commented
2 weeks ago What's wrong with getting only the first frame of jpg images? It's fine not to hack around the mpegts edge case but smartphone photos not working is really annoying and they are unlikely to be fixed in ffmpeg any time soon if they've been broken until now.
Dudemanguy
commented
2 weeks ago I restored that commit for only jpeg_pipe. I don't like the way the lavfi-complex thing is handled but not sure how else to do it.
guidocella
commented
2 weeks ago This still makes --lavfi-complex randomy fail.
magick rose: /tmp/rose.jpg
mpv --input-conf='memory://d cycle-values lavfi-complex "" "[vid1] [vid2] hstack [vo]"' /tmp/rose.jpg --external-file=/tmp/rose.jpgThis errors with
[ffmpeg] Parsed_hstack_0: Input 1 height 46 does not match input 0 height 64.
[ffmpeg] Parsed_hstack_0: Failed to configure output pad on Parsed_hstack_0
[lavfi] failed to configure the filter graphafter perssing d a random number of times.
Dudemanguy
commented
2 weeks ago I realized it's easier to just check the pos of the packet in the stream instead.
d0aeca59189c44b3999cc7add9d23642a73fe188 started passing the file name to libavformat when probing for jpeg files so some weird edgecases would work. The problem with that commit is that it causes image2 to be picked which is blacklisted in demux_lavf (for good reason) and then it falls back to demux_mf. This works, but it breaks fetching several properties (demux-w, demux-h, etc.) since demux_mf is not capable of providing this information. That leads to subpar behavior in scripts (e.g. my own but there could easily be other people) that rely on this information that was previously available for the vast majority of well-behaved files. So revert it since it's basically a sledgehammer.
To address the two fringe cases that tried to fix, a couple of different approaches are used. One is just forcing
jpeg_pipeon files with ajpgorjpegextension that get detected as mpegts. I don't know if we even want this commit, but it would address that specific issue (#13431) without affecting anything else.The other one is prohibits demux_lavf from sending more than one frame if the stream is detected as an image. I think this is pretty safe since mpv already changes its behavior a lot if it thinks the file is an image and a misdetection in that case would already be disastrous. However
demux_lavfis perfectly capable of reading more packets regardless so all I did was prohibit reading more than one frame from the stream if it is detected as an image. This prevents mpv from displaying metadata and other stuff as frames like in #13192.