[PATCH 1/2] winegstreamer: Add helper for GstCaps <-> IMFMediaType conversion.

[Derek Lesho]

On 3/26/20 4:22 PM, Zebediah Figura wrote:

> I wouldn't expect the purpose of a function called
> "media_type_from_caps()" to be to find MF-compatible GstCaps from input
> GstCaps.
>
> Even if you were to rename the function, I don't see the benefit of
> performing both tasks in a single function. As I see it, you could just
> as easily have two functions, along the lines of:
>
> static GstCaps *make_compatible_caps(const GstCaps *source_caps)
> {
>      GstCaps *caps = gst_caps_copy(source_caps);
>      /* ... */
>
>      if (!strcmp(type, "video/x-h264"))
>      {
>          /* Media Foundation does not support unparsed h264. */
>          gst_structure_set(structure, "parsed", GST_TYPE_BOOLEAN, TRUE,
> NULL);
>      }
>      return caps;
> }
>
> /* Returns NULL if the type cannot be converted to caps. */
> static IMFMediaType *media_type_from_caps(const GstCaps *caps)
> {
>      /* ... */
> }
Okay, yeah this solution is probably cleaner, I'll transition to it.  
FWIW though, some of your points against the current solution don't make 
sense to me:
>
> Besides being clearer to read, as I see it this:
>
> * allows you to use media_type_from_caps() in other places;
With the current solution, if you want to see whether caps match 
perfectly with a media type, you use gst_caps_is_equal, as we do in the 
media_source.
>
> * allows you to support advertising multiple types more easily (which I
> believe mfplat supports in general),
This is unrelated, we don't use this function for our decoder 
transform.  In that case, we do derive the desired caps from a MF 
subtype, using gst_caps_from_mf_media_type.  In the case of a media 
source, where we use this caps->media type route, there should only be 
one supported type, since it's compressed.
>
> * if, like quartz, we ever want to derive the caps from an IMFMediaType
> instead of from the source caps, you then don't have to change
> media_type_from_caps() at all.
Dido, that's what gst_caps_from_mf_media_type is for.