GNOME Bugzilla – Bug 660255
[theoraenc] Fix handling of GstForceKeyUnit events
Last modified: 2012-12-13 19:12:40 UTC
Created attachment 197582 [details] [review] Fix handling of GstForceKeyUnitEvents This fixes how GstForceKeyUnit events are handled: * Reuse the upstream event instead of creating a new one, keeping all the fields from the original event * Don't call theora_enc_force_keyframe() from the sink_event function. It calls theora_enc_reset() which can also be called from a different thread in the setcaps function generating a segfault: GST_EVENT gstpad.c:5097:gst_pad_send_event:<encoder:sink> have event type custom-downstream ** ERROR:gsttheoraenc.c:478:theora_enc_reset: assertion failed: (enc->encoder != NULL)
Review of attachment 197582 [details] [review]: Looks good.
While we're at it this could also be changed to the new force-keyunit API in base
I don't think the new API could be used anywhere since events are reused. The only place where it could be used is in theora_enc_chain, but here we are reusing the upstream event, chanding its direction and replacing the timestamp values of the event's structure. Either we create a new downstream event with the upstream info or we add a new method to modify the timestamp fields in the existing event.
Either we shouldn't reuse events (how is the event direction reversed? Changing the event type requires that the event is writable and requires internal knowledge about the event) or we should have API to make this possible (e.g. functions to change the event and return a new one if it's not writable or functions to create a downstream event from an upstream event or ...).
The event is copied internally, so we have the unique ref on it and it's therefore writeable. The direction is changed and the structure's fields are changed too. But it's probably better have some API to create a downstream force-keyunit event from and upstream event, with the possibility to update the timestamps fields which is a common use case.
This is obsolete now, the theora encoder uses the video base classes now.