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

Bugfix/js functions use single pointer options #36

Conversation

reinhrst
Copy link
Contributor

As discussed in #34 , this PR patches both av_dict_set_js and avio_open2_js to accept a AVDictionary* rather than a AVDictionary ** (since the JS code only exposed AVDictionary*).

Note that this does mean that av_dict_set_js will not be able to return a list of unused options (as ffmpeg's av_dict_set does).

I changed the 500-demuxer-decode test to include some arbitrary options dictionary. This test will fail without the code change, since free will be called on something that is not a pointer.

The JS versions of the functions should take single-pointer AVDictionary-options.
This fixes Yahweasel#34
This change only tests adding any kind of options to libav.av_dict_set_js.
Without this PR this results in an error (since it will try to free something that is not a pointer),
with this PR this will not result in an error anymore.

Note that the option set in the test is unknown to the demuxer and has no effect.
@Yahweasel
Copy link
Owner

I really can't thank you enough for finding these things. This is a corner of the types that I'd never really fought, so never really tested. I'll review these changes later today.

By the way, I believe that in one of these earlier interactions you mentioned that you were doing something with HLS, so if you didn't notice, you may want to know that I added HLS support directly to libav.js. For the tests I've done it works great, but it's also not in the test suite, since that requires external stuff to test correctly ^^´. You can find info on it in IO.md.

@reinhrst
Copy link
Contributor Author

No worried, happy I can contribute to this amazing project :)

I was not the person asking about HLS -- I'm only trying to read/write to/from FileSystemFileHandles (for now). Still, good reminder to check what is new in 4.6.6 (I like commits called "Switched to a much more robust dependencies system" :)).

@Yahweasel Yahweasel merged commit 071eee7 into Yahweasel:master Nov 17, 2023
@reinhrst reinhrst deleted the bugfix/js-functions-use-single-pointer-options branch December 1, 2023 18:32
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.

None yet

2 participants