Skip to content

Prevent double notification for frame done in external frame usage #1180

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

tszumski
Copy link
Contributor

Skip notify if ext_frame is used and derive is false (input_fmt != transport_fmt).
In this case, the frame done notification is not needed since the notification is already done after convert callback.
This is to avoid double notification(and double free) for the same frame.
Fixes #1147

@tszumski tszumski force-pushed the tszumski-fix-issue-1147 branch from 5a61aed to 6fc70f3 Compare June 17, 2025 08:12
Copy link
Collaborator

@DawidWesierski4 DawidWesierski4 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@tszumski tszumski force-pushed the tszumski-fix-issue-1147 branch from 6fc70f3 to 14ec9b6 Compare June 17, 2025 10:48
Copy link
Collaborator

@Sakoram Sakoram left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this solution is insufficient. As a user I expect notify_frame_done to be called each time the buffer ownership returns to me.
Currently I see a scenario in which it is not the case:
ext_frame, but external converter is used. I guess we need to add a call to notify_frame_done in tx_st20p_convert_put_frame()

There are possible more such scenarios, please check if the user is informed in each.

@tszumski
Copy link
Contributor Author

I think this solution is insufficient. As a user I expect notify_frame_done to be called each time the buffer ownership returns to me. Currently I see a scenario in which it is not the case: ext_frame, but external converter is used. I guess we need to add a call to notify_frame_done in tx_st20p_convert_put_frame()

There are possible more such scenarios, please check if the user is informed in each.

Thanks, good catch, fixed in fd0a79b

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.

notify_frame_done called twice in Tx video pipeline mode w/ external frames
5 participants