-
Notifications
You must be signed in to change notification settings - Fork 111
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
ndk/media_codec: Wrap common dequeued-buffer status codes in enum #401
Conversation
08b28ce
to
c6c626c
Compare
The dequeue API for output buffers can commonly return `AMEDIACODEC_INFO_OUTPUT_FORMAT_CHANGED` or `AMEDIACODEC_INFO_OUTPUT_BUFFERS_CHANGED`, both of which are not caught in `NdkMediaError::from_status()` and show up as an "unknown" error. Neither really is an error that the user would want to bubble up from the `Result<>` via `?`, but should rather handle this explicitly. A similar case already exists for `AMEDIACODEC_INFO_TRY_AGAIN_LATER` which previously resulted in a `None`, but is now also encapsuled in a self-documenting manner, for the dequeue_input_buffer()` API as well.
c6c626c
to
c95567a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
did all the archaeology for #316 and these issues android/ndk#1768 + https://issuetracker.google.com/u/1/issues/255698045?pli=1
fun :/
I haven't had to use these media APIs but yeah this all seems reasonable to me
Maybe also add @zarik5 as a reviewer considering they are really depending on this API |
We've already bikeshedded this a bit in #316, I cannot assign non-collaborators for reviewers and on a prior occasion their comment was basically that they did not have time to maintain the code they contributed... so I'm not super interested in waiting for that opinion either. |
I already discussed these changes in the other PR, and beside the exact naming scheme,this API will suit my needs. |
I switched to the rust-mobile/ndk git master in production and it works good so far. |
@zarik5 thanks for the positive feedback, glad to hear that we might release this soon :) |
Supersedes #316, CC @zarik5
Closes #316
The dequeue API for output buffers can commonly return
AMEDIACODEC_INFO_OUTPUT_FORMAT_CHANGED
orAMEDIACODEC_INFO_OUTPUT_BUFFERS_CHANGED
, both of which are not caught inNdkMediaError::from_status()
and show up as an "unknown" error. Neither really is an error that the user would want to bubble up from theResult<>
via?
, but should rather handle this explicitly.A similar case already exists for
AMEDIACODEC_INFO_TRY_AGAIN_LATER
which previously resulted in aNone
, but is now also encapsuled in a self-documenting manner, for the dequeue_input_buffer()` API as well.