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

Simplify framesync API #1327

Open
wants to merge 1 commit into
base: framesync_api
Choose a base branch
from
Open

Simplify framesync API #1327

wants to merge 1 commit into from

Conversation

nilfm99
Copy link
Collaborator

@nilfm99 nilfm99 commented Feb 6, 2024

The data parameter to vmaf_framesync_submit_filled_data and vmaf_framesync_release_buf was only used to check that it was the same pointer that was present in the framesync internal data structure. I believe this can be simplified.

As an example usage, the integer_funque implementation here creates two dummy variables, shared_buf_temp and dependent_buf_temp, just to pass into these functions. With this simplification, we could avoid those.

@nilfm99 nilfm99 requested a review from kylophone February 6, 2024 19:02
@nilfm99
Copy link
Collaborator Author

nilfm99 commented Feb 6, 2024

This PR is stacked on top of #1325

@kylophone
Copy link
Collaborator

I feel like it might be more intuitive to pass the data pointer back and forth rather than just index. Thoughts on that idea?

@nilfm99
Copy link
Collaborator Author

nilfm99 commented Feb 7, 2024

Sorry, could you clarify what you mean by

pass the data pointer back and forth rather than just index

is that the current status at #1325 or is there any difference in what you suggest?

@nilfm99
Copy link
Collaborator Author

nilfm99 commented Mar 21, 2024

@kylophone hi, can we go ahead with this one?

@kylophone
Copy link
Collaborator

@kylophone hi, can we go ahead with this one?

I think we still need to wait on the pr using this. I was expecting something sooner, so I'll ping them and see what's up.

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