[1/2] qcap: Distinguish interface and implementation pointer for VfwCapture output pin.

[Sebastian Lackner]

On 29.06.2015 09:09, Nikolay Sivov wrote:
> On 29.06.2015 9:08, Sebastian Lackner wrote:
>> -        BasePin *pin;
>> +        VfwPinImpl *pin;
>>
>>           TRACE("destroying everything\n");
>>           if (This->init)
>> @@ -226,10 +236,10 @@ static ULONG WINAPI VfwCapture_Release(IBaseFilter * iface)
>>                   qcap_driver_stop(This->driver_info, &This->filter.state);
>>               qcap_driver_destroy(This->driver_info);
>>           }
>> -        pin = (BasePin*) This->pOutputPin;
>> -        if (pin->pConnectedTo != NULL)
>> +        pin = impl_from_IPin(This->pOutputPin);
>> +        if (pin->pin.pin.pConnectedTo != NULL)
>>           {
>> -            IPin_Disconnect(pin->pConnectedTo);
>> +            IPin_Disconnect(pin->pin.pin.pConnectedTo);
> 
> This is not better than it is now, 'pOutputPin' is IPin* so you should be using public methods.

Thanks for the feedback. In this case pOutputPin is basically part of the same object,
not an arbitrary external interface. It is constructed with VfwPin_Construct ->
BaseOutputPin_Construct, and also holds a reference to the critical section of the parent
object for example, so there is no clear distinction between public/private interfaces
anyway.

Most other code parts which use BaseOutputPin_Construct store the result in something like:
(IPin **)&This->store_impl_pointer_here where the variable already has the corresponding
implementation type. Should we change it here too? Using only the public interface is not possible,
take a look at for example the following code part, where a private member is set from a
different interface:

--- snip ---
        This->driver_info = qcap_driver_init( This->pOutputPin,
               var.__VARIANT_NAME_1.__VARIANT_NAME_2.__VARIANT_NAME_3.ulVal );
        if (This->driver_info)
        {
            pin = impl_from_IPin(This->pOutputPin);
            pin->driver_info = This->driver_info; // <---
--- snip ---


> 
>>      if (IsEqualIID(riid, &IID_IUnknown) || IsEqualIID(riid, &IID_IPin))
>> -        *ppv = This;
>> +        *ppv = &This->pin.pin.IPin_iface;
> 
> This could as well say 'iface'.
> 
> 

Is there really a clear preference? I personally always preferred the long form so far,
to make it a bit more clear when many different interfaces are returned.