GNOME Bugzilla – Bug 776999
queue2: Avoid return flushing for a not-linked state
Last modified: 2017-03-17 04:52:38 UTC
Queue2 element might return FLUSHING although it's not-linked condition. This frequently happened with track switching in case of playbin3 pipeline.
Created attachment 343111 [details] [review] adaptivedemux: Restart streaming even if last flow was FLUSHING
This sounds like a bug in queue2 instead of adaptivedemux?
(In reply to Olivier Crête from comment #2) > This sounds like a bug in queue2 instead of adaptivedemux? It might be bug of queue2, but I'm not sure it's really bug. I suspect following code causes return FLUSHING https://cgit.freedesktop.org/gstreamer/gstreamer/tree/plugins/elements/gstqueue2.c#n2558
Do you have a test application to reproduce this issue? Or how do you reproduce it manually?
Another option is providing GST_DEBUG=6 logs of the issue.
Created attachment 343974 [details] Reproduced log
Created attachment 343975 [details] [review] patch for print log
(In reply to Thiago Sousa Santos from comment #5) > Another option is providing GST_DEBUG=6 logs of the issue. Please refer to attached log file. To reproduce this issue, I applied a patch for supporting playbin3 track-change by gst-play https://bugzilla.gnome.org/show_bug.cgi?id=775469 On playbin3, reproducing sequence is, repeated audio track change by press 'a' on playbin3. Test mpd : http://dash.akamaized.net/dash264/TestCases/10a/1/iis_forest_short_poem_multi_lang_480p_single_adapt_aaclc_sidx.mpd
(In reply to Seungha Yang from comment #3) > (In reply to Olivier Crête from comment #2) > > This sounds like a bug in queue2 instead of adaptivedemux? > > It might be bug of queue2, but I'm not sure it's really bug. > I suspect following code causes return FLUSHING > https://cgit.freedesktop.org/gstreamer/gstreamer/tree/plugins/elements/ > gstqueue2.c#n2558 Indeed this seems to be the issue. queue2 is returning flushing when it should be not-linked
Created attachment 343980 [details] [review] queue2: avoid return flushing if we have a not-linked This could fix it. Though I'm not sure returning not-linked for events is the right thing to do. Maybe we can enqueue them and return GST_FLOW_OK and leave the GST_FLOW_NOT_LINKED returned for when buffers are pushed, like queue does. What do you think?
(In reply to Thiago Sousa Santos from comment #10) > Created attachment 343980 [details] [review] [review] > This could fix it. Though I'm not sure returning not-linked for > events is the right thing to do. Maybe we can enqueue them and > return GST_FLOW_OK and leave the GST_FLOW_NOT_LINKED returned > for when buffers are pushed, like queue does. > > What do you think? I think it looks good, but I'd like to reason Tim-Philipp Müller's opinion. because queue2's this behaviour can be related to following bug https://bugzilla.gnome.org/show_bug.cgi?id=777009
Comment on attachment 343980 [details] [review] queue2: avoid return flushing if we have a not-linked This seems correct (same thing for queue and multiqueue please when merging)
Comment on attachment 343111 [details] [review] adaptivedemux: Restart streaming even if last flow was FLUSHING This seems to fix it at the wrong place
(In reply to Sebastian Dröge (slomo) from comment #12) > Comment on attachment 343980 [details] [review] [review] > queue2: avoid return flushing if we have a not-linked > > This seems correct (same thing for queue and multiqueue please when merging) This queue2 modification works well for me. Could we merge this patch?
The same change also should be done for queue and multiqueue. And if we don't do it now when merging the queue2 part, we will just forget :) Want to provide a patch for those? It's the same logic
Hi Seungha, are you working on porting the logic? Let me know if that is the case or I could do it this week and merge both patches.
(In reply to Thiago Sousa Santos from comment #16) > Hi Seungha, are you working on porting the logic? Let me know if that is the > case or I could do it this week and merge both patches. I didn't work for it now.. Could you please porting that on queue/multiqueue? :)
Created attachment 347982 [details] [review] queue: avoid return flushing if we have a not-linked Return the correct flow return instead of returning always flushing. This would cause queue to convert not-linked to flushing and making upstream elements stop. Based on the previous patch for queue2.
multiqueue doesn't have the issue.
Comment on attachment 347982 [details] [review] queue: avoid return flushing if we have a not-linked Just wondering, shouldn't this still convert any queue->srcresult == GST_FLOW_OK to GST_FLOW_FLUSHING here, and only pass through the other ones?
Created attachment 348061 [details] [review] qtmux: Join chunks together if they have the same size This is the piece of the original patch that fixes it for 1.10. Should I push this or the original version?
Comment on attachment 348061 [details] [review] qtmux: Join chunks together if they have the same size Sorry, wrong bug number.
(In reply to Sebastian Dröge (slomo) from comment #20) > Comment on attachment 347982 [details] [review] [review] > queue: avoid return flushing if we have a not-linked > > Just wondering, shouldn't this still convert any queue->srcresult == > GST_FLOW_OK to GST_FLOW_FLUSHING here, and only pass through the other ones? If I understand your question correctly. The QUEUE_MUTEX_LOCK_CHECK checks for GST_FLOW_OK and only jumps into the label if it isn't in OK. So it can only reach that code if it isn't with q OK result so far.
Indeed, sorry for the confusion
commit 058bdcfe6b6ae4c1ceb8c31560d2a9bffe075e1d Author: Thiago Santos <thiagossantos@gmail.com> Date: Tue Mar 14 22:18:36 2017 -0700 queue: avoid return flushing if we have a not-linked Return the correct flow return instead of returning always flushing. This would cause queue to convert not-linked to flushing and making upstream elements stop. Based on the previous patch for queue2. https://bugzilla.gnome.org/show_bug.cgi?id=776999 commit 045700e80ed3c5bb9c5af04a8dfa9d0210d05fe3 Author: Thiago Santos <thiagossantos@gmail.com> Date: Sun Jan 22 11:26:56 2017 -0300 queue2: avoid return flushing if we have a not-linked Return the correct flow return instead of returning always flushing. This would cause queue2 to convert not-linked to flushing and making upstream elements stop. https://bugzilla.gnome.org/show_bug.cgi?id=776999