GNOME Bugzilla – Bug 774049
aggregator, videoaggregator: force waiting even in live mode and other features
Last modified: 2018-11-03 13:56:31 UTC
In the recent Gstreamer conference (2016) I have presented a talk about using Gstreamer pipelines for image processing in industrial applications. In this talk I mentioned that I had the need of muxing some metadata generated by processing (and thus modifying) image frames into the original (unmodified) images. For this purpose I have created my own muxing element (using the Compositor as inspiration) which uses the VideoAggregator as the base class. However the VideoAggreagtor and it's base class the Aggregator didn't quite cover my needs so I ended up modifying my own local copies of these 2 classes. During the conference there was interest expressed in hearing which where my different use cases because maybe they could be added upstream. Since I want to mux what is actually the same original frame (just divided and processed differently by 2 separate tee branches) I want to do timestamp matching of the buffers (I don't care about maintaining the fps and don't want to duplicate frames): -Therefore I want to produce output frames only when I have received input frames in all pads (in contrast to the Aggregator producing output as soon as 1 pad has data, in case of a live source which is what I have). My easy solution for this was adding a new property to the Aggregator that forces it to work as with non-live sources (so that it requires data in all pads). -Then I added to VideoAggregator a property that defines if it should work "normally" (producing frames in a deadline, fps) or if it should check the buffer timestamps and only produce frames when they match. I have created a function like the fill_queues() function which does the timestamp checking. -In case I'm only producing frames based on matched timestamps I have problems with QOS because the segment times might not match anymore (since frames are not necessarily produced regularly anymore). To solve this I have created a function that adjusts the segment times based on the produced frames. -Lastly, in my "Compositor like" Muxer element I needed the option to drop frames in some cases so I have added a new GstFlowReturn to VideoAggregator (GST_FLOW_VIDEO_AGGREGATOR_DROP_FRAME == GST_FLOW_CUSTOM_SUCCESS) just like it exists for the BaseTransform
Created attachment 339239 [details] [review] patch to Aggregator patch to Aggregator which adds to option to always operate as with non-live sources (requires data in all pads)
Created attachment 339240 [details] [review] patch to VideoAggregator patch to VideoAggregator which adds the option to only generate frames when the timestamps of the input buffers match. Also adjusts the segment as needed (based on generated frames). Adds a GstFlowReturn DROP_FRAME
Review of attachment 339240 [details] [review]: ::: gst-libs/gst/video/gstvideoaggregator.c @@ +1310,3 @@ + int buffer_nr = 0; + + //check the timestamp delay No // comments, please do /* ... */ @@ +1312,3 @@ + //check the timestamp delay + GstClockTime timeNow = + gst_util_get_timestamp () - GST_ELEMENT (vagg)->base_time; you need to hold the object lock to get the base_time.. Also you can't compare gst_util_get_timestamp() with it. I'd just drop that line entirely. @@ +1329,3 @@ + if (buf) { + buffer_timestamps[buffer_nr] = GST_BUFFER_TIMESTAMP (buf); + if (buffer_timestamps[buffer_nr] == -1) { Please use GST_CLOCK_TIME_NONE instead of -1 @@ +1333,3 @@ + GST_DEBUG_OBJECT (pad, "Need timestamped buffers!"); + GST_OBJECT_UNLOCK (vagg); + return GST_FLOW_ERROR; if you return GST_FLOW_ERROR, you also need ot post an error with GST_ELEMENT_ERROR() so that application can know why. @@ +2304,3 @@ + g_param_spec_boolean ("check-timestamps", "Check Timestamps", + "Checks that the timestamps from all sink pads match. Ignores the stream fps, will only output a new frame when they all match (won't duplicate any buffers).", + FALSE, G_PARAM_READWRITE | G_PARAM_STATIC_STRINGS)); My impression is that this really is specific to the subclass and not to the application? So it should probably be a regular setter function instead of a property. Name is also not so clear, maybe something like "only-equal-timestamps" ?
Review of attachment 339239 [details] [review]: I think the option name should be more explicit as to what it does.. Something like "no-timeout" or "wait-forever"
-- 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-bad/issues/437.