Created attachment 329416 [details] [review]
tee: Properly handle return value when only 1 pad

This patch handle the case when you have 1 pad (so the fast path is
being used) but this pad is removed. If we are in allow-not-linked, we
should return GST_FLOW_OK, otherwise, we should return GST_FLOW_UNLINKED
and ignore the meaningless return value obtained from pushing.

While at it, can we document this behaviour, please ?

ooc, what's the meaningless flow return value we get without this?

What behaviour do you want to see documented Xavier?

The flow return of a pad that is no longer there (released) has no meaning for the remaining pipeline. Before the fast path was added, that is the behaviour we had and what the patch fixes. This behaviour let you release a pad at any moment without pad probe and is neither documented or unit tested. I'm not actively working on that one this week, but I wanted to do both. This is a minor bug since we have told everyone to always use a pad probe callback.

Tim: the nice trick here is we can remove a "branch" from a tee without idle pad probe:

  gst_element_release_request_pad (tee, pad);
  gst_bin_remove (parent, sink);
  gst_element_set_state (sink, GST_STATE_NULL);

That sequence is safe (with the patch attached here), because if a buffer was in the sink element, setting its state to NULL will return GST_FLOW_FLUSHING, but since the pad was already removed from the tee, that return value is ignored and GST_FLOW_OK (or GST_FLOW_UNLINKED if that was last pad and allow-not-linked==FALSE) is propagated to the source.

Ok, what I thought then, just checking I understood it right.

I've been telling people this should work without a probe, so would definitely be good to fix it if that's not the case ;)

Patch looks fine of course.

Attachment 329416 [details] pushed as 4603722 - tee: Properly handle return value when only 1 pad