[PATCH 3/3] winegstreamer: Implement IMFMediaStream::GetStreamDescriptor.

Zebediah Figura zfigura at codeweavers.com
Mon Sep 14 15:49:13 CDT 2020



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_source.c#L1133
> 
> 
> 

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://www.winehq.org/pipermail/wine-devel/attachments/20200914/483babf7/attachment.sig>


More information about the wine-devel mailing list