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.