GNOME Bugzilla – Bug 629917
[output-selector] Recheck pending_pad after pushing a buffer
Last modified: 2010-11-22 20:46:43 UTC
Camerabin uses output-selector to control its multiple internal branches. One of these branches is the image capture one. This one only wants to receive one buffer and camerabin uses pad_probes to make output-selector active pad switching to only allow a single buffer to go through on the image capture branch. The problem is that output-selector might push 2 buffers on its chain function when resend-latest is true. In pseudocode, here's what output-selector does in its chain: chain (buf) { if (pending_pad_switch) { change_active_pad() resend_latest_buffer() } push_new_buffer() } As you can see, if we add a buffer probe to the src pad and we change the active pad on it we will still get a second buffer from the chain function on the old (now not active) pad. I was not sure if this is desirable as I see pad probes as a natural way to control output selector's pads. So I wanted to raise a discussion. If people agree that this is not a good idea, I'll just do a fix inside camerabin.
Created attachment 170481 [details] [review] outputselector: Recheck pending switch after pushing buffer This patch makes output-selector always recheck if there's a pending pad switch after pushing a buffer, preventing that it pushes a buffer on the 'wrong' pad.
Created attachment 170482 [details] [review] outputselector: Avoid losing the last_buffer when switching This patch makes outputselector take an extra when pushing the last_buffer to avoid it losing it during the switch function. This makes resend-latest properly work if the active-pad is changed during the switch function buffer pushing (on a pad probe, for example).
Review of attachment 170482 [details] [review]: The commit message misses a "ref" in the first line after "This patch makes outputselector take an extra". The patch looks okay, the buffer is freed in _chain or _reset().
a grep on gstreamer modules shows that no other element currently uses output-selector.
Pushing a buffer on a not-anymore-active branch sounds buggy. So the patches make sense to me. The current behaviour of output-selector is also causing this test failure and the patches fix them. Unexpected critical/warning: Changing the `location' property on filesink when a file is open is not supported. 87%: Checks: 8, Failures: 1, Errors: 0 ../../../../libs/gst/check/gstcheck.c:72:F:general:test_image_tags_setting:0: Unexpected critical/warning: Changing the `location' property on filesink when a file is open is not supported.
commit 8d5cfc6511ca986f1a0511bfd28c5b4209aca803 commit 0451adeabd9ffb443795055295655fbe6347fe3d Fixed.
*** Bug 631399 has been marked as a duplicate of this bug. ***