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.