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

MediaCodec/ImageReader callbacks not Send #454

Closed
Tazdevil971 opened this issue Dec 11, 2023 · 1 comment · Fixed by #455
Closed

MediaCodec/ImageReader callbacks not Send #454

Tazdevil971 opened this issue Dec 11, 2023 · 1 comment · Fixed by #455

Comments

@Tazdevil971
Copy link

Tazdevil971 commented Dec 11, 2023

Both MediaCodec and ImageReader use callbacks, such as set_async_notify_callback, but none of the functions are marked as Send.

The documentation for MediaCodec::set_async_notify_callback specifically states that the callbacks will be invoked on an NDK internal thread, so Send should be mandatory.

For ImageReader::set_image_listener and ImageReader::set_buffer_removed_listener the documentation doesn't say anything, but from the looks of it they must run from an internal thread as well.

And after this we should see if making MediaCodec and ImageReader at least Send would make sense, as they are already internally shared between threads.

EDIT

While looking more thoroughly at the API, it may be necessary to also make Image Send, as the current recommended workflow for ImageReader is to acquire the image in the image_listener callback, and then dispatch it to another thread for processing.

Keeping Image !Send while also making the callback Send basically means that all image processing must be done in the callback, which is not ideal.

@MarijnS95
Copy link
Member

MarijnS95 commented Dec 11, 2023

Relevant documentation (always useful to link): https://developer.android.com/ndk/reference/group/media#amediacodec_setasyncnotifycallback

Note that it's not just "an NDK internal thread", it is specifically:

All callbacks are fired on one NDK internal thread.

Emphasis mine. So no more than one thread, hence no need for Sync on borrowed items. Note that the AMediaExtractor bindings state that it can be called from any thread, so #453 carries Send + Sync straight from the get-go.

Should we file an upstream documentation issue for the other two methods?

CC @spencercw

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 a pull request may close this issue.

2 participants