30 Jun
2022
30 Jun
'22
8:05 a.m.
On Fri Jun 24 06:22:15 2022 +0000, **** wrote:
Zebediah Figura replied on the mailing list:
On 6/23/22 12:12, Rémi Bernon wrote: > @@ -291,6 +297,7 @@ enum unix_funcs > > unix_wg_transform_create, > unix_wg_transform_destroy, > + unix_wg_transform_set_format, > > unix_wg_transform_push_data, > unix_wg_transform_read_data, Perhaps set_output_format? Not that we're likely to need a set_input_format counterpart... > +NTSTATUS wg_transform_set_format(void *args) > +{ > + struct wg_transform_set_format_params *params = args; > + struct wg_transform *transform = params->transform; > + GstSample *sample; > + GstEvent *event; > + GstCaps *caps; > + gchar *str; > + > + if (!(caps = wg_format_to_caps(params->format))) > + { > + GST_ERROR("Failed to convert format to caps."); > + return STATUS_UNSUCCESSFUL; > + } > + > + if (gst_caps_is_always_compatible(transform->output_caps, caps)) > + { > + gst_caps_unref(caps); > + return STATUS_SUCCESS; > + } > + > + gst_caps_unref(transform->output_caps); > + transform->output_caps = caps; This isn't thread-safe; a simultaneous GST_EVENT_CAPS can modify the output caps. > + > + if (!gst_pad_set_caps(transform->my_sink, caps) The only effect of gst_pad_set_caps() is to send a CAPS event to the sink, but you've already set transform->output_caps above. Note also that the event won't be serialized, which begs the question—should it be? I suspect the answer is, "well, we actually need buffers already sent to be re-decoded in the new format, which GStreamer doesn't support". Which would be a helpful thing to write in a comment if so. Since we do flush output samples below, I suspect we should explicitly try to flush out samples which haven't been received yet before doing anything else in this function, which neatly solves all of the thread safety problems as well. I think the correct way to do that is with flush-start + flush-stop; a DRAIN query might work but I'm not sure if it's guaranteed to force decoders to stop waiting for more data before sending what they have.
I've left this part aside for a later MR instead.