Skip to content

Fixed crashes caused by invalid concurrent writes to drv->nextObjId #366

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

Merged
merged 1 commit into from
May 8, 2025

Conversation

thesword53
Copy link
Contributor

I restored the old behavior in nvCreateContext with num_render_targets and I implemented reconfigureDecoderSurfaceCount to reconfigure decoder if the application create surfaces after context creation.
I also rounded up memory size to 4096 in alloc_image as allocation seems to fail in mpv with the 570.133.07 driver:
[drm:__nv_drm_nvkms_gem_obj_init [nvidia_drm]] *ERROR* [nvidia-drm] [GPU ID 0x00002d00] NvKmsKapiMemory 0x00000000e087270f size should be in a multiple of page size to create a gem object

@thesword53 thesword53 marked this pull request as draft April 13, 2025 21:48
@thesword53 thesword53 changed the title Reconfigure CUVID decoder if the application creates surfaces after context creation Fixed crashes caused by invalid concurrent writes to drv->nextObjId May 3, 2025
@thesword53 thesword53 marked this pull request as ready for review May 3, 2025 16:11
@thesword53
Copy link
Contributor Author

Hello @elFarto, I modified the pull request to properly address crashes with Firefox 137 when decoding multiple video streams #359. The increment of drv->nextObjId should occur in drv->objectCreationMutex as objects can be created concurrently in multiple threads.

newObj->id = (++drv->nextObjId);

Firefox 137 seems to use 1 driver context with multiple decode contexts and prior versions are using multiple driver context with 1 decode context. It would explain why we can't observe crashes in Firefox < 137

Check if we have the right object type in `getObject`, `getObjectPtr` and `getObjectByPtr`.
Fixed some integer types.
@elFarto elFarto merged commit 8e6a439 into elFarto:master May 8, 2025
2 checks passed
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.

2 participants