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

Fix some potential deadlocks and H.264 video decode error when seeking #347

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

thesword53
Copy link
Contributor

This commit fixes 2 possible deadlocks:

  • When a decode error occurs, the surface condition should be signaled otherwise decoding will hang as in Decode error when seeking in H.264 videos #343
  • deleteAllObjects should delete all objects not only context objects. I also removed deleteObject because it also calls pthread_mutex_lock(&drv->objectCreationMutex);, so it will cause a deadlock.

@thesword53 thesword53 changed the title Fix some potential deadlocks Fix some potential deadlocks and H.264 video decode error when seeking Jan 9, 2025
@thesword53
Copy link
Contributor Author

I added a commit to fix #343. In nvCreateContext, I set surfaceCount to 32 if num_render_targets as video players can create surfaces with vaCreateSurfaces/ vaCreateSurfaces2, after calling vaCreateContext.

@elFarto
Copy link
Owner

elFarto commented Jan 13, 2025

Thanks for the effort! A couple of comments:

  • The objectCreationMutex lock is created as a recursive lock, so it can be taken repeatedly without issues.
  • I'm not sure I like that those macros have been removed for the deleteAllObjects. I can see why you needed to do it, but it doesn't feel right to have all that array manipulation there. I think we need a new method in list.c call something like clear_array that frees all elements, NULLs out the element, and sets the list size to 0. That leaves deleteAllObjects just freeing o->obj.

@thesword53
Copy link
Contributor Author

Thanks for the effort! A couple of comments:

* The `objectCreationMutex` lock is created as a recursive lock, so it can be taken repeatedly without issues.

* I'm not sure I like that those macros have been removed for the `deleteAllObjects`. I can see why you needed to do it, but it doesn't feel right to have all that array manipulation there. I think we need a new method in `list.c` call something like `clear_array` that frees all elements, NULLs out the element, and sets the list size to 0. That leaves `deleteAllObjects` just freeing o->obj.

I reverted changes in deleteAllObjects

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