From: "Rémi Bernon (@rbernon)" <wine@gitlab.winehq.org>
Subject: Re: [PATCH v4 0/5] MR302: winegstreamer: Some wg_transform H264 fixes for Mortal Kombat 11 and Yakuza 4.
Message-Id: <wine-wine-mr302-v4-note3001@gitlab-mail-bridge>
Date: Thu, 30 Jun 2022 08:05:42 +0000
In-Reply-To: <wine-wine-mr302-v4@gitlab-mail-bridge>
References: <merge_request_373@gitlab.winehq.org> <wine-wine-mr302-v4@gitlab-mail-bridge>

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.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/302#note_3001