-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
d3d11vpp: add GPU scale option for intel and nvidia GPUs #14155
base: master
Are you sure you want to change the base?
Conversation
Quate the comment from #11390
|
I have refacted the code to reuse d3d11vpp, also updatd the option name. There will be another change later to change SuperResolution to Scale to avoid potential copyright issue.
RTX video do support any resolution. Idealy, we should take the actual window size, and use them for the target height/width @kasper93 The GPU API are not quite the same for different venders, as you can see from my code for nvidia and intel, they may be similar, but not the same, it is hard to simplely provide API for user to set the GUID, and it just works. AMD has super res which called FSR, but it is not a GPU API, the usage is not the same as intel/nvidia, there are some glsl shades implemented the FSR for MPV. Also I have tried to use the intel auto HDR API, but failed, do we have examples for turning a SDR video to HDR video. |
I don't think they call their video hq scaler FSR in any documentation. Though indeed looking at it they did expose it through own interface, rather than rolling custom video processor in d3d11 video processing interface (to be confirmed if not available through it, not sure what browsers use). It could live in the same mpv filter (d3d11vpp), same as they integrated it in VLC. See: |
The AMF doc says it supports
FWIW I'm curious about whether it is possible to be implemented as a cross-platform filter with both d3d11 and vulkan backends. |
For AMD AMF stuff, it seems to be integrated in ffmpeg https://patchwork.ffmpeg.org/project/ffmpeg/list/?series=11632 so we can skip this part and have it for free once it get merged. |
FFMPEG are being difficult again (https://www.phoronix.com/news/AMD-AMF-FFmpeg-Better-2024) so not really sure if it would get merged soon. |
Not sure if this is helpful, but was staring at how Chrome's Auto HDR works and is the original commit (there might be bug fixes after this commit): chromium/chromium@b2fae4c Here is another example: Aleksoid1978/VideoRenderer#128 |
.scale = SUPER_RESOLUTION_OFF, | ||
.scale_target = SUPER_RESOLUTION_AUTO, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This config interface is bloated. It should be simple and customizable.
scale
should be floating point value of scaling that should be applied to source size. This can be extended to also include special options like target
, which would get VO display size.
Current "scale" should be renamed to scale_method
or similar. Although since currently none of the vendors offer selection, this can be made as a boolean value to enable high quality scalers.
Whole interface should work also without specifying those fancy GUIDs, d3d11vpp offers scaling as one of it's basic features, which should also be available for "normal" scaling.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Current "scale" should be renamed to scale_method or similar. Although since currently none of the vendors offer selection, this can be made as a boolean value to enable high quality scalers.
You mean none of the currently supported vendors? If this is expanded to AMD, they do support different methods with AMF_HQ_SCALER_ALGORITHM.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At current stage, there is no need for such option. scaling_algo
can be added in the future when it will be useful.
p->pool = mp_image_pool_new(f); | ||
mp_image_pool_set_allocator(p->pool, alloc_pool, f); | ||
mp_image_pool_set_lru(p->pool); | ||
|
||
mp_refqueue_add_in_format(p->queue, IMGFMT_420P, 0); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is big no-no in the filter itself. This is d3d11vpp processor and it accept only IMGFMT_D3D11
as an input. And hardware upload should be supported in different layer, possibly transparently, but not in this filer.
case SUPER_RESOLUTION_3X: | ||
window_w = 3 * in_fmt->w; | ||
window_h = 3 * in_fmt->h; | ||
break; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I already commented on the options, this is no go to maintain such list of arbitrary selected target resolutions.
video/filter/vf_d3d11vpp.c
Outdated
SetSuperResNvidia(vf); | ||
break; | ||
case SUPER_RESOLUTION_INTEL: | ||
SetSuperResIntel(vf); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please follow https://github.com/mpv-player/mpv/blob/master/DOCS/contribute.md for codding style and naming. Generally following the style of current file is ok.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated
// mp_image_copy_attributes overwrites the height and width | ||
// set it the size back if we are using scale | ||
mp_image_set_size(out, p->out_params.w, p->out_params.h); | ||
out->params.crop = bakup_crop; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typo, also crop value will not really be valid anymore after scaling. Crop value should be also rescaled with the size of the video.
ID3D11Texture2D *d3d_tex; | ||
int d3d_subindex = 0; | ||
|
||
if(in->imgfmt == IMGFMT_420P){ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wrong place to do any hardware upload.
video/filter/vf_d3d11vpp.c
Outdated
if (p->sr_state != 0){ | ||
return; | ||
} | ||
GUID GUID_INTEL_VPE_INTERFACE = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use DEFINE_GUID
macro, move it to the top of the file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated
video/filter/vf_d3d11vpp.c
Outdated
0x4cbc, | ||
{0xa4, 0xd6, 0x98, 0x31, 0xa2, 0x16, 0x3a, 0xc3} }; | ||
|
||
enum : UINT { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Move all enum/struct definitions to the top or at least outside function.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated
video/filter/vf_d3d11vpp.c
Outdated
@@ -72,6 +86,7 @@ struct priv { | |||
struct mp_image_pool *pool; | |||
|
|||
struct mp_refqueue *queue; | |||
int sr_state; // -1 for failed, 0 for off, 1 for on |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can be enum, tho avoid awkward comments explaining what each value mean.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated
Also needs fixes for gcc compilation and commits should be squashed into logical parts. |
is this helpful? |
@srk24 What is this NGX? Is it even supported anymore? |
NGX is off topic for the work in this PR. It's nvidia's umbrella for some AI based image/video processing scalers - none of which are usable via the d3d11 scaling mechanism covered by this PR. |
adjust order FIX: api usage adjust order update log refactor fix error logging logging fix error fix error update logging update update update update logging update update update Revert "update" This reverts commit 2642c41. update fixed 2K hardcode 2k test size test size test Revert "test" This reverts commit 596890b. Revert "test size" This reverts commit ef2b98a. Revert "test size" This reverts commit 38001eb. GNU GLOBAL LOGGING TEST logging fix logging Revert "fix logging" This reverts commit ec4f9c0. fix error revert logging remove logging because POC complete logging logging logging logging comment update update try fix update fix fix fix fix fix fix fix fix use 4K add option fix build more options fix build fix add defaults add vulkan for build add d3d11sr delete code remove processing rename Revert "rename" This reverts commit 82742d8. Revert "remove processing" This reverts commit c5acd59. Revert "delete code" This reverts commit 14250fb. Revert "add d3d11sr" This reverts commit 6bc9c1e. add d3d11sr disable deint fix blur fix size revert change fix warning remove logging add intel code rename field remove mode param remove unused variables Support SR selection add shorts revert d3d11vpp changes remove unused optmization handle SW decoding, dint not included stop processing if width or height is uneven cleanup cleanup Store SR state Revert "add vulkan for build" This reverts commit 0952b9f. revert unrelated code migrate back to d3d 11vpp
code format
updated the easy parts.
|
Hey any update on TODOs? |
usage:
vf set d3d11vpp=scale=nvidia:scale-target
PS:
The pull requests is has not been rebased to one commit yet, as I still need some changes, and will rebase to one commit once ready for merge.