-
Notifications
You must be signed in to change notification settings - Fork 12
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
fix: deadlock in encoder when replacing filter #40
base: master
Are you sure you want to change the base?
Conversation
Could you split them up? |
sure, as separate commits or separate pull requests? |
7edaa45
to
ed2ed2e
Compare
- Added a new FIFO flag: `XXX_FIFO_PULL_POKE` - Added `sp_xxx_fifo_poke` to send a "poke" to a FIFO, forcing any pull operation with the `XXX_FIFO_PULL_POKE` flag to return with `EAGAIN` - Added `sp_xxx_fifo_peek_flags` - Added more logging
The encoder would be stuck pulling a frame when its input is disconnected, preventing it from ever linking with a new input.
Controls whether filters send EOS when destroyed.
Shows how one can hot-swap a filtergraph without killing the encoder.
ed2ed2e
to
df438cb
Compare
I pushed a fix for a suspected data race when poking a FIFO that hasn't yet reached a |
@@ -814,6 +814,10 @@ static int filter_ioctx_ctrl_cb(AVBufferRef *event_ref, void *callback_ctx, | |||
sp_frame_fifo_set_max_queued(ctx->in_pads[i]->fifo, len); | |||
} | |||
} | |||
if ((tmp_val = dict_get(event->opts, "send_eos"))) { |
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.
IMO send_eos should be a standard option that should be handled by all ioctx_ctrl functions. Could you look into adding it there?
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.
is there a single place where i can add it? are there other such "standard options"? i can't figure out where to add it so that it applies to all ioctx_ctrl
callbacks 😕 (unless im actually adding this option to all other components manually?)
int fifo_size; | ||
int send_eos; |
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.
Why do you remove fifo_size?
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.
oh my bad, i think i noticed it wasn't used anywhere so i thought i could remove it
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.
actually, it looks like it is really never used. the fifo_size
option controls sp_frame_fifo_set_max_queued
directly. should i still keep this int fifo_size
field here?
replace_node.c
example showing how one can hot-swap a filtergraphsend_eos
init option to control whether filters send EOS when destroyedXXX_FIFO_PULL_POKE
sp_xxx_fifo_poke
to send a "poke" to a FIFO, forcing any pull operation with theXXX_FIFO_PULL_POKE
flag to return withEAGAIN
encode.c
where the encoder would be stuck pulling a frame when its input is disconnected, preventing it from ever linking with a new input.sp_xxx_fifo_peek_flags