vo_gpu_next: use pl deinterlace shader for bwdif instead of sw filter - #17394
vo_gpu_next: use pl deinterlace shader for bwdif instead of sw filter#17394llyyr wants to merge 7 commits into
Conversation
|
Yes, for some reason bwdif_vulkan stopped working for me after around FFmpeg 7.1, just outputs a green screen instead. No idea what happened but libplacebo's deinterlacing works. |
0939147 to
6da6403
Compare
|
This should be implemented such that it's possible to output full frame rate (60i -> 60p), instead of every other frame. |
|
Any updates? |
|
No, and I'm quite busy irl so I won't have time for this for a few months still. If anyone wants to pick this up they're welcome |
|
Rebased on top of #18099 This is ready for testing/review now |
|
unless I'm misunderstanding it, documentation of pl_source_frame.first_field already says libplacebo is doing the doubling internally. Can you tell me what I need to do here or how I can check if it's working right? |
It does, but you have to drive rendering twice as fast. Two times per each frame to output full frame rate, else you are dropping every other field (output frame). Get this sample and test it. You should be able to see all consecutive numbers. You can pause and frame step, both forward and backward frame step should work correctly. https://github.com/bbgdzxng1/interlace-detection-test-patterns/raw/9d163e87b222e2711ab5fae805d80b2e655e04c1/bt601-525_480_interlaced_bff.ts EDIT: You can find more samples at https://github.com/bbgdzxng1/interlace-detection-test-patterns/ |
|
From those samples, even current vaapi/vulkan deint filters skip half the fields on gpu-next. On gpu, 60i becomes 60p correctly. I'll need to check why |
|
This outputs full frame rate now. Though I'm not sure if there's a simpler way to do this than adding a dummy video filter |
f0a5310 to
09c39d9
Compare
| pars->params.frame_mixer = opts->interpolation ? map_scaler(p, SCALER_TSCALE) : NULL; | ||
|
|
||
| pars->params.deinterlace_params = fopts->deinterlace != 0 ? &pars->deinterlace_params : NULL; | ||
| pars->deinterlace_params.algo = PL_DEINTERLACE_BWDIF; |
There was a problem hiding this comment.
This should be an option, not hardcoded.
There was a problem hiding this comment.
https://mpv.io/manual/master/#options-deinterlace
Deinterlace option says bwdif is inserted typically, and making this an option would be out of scope for this PR as it would need to change for hardware deinterlacers too
There was a problem hiding this comment.
The limitation of autofilter, should not be transposed to gpu-next.
There was a problem hiding this comment.
Added option to select it
25ad50c to
fbca8de
Compare
|
frame stepping backwards doesn't work. Also quality of image after deinterlacing is not good, you can see it especially in =bob mode, which should just show alternating fields. It looks like chroma is not aligned with luma correctly. Look at the bottom of the numbers, heavily de-saturated. This needs to be fixed before we can enable it by default. |
Can be reproduced in plplay too, would need to fix libplacebo implementation first. From what I can tell, only yadif similar to how the software implementation does. bob and bwdif have artifacts that aren't present with the software filter.
Will look into this after the libplacebo side is fixed |
To avoid having to manually compare this, I think we should add a test in libplacebo, that compares output to software reference, we can make this be enable only if building with ffmpeg. Naturally due to all pipeline being in float, it won't be bitexact, but we can compute PSNR. The result should be very close, give the same configured algorithm and sampling method. |
| .pts = mpi->pts, | ||
| .duration = can_interpolate ? frame->approx_duration : 0, | ||
| .duration = can_interpolate ? frame->approx_duration : | ||
| use_fields ? mpi->pkt_duration : 0, |
There was a problem hiding this comment.
In some cases we might not have pkt_duration, not sure if that's critical, but we could use frame->approx_duration which is our estimation.
There was a problem hiding this comment.
Isn't this the opposite? We should always have pkt_duration, but approx_duration is only computed in display synced modes
|
When deinterlacing is enabled you need to request one more frame (at least) in vo_set_queue_params, because we need to have future frame in pl_queue |
|
Fixed the backstep problem and changed it to request extra frame when deinterlacing. The alignment issues persist however and still need fixing |
Do you still see issues after applying those changes https://code.videolan.org/videolan/libplacebo/-/merge_requests/874 ? |
|
ed5a365 to
95a9785
Compare
I've fixed this by pushing the previous frame to queue. This PR along with the above libplacebo MR works pretty well now |
| // Optional decoded enhancement-layer frame | ||
| struct mp_image *enhancement_layer; | ||
| // Previous source frame retained for VO-side deinterlacing | ||
| struct mp_image *deint_prev; |
There was a problem hiding this comment.
I don't think this is good place to hold such information. vavpp and d3d11 may by itself request more past frames. And while libplacebo currently only needs 1 past frame, it is not extensible.
If anything we shouldn't hold any dummy frames, but instead demux a frame before again. Same as hr-seek works, where we already jump way before the seek target and iterate from keyframe.
Either way, mp_image is not a good vessel to carry over past frames.
There was a problem hiding this comment.
Getting the past frame isn't hard, refqueue already has it. The difficult part is plumbing it to the VO.
Does adding num_past_frames to vo_set_queue_params and past_frames to vo_frame make sense? Alternatively I was thinking of making vf_fieldrate always produce 1 frame before the target with MP_IMGFIELD_TEMPORAL_HISTORY or such, but this would need to be special cased to not affect all the vsync/display related things
There was a problem hiding this comment.
Does adding num_past_frames to vo_set_queue_params and past_frames to vo_frame make sense?
Yes, I think this make sense. It mirrors future frames nicely.
There was a problem hiding this comment.
I've implemented this
Used in next commits
This is used to emit frame per field, so the VO is asked to draw both fields.
This is used to signal when the VO can do deinterlacing itself, instead of using a filter
Allow setting the requested number of past frames via vo_set_queue_params, and make them available in vo_frame.


No description provided.