[MPlayer-dev-eng] [PATCH] vfw fixes

[Gianluigi Tiesi]

On Sun, Sep 16, 2007 at 10:12:09AM +0200, Reimar D?ffinger wrote:
> Hello,
> On Sun, Sep 16, 2007 at 05:04:11AM +0200, Gianluigi Tiesi wrote:
> > xx_vfw_icclose.diff adds the proper close routine on the vfw driver
> > xx_vfw_coinit.diff windows only, calls coinitialize() and
> > couninitialize() that may be needed for some dlls
> > coinit is need for most dialogs of vfw codecs
> 
> So is this windows only because it is only needed there or because we
> don't have it on Linux?
because the win32 loader doesn't implement the calls
> 
> > +static void uninit(struct vf_instance_s* vf)
> > +{
> > +    HRESULT ret;
> > +
> > +    if(encoder_hic){
> > +	if(encoder_buf){
> > +	    ret=ICCompressEnd(encoder_hic);
> > +	    if(ret){
> > +		mp_msg(MSGT_WIN32, MSGL_WARN, "ICCompressEnd failed: %ld\n", ret);
> > +		/* return; */
> > +	    }
> > +	    free(encoder_buf);
> > +	    encoder_buf=NULL;
> > +	}
> > +	
> > +	ret=ICClose(encoder_hic);
> > +	if(ret){
> > +	    mp_msg(MSGT_WIN32, MSGL_WARN, "ICClose failed: %ld\n", ret);
> > +	    return;
> > +	}
> > +	encoder_hic=0;
> > +    }
> > +}
> 
> That ret variable is pointless. I see that the other code in the file
> enjoys using variables pointlessly but that's no reason to do it here as
> well.
> I also can't see how that commented-out return should ever make sense
> unless maybe you move the free() up, so please remove it.
> ve_vfw also uses almost no tabs, so please remove them from your
> new code.
> 
Ok It's not my patch, I promise to review by myself next submitted
patches :D, I was using it for a while, I've not rewieved it
in details, ret is needed to print the return code of the function,
if fails. I can cleanup it, but without the ret var, should
I remove the warning message?

Regards

-- 
Gianluigi Tiesi <sherpya at netfarm.it>
EDP Project Leader
Netfarm S.r.l. - http://www.netfarm.it/
Free Software: http://oss.netfarm.it/