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

vo_gpu_next: use pl_dispatch_info_move to avoid useless data copy #11875

Merged
merged 2 commits into from
Jul 2, 2023

Conversation

kasper93
Copy link
Contributor

@kasper93 kasper93 commented Jul 1, 2023

Instead copy the data on-demand when VOCTRL_PERFORMANCE_DATA is requested.

This require minimal libplacebo API version bump. Since new libplacebo release is close and the requirement in mpv is planned to be bumped too let's wait for it instead ifdefing everything. In the meantime we can review, so it is ready when it gets unblocked.

@haasn
Copy link
Member

haasn commented Jul 1, 2023

Question: Why does this struct need to be locked again, at all? Doesn't the VOCTRL run synchronously on VO thread, so how can it possibly race with the updates coming from the callbacks from pl_render_* (also on VO thread)?

Food for thought: If the above is true and the VOCTRL can run concurrently, we should hold the lock during the entire pl_render_() call, not just the callbacks! Otherwise VOCTRL may see inconsistent state...

@kasper93
Copy link
Contributor Author

kasper93 commented Jul 2, 2023

Question: Why does this struct need to be locked again, at all? Doesn't the VOCTRL run synchronously on VO thread, so how can it possibly race with the updates coming from the callbacks from pl_render_* (also on VO thread)?

I just double checked VOCTRL is correctly run on VO thread, so it is fine. Previously the issue was that it were passing a pointer, that were accessed form VO and LUA thread, but now since we are doing full copy, the data is decoupled so in fact this lock were never needed.

video/out/vo_gpu_next.c Outdated Show resolved Hide resolved
video/out/vo_gpu_next.c Outdated Show resolved Hide resolved
Instead copy the data on-demand when VOCTRL_PERFORMANCE_DATA is
requested.
@haasn haasn merged commit d2c28bc into mpv-player:master Jul 2, 2023
6 of 13 checks passed
@llyyr llyyr mentioned this pull request Jul 17, 2023
@kasper93 kasper93 deleted the info_move branch October 7, 2023 15:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants