-
Notifications
You must be signed in to change notification settings - Fork 12
fix: deadlock in encoder when replacing filter #40
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
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 |
| 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 dump_graph; | ||
| 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.cexample showing how one can hot-swap a filtergraphsend_eosinit option to control whether filters send EOS when destroyedXXX_FIFO_PULL_POKEsp_xxx_fifo_poketo send a "poke" to a FIFO, forcing any pull operation with theXXX_FIFO_PULL_POKEflag to return withEAGAINencode.cwhere 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