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

Add bitstream filters #1375

Merged
merged 4 commits into from
Apr 9, 2024
Merged

Add bitstream filters #1375

merged 4 commits into from
Apr 9, 2024

Conversation

skeskinen
Copy link
Contributor

Loosely based on #565
Additional discussion in #489

This adds bindings for AVBSFContext (BitStreamFilterContext)

I decided against adding binding for AVBitStreamFilter, because it seems like a little bit useless API. You can't do anything with such object besides creating a new BSF context. (except checking which codecs are supported for that BSF, I guess)
It makes the code simpler and imo easier to maintain. Let me know if this is a mistake and those bindings should be added as well.

I think it makes sense to force BitstreamFilterContext creation through the string parsing API, since that's the only way I can see how to set filter options and also allows things like filter chains, etc.

Adds tests for 'chomp', 'setts' and 'h264_mp4toannexb' filters. We could also test that filter chains work.

h264_mp4toannexb requires that AVCodecParameters *par_in is set correctly. To that end BitStreamFilterContext takes a Stream as an argument and copies the codec parameters from there.

There are also attributes time_base_in, time_base_out which are probably required for 100% support of setts option constraints. Let me know if those should be hooked up as well. I guess it would require that BitStreamFilterContext takes separate in_stream, out_stream arguments instead of single stream.

@skeskinen
Copy link
Contributor Author

skeskinen commented Apr 9, 2024

Excuse me about the linter stuff. I can't run make lint locally because the flake8 always fails. I think it should work now. Both black and isort are happy

@WyattBlue
Copy link
Member

I think the PR is good as it is now.
One thing I changed was inlining def self._recv and def self._send to cpdef filter. This eliminates the overhead in calling Python functions and IMO makes it easier to read. I also add type hints and docstrings.

I'll give you a chance to comment, then it can be merged.

@skeskinen
Copy link
Contributor Author

Looks good to me. Thanks!

@WyattBlue WyattBlue merged commit 82e3397 into PyAV-Org:main Apr 9, 2024
14 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