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 761761 - splitmuxsink: Caps changes not handled
splitmuxsink: Caps changes not handled
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-good
1.7.1
Other All
: Normal normal
: 1.11.2
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2016-02-09 12:18 UTC by bob_jam11
Modified: 2017-02-17 20:12 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
splitmuxsink: Change files on incompatible caps (3.40 KB, patch)
2017-02-15 20:15 UTC, Olivier Crête
none Details | Review
splitmuxsink: Remove unused ready_for_output (1.73 KB, patch)
2017-02-15 20:15 UTC, Olivier Crête
rejected Details | Review
splitmuxsink: Remove always TRUE if() (6.46 KB, patch)
2017-02-15 20:15 UTC, Olivier Crête
rejected Details | Review
splitmuxsink: Remove unused muxed_out_time (2.01 KB, patch)
2017-02-15 20:16 UTC, Olivier Crête
committed Details | Review
splitmuxsink: Remove unused next_max_out_running_time (773 bytes, patch)
2017-02-15 20:16 UTC, Olivier Crête
committed Details | Review
splitmuxsink: Reset ready_for_output on state change (912 bytes, patch)
2017-02-15 21:35 UTC, Olivier Crête
committed Details | Review
splitmuxsink: Change files on incompatible caps (3.70 KB, patch)
2017-02-17 00:32 UTC, Olivier Crête
none Details | Review
splitmuxsink: Change files on incompatible caps (3.78 KB, patch)
2017-02-17 00:42 UTC, Olivier Crête
none Details | Review
splitmuxsink: Change files on incompatible caps (3.77 KB, patch)
2017-02-17 00:44 UTC, Olivier Crête
committed Details | Review

Description bob_jam11 2016-02-09 12:18:44 UTC
When changing caps the splitmuxsink closes the file being written to and stops processing frames.

pipeline:
appsrc -> videoscale -> capsfilter-> videorate -> videoconverter -> x264enc -> splitmuxsink

The mux in the splitmuxsink is an avimux.
The sink in the splitmuxsink is a filesink.

Incoming frames have variable frame dimensions.

If I replace the splitmuxsink with the avimux and filesink then the pipeline successfully adapts to changing frame dimensions.
Comment 1 Thiago Sousa Santos 2016-02-11 14:50:13 UTC
Could you provide a sample application or an unit test to reproduce this issue?
Comment 2 Olivier Crête 2017-02-15 20:15:49 UTC
Created attachment 345871 [details] [review]
splitmuxsink: Change files on incompatible caps
Comment 3 Olivier Crête 2017-02-15 20:15:53 UTC
Created attachment 345872 [details] [review]
splitmuxsink: Remove unused ready_for_output
Comment 4 Olivier Crête 2017-02-15 20:15:58 UTC
Created attachment 345873 [details] [review]
splitmuxsink: Remove always TRUE if()
Comment 5 Olivier Crête 2017-02-15 20:16:02 UTC
Created attachment 345874 [details] [review]
splitmuxsink: Remove unused muxed_out_time
Comment 6 Olivier Crête 2017-02-15 20:16:06 UTC
Created attachment 345875 [details] [review]
splitmuxsink: Remove unused next_max_out_running_time
Comment 7 Olivier Crête 2017-02-15 20:36:11 UTC
The first patch in the series fixes this bug by cutting a new file when the caps change and the muxer rejects that change. I'm totally unsure if I'm doing it correctly. My test case includes a RTP stream which a variable frame size.
Comment 8 Olivier Crête 2017-02-15 21:35:57 UTC
Created attachment 345884 [details] [review]
splitmuxsink: Reset ready_for_output on state change

I'm not sure if this is enough, but it was strange that it was never reset.
Comment 9 Jan Schmidt 2017-02-16 02:52:00 UTC
Review of attachment 345874 [details] [review]:

muxed_out_time isn't needed any more since I rewrote how fragments are scheduled for output / switching.
Comment 10 Jan Schmidt 2017-02-16 02:52:56 UTC
Review of attachment 345875 [details] [review]:

cool
Comment 11 Jan Schmidt 2017-02-16 02:54:54 UTC
Review of attachment 345884 [details] [review]:

Yes, thanks!
Comment 12 Jan Schmidt 2017-02-16 03:07:47 UTC
Review of attachment 345871 [details] [review]:

I'm not sure this will work entirely correctly. I think it will attempt to open a new fragment without having collected the first buffer of the new fragment, which is needed for the format-location-full signal.

We probably need some extra logic to end the old file, but defer opening the new one by setting ready_for_output to FALSE until the next video buffer arrives.
Comment 13 Olivier Crête 2017-02-17 00:32:32 UTC
Created attachment 346028 [details] [review]
splitmuxsink: Change files on incompatible caps

Here is an updated version that instead just sends the EOS
directly and drops all event until a buffer arrives so that
we have the buffer for the signal.
Comment 14 Olivier Crête 2017-02-17 00:42:50 UTC
Created attachment 346029 [details] [review]
splitmuxsink: Change files on incompatible caps
Comment 15 Olivier Crête 2017-02-17 00:44:54 UTC
Created attachment 346030 [details] [review]
splitmuxsink: Change files on incompatible caps

Updated again to reset the ready_for_output.. And also only drop sticky events as I think we want to let flushes through.
Comment 16 Jan Schmidt 2017-02-17 09:55:51 UTC
Review of attachment 346030 [details] [review]:

Looks OK to me now :)
Comment 17 Olivier Crête 2017-02-17 20:11:26 UTC
Merged:

commit d8868c633900a77139ea263b4bfe26970991d8a0
Author: Olivier Crête <olivier.crete@collabora.com>
Date:   Wed Feb 15 14:48:58 2017 -0500

    splitmuxsink: Change files on incompatible caps
    
    https://bugzilla.gnome.org/show_bug.cgi?id=761761

commit f79a7afac2413687705ba9ddb999b4914404c68b
Author: Olivier Crête <olivier.crete@collabora.com>
Date:   Wed Feb 15 16:35:01 2017 -0500

    splitmuxsink: Reset ready_for_output on state change
    
    https://bugzilla.gnome.org/show_bug.cgi?id=761761

commit 5059b9b8c99608cfd3321f96170e0c135457c041
Author: Olivier Crête <olivier.crete@collabora.com>
Date:   Wed Feb 15 15:09:06 2017 -0500

    splitmuxsink: Remove unused next_max_out_running_time
    
    https://bugzilla.gnome.org/show_bug.cgi?id=761761

commit c98d932fb869ab6e763972360b435830147a4414
Author: Olivier Crête <olivier.crete@collabora.com>
Date:   Wed Feb 15 15:07:32 2017 -0500

    splitmuxsink: Remove unused muxed_out_time
    
    https://bugzilla.gnome.org/show_bug.cgi?id=761761