On 9/14/20 2:28 PM, Derek Lesho wrote:
On 9/10/20 7:54 PM, Zebediah Figura wrote:
It's not really idiomatic code structure. More concretely, it also means you're copying the caps when you don't need to.
This doesn't strike me as a concern during source initialization. We won't be calling this function while media is playing.
Sure, though startup time shouldn't exactly be ignored, either. [For instance, quartz + gstreamer currently can take multiple hundreds of milliseconds to start up, which is actually kind of abhorrent.]
My concern is more about idiomatic structure than anything else, in any case. The function, for instance, is doing multiple things at once, and it's not how I'd expect a conversion function to behave. Wrapping it in a different function helps abstract that away to a more idiomatic interface, but the implementation remains harder to read simply because it's not (maximally) modularized. The fact that those tasks are (at least as of this patch) entirely orthogonal doesn't help either.
If you really need to do something like this, you probably want a separate function that takes a GstCaps and returns a new GstCaps that is compatible with mfplat, then you feed the output of that through mf_media_type_from_caps().
This is what I currently do[1], it's just that internally I prefer to wrap this into a single function, to keep the conversion code and the correction code next to eachother.
I'm about to send v2, and everything but this comment should be addressed.
1: https://github.com/Guy1524/wine/blob/mfplat_cleanup/dlls/winegstreamer/media...