GNOME Bugzilla – Bug 783754
splitmuxsink: Added new async-finalize mode
Last modified: 2018-11-03 15:20:04 UTC
See commit message
Created attachment 353696 [details] [review] splitmuxsink: Added new async-finalize mode This mode is useful for muxers that can take a long time to finalize a file. Instead of blocking the whole upstream pipeline while the muxer is doing its stuff, we can unlink it and spawn a new muxer+sink combination to continue running normally. This requires us to receive the muxer and sink (if needed) as factories, optionally accompanied by their respective properties structures. Also added the muxer-added and sink-added signals, in case custom code has to be called for them.
Out of curiosity - could you elaborate more on the scenario where the muxer takes too long to finalise things? Why is that? What muxer is it, etc?
Sometimes the file has to be rewritten during EOS, and if it's a very long file (MXF with XDCAM in our case), it can take up to one minute to rewrite it.
Created attachment 353747 [details] [review] splitmuxsink: Added new async-finalize mode This mode is useful for muxers that can take a long time to finalize a file. Instead of blocking the whole upstream pipeline while the muxer is doing its stuff, we can unlink it and spawn a new muxer+sink combination to continue running normally. This requires us to receive the muxer and sink (if needed) as factories, optionally accompanied by their respective properties structures. Also added the muxer-added and sink-added signals, in case custom code has to be called for them.
Created attachment 353804 [details] [review] splitmuxsink: Added new async-finalize mode This mode is useful for muxers that can take a long time to finalize a file. Instead of blocking the whole upstream pipeline while the muxer is doing its stuff, we can unlink it and spawn a new muxer+sink combination to continue running normally. This requires us to receive the muxer and sink (if needed) as factories, optionally accompanied by their respective properties structures. Also added the muxer-added and sink-added signals, in case custom code has to be called for them.
Created attachment 353830 [details] [review] splitmuxsink: Added new async-finalize mode This mode is useful for muxers that can take a long time to finalize a file. Instead of blocking the whole upstream pipeline while the muxer is doing its stuff, we can unlink it and spawn a new muxer+sink combination to continue running normally. This requires us to receive the muxer and sink (if needed) as factories, optionally accompanied by their respective properties structures. Also added the muxer-added and sink-added signals, in case custom code has to be called for them.
Created attachment 353832 [details] [review] splitmuxsink: Added new async-finalize mode This mode is useful for muxers that can take a long time to finalize a file. Instead of blocking the whole upstream pipeline while the muxer is doing its stuff, we can unlink it and spawn a new muxer+sink combination to continue running normally. This requires us to receive the muxer and sink (if needed) as factories, optionally accompanied by their respective properties structures. Also added the muxer-added and sink-added signals, in case custom code has to be called for them.
Created attachment 354845 [details] [review] splitmuxsink: Added new async-finalize mode This mode is useful for muxers that can take a long time to finalize a file. Instead of blocking the whole upstream pipeline while the muxer is doing its stuff, we can unlink it and spawn a new muxer+sink combination to continue running normally. This requires us to receive the muxer and sink (if needed) as factories, optionally accompanied by their respective properties structures. Also added the muxer-added and sink-added signals, in case custom code has to be called for them.
This should be OK now - it's been tested with the initial "broken" use case and has been found to solve (work around) the issue, with all pre-existing unit tests still passing. Please review this while I'm making some unit tests with asyncfinalize enabled.
Review of attachment 354845 [details] [review]: It would be good to add some more text to the element blurb at the top talking about async-finalize mode, and how to use the factory and properties properties. ::: gst/multifile/gstsplitmuxsink.c @@ +1485,3 @@ + NULL); + gst_element_call_async (sink, _lock_and_set_to_null, splitmux, + NULL); I don't think these calls should be async. At this point the muxer and sink should already be EOS, so it shouldn't take long to reach NULL synchronously. Otherwise, there's no guarantee the elements still exist by the time the callback happens since there's no ref held.
Created attachment 355085 [details] [review] splitmuxsink: Added new async-finalize mode This mode is useful for muxers that can take a long time to finalize a file. Instead of blocking the whole upstream pipeline while the muxer is doing its stuff, we can unlink it and spawn a new muxer+sink combination to continue running normally. This requires us to receive the muxer and sink (if needed) as factories, optionally accompanied by their respective properties structures. Also added the muxer-added and sink-added signals, in case custom code has to be called for them.
(In reply to Jan Schmidt from comment #10) > Review of attachment 354845 [details] [review] [review]: > > ::: gst/multifile/gstsplitmuxsink.c > @@ +1485,3 @@ > + NULL); > + gst_element_call_async (sink, _lock_and_set_to_null, splitmux, > + NULL); > > I don't think these calls should be async. At this point the muxer and sink > should already be EOS, so it shouldn't take long to reach NULL > synchronously. Otherwise, there's no guarantee the elements still exist by > the time the callback happens since there's no ref held. They should be async, at least for the sink - because we're at the callback of the EOS message held by the sink. Added a comment and a ref. Also fixed some small races (most notably, there was no guarantee anymore that the reference context would have a buffer at the start of a new fragment, so format-location-full might not have a buffer attached to it - fixed that), and added automated tests.
Created attachment 370011 [details] [review] splitmuxsink: Added new async-finalize mode This mode is useful for muxers that can take a long time to finalize a file. Instead of blocking the whole upstream pipeline while the muxer is doing its stuff, we can unlink it and spawn a new muxer+sink combination to continue running normally. This requires us to receive the muxer and sink (if needed) as factories, optionally accompanied by their respective properties structures. Also added the muxer-added and sink-added signals, in case custom code has to be called for them.
New patch attached the fixes some problems I found - primarily that the async-finalisation mode starts a new fragment too early. It starts the new fragment as soon as a pad enters the ENDING_FILE mode, without first waiting for each other pad to send the related buffers to reach the fragment-end-timestamp. Instead we track which contexts have gone 'async-eos' and start the new fragment as soon as all of them have queued up an EOS to the old muxer pad.
Created attachment 371458 [details] [review] splitmuxsink: Added new async-finalize mode This mode is useful for muxers that can take a long time to finalize a file. Instead of blocking the whole upstream pipeline while the muxer is doing its stuff, we can unlink it and spawn a new muxer+sink combination to continue running normally. This requires us to receive the muxer and sink (if needed) as factories, optionally accompanied by their respective properties structures. Also added the muxer-added and sink-added signals, in case custom code has to be called for them.
Rebased against current master.
Created attachment 371711 [details] [review] splitmuxsink: Added new async-finalize mode This mode is useful for muxers that can take a long time to finalize a file. Instead of blocking the whole upstream pipeline while the muxer is doing its stuff, we can unlink it and spawn a new muxer+sink combination to continue running normally. This requires us to receive the muxer and sink (if needed) as factories, optionally accompanied by their respective properties structures. Also added the muxer-added and sink-added signals, in case custom code has to be called for them.
Fixed filename in fragment-opened and fragment-closed messages.
Created attachment 371811 [details] [review] splitmuxsink: Added new async-finalize mode This mode is useful for muxers that can take a long time to finalize a file. Instead of blocking the whole upstream pipeline while the muxer is doing its stuff, we can unlink it and spawn a new muxer+sink combination to continue running normally. This requires us to receive the muxer and sink (if needed) as factories, optionally accompanied by their respective properties structures. Also added the muxer-added and sink-added signals, in case custom code has to be called for them.
Previous version was prepending each context to the contexts list when creating it. That caused the order of audio streams to be reversed at the second fragment onwards. Using g_list_append instead of g_list_prepend.
Created attachment 371865 [details] [review] splitmuxsink: Added new async-finalize mode This mode is useful for muxers that can take a long time to finalize a file. Instead of blocking the whole upstream pipeline while the muxer is doing its stuff, we can unlink it and spawn a new muxer+sink combination to continue running normally. This requires us to receive the muxer and sink (if needed) as factories, optionally accompanied by their respective properties structures. Also added the muxer-added and sink-added signals, in case custom code has to be called for them.
fragment-closed messages could sometimes (racily) contain the wrong running-time, if the sink took its time to finalise the fragment. Fixed by setting the out_running_time as qdata on the sink and retrieving it when sending the message.
Created attachment 371866 [details] [review] splitmuxsink: Added new async-finalize mode This mode is useful for muxers that can take a long time to finalize a file. Instead of blocking the whole upstream pipeline while the muxer is doing its stuff, we can unlink it and spawn a new muxer+sink combination to continue running normally. This requires us to receive the muxer and sink (if needed) as factories, optionally accompanied by their respective properties structures. Also added the muxer-added and sink-added signals, in case custom code has to be called for them.
Oops, wrong patch. This is the right one.
Created attachment 371940 [details] [review] splitmuxsink: Added new async-finalize mode This mode is useful for muxers that can take a long time to finalize a file. Instead of blocking the whole upstream pipeline while the muxer is doing its stuff, we can unlink it and spawn a new muxer+sink combination to continue running normally. This requires us to receive the muxer and sink (if needed) as factories, optionally accompanied by their respective properties structures. Also added the muxer-added and sink-added signals, in case custom code has to be called for them.
Rebased to current master.
commit d11339d61689a750672cffc6b88906e5bd977ee0 (HEAD -> master, dev/master) Author: Vivia Nikolaidou <vivia@ahiru.eu> Date: Tue Jun 13 17:42:55 2017 +0300 splitmuxsink: Added new async-finalize mode This mode is useful for muxers that can take a long time to finalize a file. Instead of blocking the whole upstream pipeline while the muxer is doing its stuff, we can unlink it and spawn a new muxer+sink combination to continue running normally. This requires us to receive the muxer and sink (if needed) as factories, optionally accompanied by their respective properties structures. Also added the muxer-added and sink-added signals, in case custom code has to be called for them. https://bugzilla.gnome.org/show_bug.cgi?id=783754
Review of attachment 371940 [details] [review]: commit d11339d61689a750672cffc6b88906e5bd977ee0 (HEAD -> master, dev/master) Author: Vivia Nikolaidou <vivia@ahiru.eu> Date: Tue Jun 13 17:42:55 2017 +0300 splitmuxsink: Added new async-finalize mode This mode is useful for muxers that can take a long time to finalize a file. Instead of blocking the whole upstream pipeline while the muxer is doing its stuff, we can unlink it and spawn a new muxer+sink combination to continue running normally. This requires us to receive the muxer and sink (if needed) as factories, optionally accompanied by their respective properties structures. Also added the muxer-added and sink-added signals, in case custom code has to be called for them. https://bugzilla.gnome.org/show_bug.cgi?id=783754
-- GitLab Migration Automatic Message -- This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/gstreamer/gst-plugins-good/issues/381.