After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 795332 - flvmux: gst_flv_mux_find_best_pad being called as fast as possible (tight-loop)
flvmux: gst_flv_mux_find_best_pad being called as fast as possible (tight-loop)
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-good
git master
Other All
: Normal blocker
: 1.14.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2018-04-17 14:39 UTC by Håvard Graff (hgr)
Modified: 2018-05-04 12:02 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
aggregator: Add API to check if a pad has a new buffer (1.74 KB, patch)
2018-04-23 15:58 UTC, Olivier Crête
committed Details | Review
flvmux: Save the current position in the output segment (1.08 KB, patch)
2018-04-23 15:58 UTC, Olivier Crête
committed Details | Review
flvmux: Don't wake up the muxer unless there is data (1.32 KB, patch)
2018-04-23 15:59 UTC, Olivier Crête
committed Details | Review

Description Håvard Graff (hgr) 2018-04-17 14:39:42 UTC
When logging some normal operation of flvmux running it with gst-launch, I noticed a lot of these:

0:00:06.363013498  4326       0xd7a000 DEBUG                 flvmux gstflvmux.c:1813:gst_flv_mux_get_next_time_for_segment:<mux> next_time: 0:00:00.000000000
0:00:06.363021407  4326       0xd7a000 DEBUG                 flvmux gstflvmux.c:1648:gst_flv_mux_find_best_pad:<mux> counter = 135650: Best pad found with 99:99:99.999999999: (NULL)

Adding some simple static counters to measure, I found that after having popped 500 buffers (audio and video), gst_flv_mux_find_best_pad had been called 131909 times...
Comment 1 Olivier Crête 2018-04-17 17:30:39 UTC
I guess there is a codepath where it fails to block on the clock...
Comment 2 Mathieu Duponchelle 2018-04-17 22:53:09 UTC
If this happens at EOS, I fixed a similar issue in acb6090e47ac36495c92ba30a9656e566ca1bc95 for what it's worth :)
Comment 3 Sebastian Dröge (slomo) 2018-04-18 07:22:13 UTC
Should also check if the same happens for audiomixer/compositor and others. From what I understood hgr wanted to check that yesterday?
Comment 4 Vinod Kesti 2018-04-19 09:15:00 UTC
CPU usage is normal with audiomixer/compositor. 
I used below pipelines to test this. 

gst-launch-1.0 compositor name=comp sink_1::alpha=0.5 sink_1::xpos=50 sink_1::ypos=50 ! \
  videoconvert ! fakesink \
  videotestsrc is-live=true pattern=snow timestamp-offset=3000000000 ! \
  "video/x-raw,format=AYUV,width=640,height=480,framerate=(fraction)30/1" ! \
  timeoverlay ! queue2 ! comp. \
  videotestsrc is-live=true pattern=smpte ! \
  "video/x-raw,format=AYUV,width=800,height=600,framerate=(fraction)10/1" ! \
  timeoverlay ! queue2 ! comp.
  
	
gst-launch-1.0 \
audiotestsrc is-live=true freq=100 ! audiomixer name=mix ! audioconvert ! fakesink \
audiotestsrc is-live=true freq=500 ! mix.

But with flvmux CPU usage is more than 100% due to tight loop.
Comment 5 Olivier Crête 2018-04-23 15:58:01 UTC
Created attachment 371283 [details] [review]
aggregator: Add API to check if a pad has a new buffer

This goes into the core to add a more simple convenience API.
Comment 6 Olivier Crête 2018-04-23 15:58:53 UTC
Created attachment 371284 [details] [review]
flvmux: Save the current position in the output segment
Comment 7 Olivier Crête 2018-04-23 15:59:14 UTC
Created attachment 371285 [details] [review]
flvmux: Don't wake up the muxer unless there is data

This patch depends on the patch from bug #794722
Comment 8 Olivier Crête 2018-04-26 19:45:10 UTC
Attachment 371284 [details] pushed as 11297c3 - flvmux: Save the current position in the output segment
Attachment 371285 [details] pushed as c2c7d11 - flvmux: Don't wake up the muxer unless there is data
Comment 9 Olivier Crête 2018-04-26 19:46:03 UTC
Comment on attachment 371283 [details] [review]
aggregator: Add API to check if a pad has a new buffer

Pushed as 96c3a635ce098c9bf2d692354e0c1e9b9bf64640
Comment 10 Sebastian Dröge (slomo) 2018-04-27 05:59:11 UTC
This should also go into 1.14 as it's a regression
Comment 11 Olivier Crête 2018-05-04 12:01:55 UTC
Also pushed new aggregator API to 1.14
Comment 12 Olivier Crête 2018-05-04 12:02:35 UTC
Also pushed to 1.14 branch