On 5/6/22 02:01, Rémi Bernon (@rbernon) wrote:
Actually, for that matter, why do we need to store the wg_format on the PE side? Can't we just store it on the unix side?
(Maybe even check it in the chain function instead of in read_data, and store it in a flag on the object with gst_mini_object_set_qdata(), and then that'd remove the need to allocate GstSample objects. I don't remember if there was another impetus for using those?)
I don't see what benefit it would have, we need to return the information about the new format to the client anyway so that it can update the properties it exposes to the MF caller.
Er, right, I forgot we still need to store the format anyway, so ignore that part :-)
Still, the first part seems reasonable, unless I'm missing something?
Or, frankly, we could make it output-only, and set the "format changed" flag entirely on the client side. The way things currently are, the logic is kind of split in two, and it feels awkward.
There are cases, like when the application calls ProcessMessage with BEGIN_STREAMING, where we want to reset the format and receive format change notification, again. Having the stream format stored on the MF transform side lets us do that by reseting it to its default, and trigger the format change again.
Sure, then the question becomes, can we move the entire logic to the client side? That would require a separate call to retrieve the type of the current sample, given the below (or alternatively some extra queuing on the client side, but that seems more complex than necessary), but structurally it feels a lot less awkward.
Call of Duty: Black Ops 3 depends on these notifications.
I'll try something, but that game, and the others using the H264 have been really sensitive to tiny behavior changes.
@@ -427,7 +462,18 @@ NTSTATUS wg_transform_read_data(void *args) return STATUS_SUCCESS; }
- if ((status = read_transform_output_data(transform->output_buffer, sample)))
- if (sample->format && (caps = gst_sample_get_caps(transform->output_sample)))
- {
wg_format_from_caps(&format, caps);if (!wg_format_compare(&format, sample->format)){*sample->format = format;params->result = MF_E_TRANSFORM_STREAM_CHANGE;return STATUS_SUCCESS;}- }
This looks wrong; aren't we dropping the sample data on the floor?
No? We're not releasing output_sample there.
Sorry, not dropping it on the floor exactly, but we're also not filling the buffer, whereas the mfplat code (and the tests) looks like it expects the buffer to be filled.
The tests check that the returned sample, on format change, is empty, which should be the case, and mf_destroy_wg_sample will convert the wg sample properties back to the MF side.
After a stream change you need, and can, call ProcessOutput once more, immediately, to get the sample data.
Okay, I missed that. That's a bizarre API design; I don't know why they'd send an empty sample instead of no sample at all...