On 9/14/20 3:49 PM, Zebediah Figura wrote:
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.]
Oof, have you profiled this? I doubt that excessive caps copying would be the reason. Maybe the issue is with gstreamer?
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.
Maybe the function seems simpler to read for me than it actually is, since I wrote it. Would adding a better description comment `transform_to_media_type` help?
The fact that those tasks are (at least as of this patch) entirely orthogonal doesn't help either.
Can you elaborate what you mean by the tasks being orthogonal? As I said in my last email, the two functions based on `transform_to_media_type` are used hand-in-hand in the main parser path, as I linked in my last email. I do now realize though that `make_mf_compatible_caps` is dead code in this patch and should be moved up though.
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...