Skip to content

Conversation

@nasso
Copy link
Contributor

@nasso nasso commented Sep 3, 2024

  • Added a replace_node.c example showing how one can hot-swap a filtergraph
  • Added a send_eos init option to control whether filters send EOS when destroyed
  • 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
  • Fix deadlock in 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.
  • Added sp_xxx_fifo_peek_flags
  • Added some logging here and there...

@cyanreg
Copy link
Owner

cyanreg commented Sep 3, 2024

Could you split them up?

@nasso
Copy link
Contributor Author

nasso commented Sep 3, 2024

sure, as separate commits or separate pull requests?

@nasso nasso force-pushed the fix-filter-swap-encoder-deadlock branch from 7edaa45 to ed2ed2e Compare September 3, 2024 13:37
nasso added 4 commits October 1, 2024 15:42
- 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.
@nasso nasso force-pushed the fix-filter-swap-encoder-deadlock branch from ed2ed2e to df438cb Compare October 1, 2024 13:43
@nasso
Copy link
Contributor Author

nasso commented Oct 1, 2024

I pushed a fix for a suspected data race when poking a FIFO that hasn't yet reached a fifo_pop call. the poke would've been "lost" and the FIFO might end up stuck forever

sp_frame_fifo_set_max_queued(ctx->in_pads[i]->fifo, len);
}
}
if ((tmp_val = dict_get(event->opts, "send_eos"))) {
Copy link
Owner

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?

Copy link
Contributor Author

@nasso nasso Oct 7, 2024

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;
Copy link
Owner

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?

Copy link
Contributor Author

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

Copy link
Contributor Author

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?

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.

2 participants