[PATCH v2 2/4] winegstreamer: Implement ::Set(Input/Output)Type for audio conversion transform.

[Derek Lesho]

On 12/1/20 3:10 PM, Zebediah Figura (she/her) wrote:
> This field is unused in this patch, but is there a reason its uses can't
> be replaced with "converter->input_type && converter->output_type"?
If the functionality of update_pipeline_state were moved to the 
validation stage of Set(Input/Output)Type, yes, that'd be possible. I'll 
do that.
>
>>   };
>>   
>>   static struct audio_converter *impl_audio_converter_from_IMFTransform(IMFTransform *iface)
>> @@ -63,6 +68,7 @@ static ULONG WINAPI audio_converter_Release(IMFTransform *iface)
>>   
>>       if (!refcount)
>>       {
>> +        DeleteCriticalSection(&transform->cs);
>>           heap_free(transform);
>>       }
>>   
>> @@ -252,18 +258,191 @@ static HRESULT WINAPI audio_converter_GetOutputAvailableType(IMFTransform *iface
>>       return hr;
>>   }
>>   
>> +static void audio_converter_update_pipeline_state(struct audio_converter *converter)
>> +{
>> +    GstCaps *input_caps, *output_caps;
>> +
>> +    if (!(converter->valid_state = converter->input_type && converter->output_type))
>> +        return;
>> +
>> +    if (!(input_caps = caps_from_mf_media_type(converter->input_type)))
>> +    {
>> +        converter->valid_state = FALSE;
>> +        return;
>> +    }
>> +    if (!(output_caps = caps_from_mf_media_type(converter->output_type)))
>> +    {
>> +        converter->valid_state = FALSE;
>> +        gst_caps_unref(input_caps);
>> +        return;
>> +    }
>> +
>> +    if (TRACE_ON(mfplat))
>> +    {
>> +        gchar *input_caps_str, *output_caps_str;
>> +
>> +        input_caps_str = gst_caps_to_string(input_caps);
>> +        output_caps_str = gst_caps_to_string(output_caps);
>> +
>> +        TRACE("Audio converter MFT configured to transform caps %s to caps %s\n",
>> +            debugstr_a(input_caps_str), debugstr_a(output_caps_str));
>> +
>> +        g_free(input_caps_str);
>> +        g_free(output_caps_str);
>> +    }
>> +
>> +    gst_caps_unref(input_caps);
>> +    gst_caps_unref(output_caps);
>> +
>> +    return;
>> +}
>> +
> This function doesn't really do anything but trace; I'm not sure it
> makes sense in this patch.
I agree, even in the later patches where it does more, it just does the 
same thing twice for both the input and output type.  It makes more 
sense to include it inline.+
>> +        if (!found)
>> +            return MF_E_INVALIDTYPE;
>> +    }
>> +
>> +    if (flags & MFT_SET_TYPE_TEST_ONLY)
>> +        return S_OK;
> Wait, so what happens if MFT_SET_TYPE_TEST_ONLY is used with a NULL type?
S_OK is returned, indicating that you can unset the input type.