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 783754 - splitmuxsink: Added new async-finalize mode
splitmuxsink: Added new async-finalize mode
Status: RESOLVED OBSOLETE
Product: GStreamer
Classification: Platform
Component: gst-plugins-good
unspecified
Other All
: Normal enhancement
: 1.15.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2017-06-13 14:45 UTC by Vivia Nikolaidou
Modified: 2018-11-03 15:20 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
splitmuxsink: Added new async-finalize mode (18.79 KB, patch)
2017-06-13 14:45 UTC, Vivia Nikolaidou
none Details | Review
splitmuxsink: Added new async-finalize mode (18.86 KB, patch)
2017-06-14 15:15 UTC, Vivia Nikolaidou
none Details | Review
splitmuxsink: Added new async-finalize mode (18.86 KB, patch)
2017-06-15 08:46 UTC, Vivia Nikolaidou
none Details | Review
splitmuxsink: Added new async-finalize mode (18.93 KB, patch)
2017-06-15 14:20 UTC, Vivia Nikolaidou
none Details | Review
splitmuxsink: Added new async-finalize mode (18.93 KB, patch)
2017-06-15 14:37 UTC, Vivia Nikolaidou
none Details | Review
splitmuxsink: Added new async-finalize mode (23.52 KB, patch)
2017-07-03 16:47 UTC, Vivia Nikolaidou
none Details | Review
splitmuxsink: Added new async-finalize mode (31.38 KB, patch)
2017-07-07 12:12 UTC, Vivia Nikolaidou
none Details | Review
splitmuxsink: Added new async-finalize mode (34.65 KB, patch)
2018-03-22 13:17 UTC, Jan Schmidt
none Details | Review
splitmuxsink: Added new async-finalize mode (29.88 KB, patch)
2018-04-27 13:02 UTC, Vivia Nikolaidou
none Details | Review
splitmuxsink: Added new async-finalize mode (36.46 KB, patch)
2018-05-05 13:10 UTC, Vivia Nikolaidou
none Details | Review
splitmuxsink: Added new async-finalize mode (36.81 KB, patch)
2018-05-08 18:10 UTC, Vivia Nikolaidou
none Details | Review
splitmuxsink: Added new async-finalize mode (37.67 KB, patch)
2018-05-09 17:47 UTC, Vivia Nikolaidou
none Details | Review
splitmuxsink: Added new async-finalize mode (38.01 KB, patch)
2018-05-09 18:00 UTC, Vivia Nikolaidou
none Details | Review
splitmuxsink: Added new async-finalize mode (38.26 KB, patch)
2018-05-11 13:48 UTC, Vivia Nikolaidou
committed Details | Review

Description Vivia Nikolaidou 2017-06-13 14:45:55 UTC
See commit message
Comment 1 Vivia Nikolaidou 2017-06-13 14:45:59 UTC
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.
Comment 2 Tim-Philipp Müller 2017-06-13 15:10:53 UTC
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?
Comment 3 Vivia Nikolaidou 2017-06-13 15:29:53 UTC
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.
Comment 4 Vivia Nikolaidou 2017-06-14 15:15:48 UTC
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.
Comment 5 Vivia Nikolaidou 2017-06-15 08:46:04 UTC
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.
Comment 6 Vivia Nikolaidou 2017-06-15 14:20:59 UTC
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.
Comment 7 Vivia Nikolaidou 2017-06-15 14:37:31 UTC
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.
Comment 8 Vivia Nikolaidou 2017-07-03 16:47:07 UTC
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.
Comment 9 Vivia Nikolaidou 2017-07-03 16:49:14 UTC
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.
Comment 10 Jan Schmidt 2017-07-04 06:11:21 UTC
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.
Comment 11 Vivia Nikolaidou 2017-07-07 12:12:39 UTC
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.
Comment 12 Vivia Nikolaidou 2017-07-07 12:15:01 UTC
(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.
Comment 13 Jan Schmidt 2018-03-22 13:17:35 UTC
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.
Comment 14 Jan Schmidt 2018-03-22 13:21:05 UTC
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.
Comment 15 Vivia Nikolaidou 2018-04-27 13:02:59 UTC
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.
Comment 16 Vivia Nikolaidou 2018-04-27 13:03:52 UTC
Rebased against current master.
Comment 17 Vivia Nikolaidou 2018-05-05 13:10:27 UTC
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.
Comment 18 Vivia Nikolaidou 2018-05-05 13:10:57 UTC
Fixed filename in fragment-opened and fragment-closed messages.
Comment 19 Vivia Nikolaidou 2018-05-08 18:10:15 UTC
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.
Comment 20 Vivia Nikolaidou 2018-05-08 18:12:38 UTC
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.
Comment 21 Vivia Nikolaidou 2018-05-09 17:47:34 UTC
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.
Comment 22 Vivia Nikolaidou 2018-05-09 17:49:40 UTC
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.
Comment 23 Vivia Nikolaidou 2018-05-09 18:00:06 UTC
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.
Comment 24 Vivia Nikolaidou 2018-05-09 18:00:33 UTC
Oops, wrong patch. This is the right one.
Comment 25 Vivia Nikolaidou 2018-05-11 13:48:13 UTC
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.
Comment 26 Vivia Nikolaidou 2018-05-11 13:48:51 UTC
Rebased to current master.
Comment 27 Vivia Nikolaidou 2018-05-24 09:48:54 UTC
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
Comment 28 Vivia Nikolaidou 2018-05-24 09:49:20 UTC
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
Comment 29 GStreamer system administrator 2018-11-03 15:20:04 UTC
-- 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.