[MPlayer-dev-eng] [PATCH] MNG output support for MPlayer

[Stefan Schuermans]

Hi Diego and others,

Thanks for the review and the discussion.

Diego Biurrun schrieb:
> On Sat, May 07, 2011 at 01:01:31PM +0200, Stefan Schuermans wrote:
>> --- libvo/vo_mng.c	(Revision 0)
>> +++ libvo/vo_mng.c	(Revision 0)
>> +static vo_info_t info = {
>> +  "MNG driver",
>> +  "mng",
>> +  "Stefan Schuermans <stefan blinkenarea org>",
>> +  ""
>> +};
> 
> We use 4 spaces indentation for new files.

I've changed this (and most of your other points) in my new patch.

>> +void vomng_free(mng_ptr ptr, mng_size_t i_size)
>> +{
>> +  (void)i_size; /* ignored */
>> +  free(ptr);
>> +}
> 
> gcc has an attribute for unused function arguments.  I think we already
> use it in other places.

I've kept those (void) casts for now, as I saw that there is no common 
opinion on this list and I personally like this non-gcc-specific way 
better. However, I'd change that to __attribute__((unused)) if the 
majority prefers this.

>> +mng_bool vomng_writedata(mng_handle h_mng, mng_ptr p_buf,
>> +                         mng_uint32 i_size, mng_uint32 *i_written)
>> +{
>> +  FILE *h_file = (FILE *)mng_get_userdata(h_mng);
>> +  *i_written = fwrite(p_buf, 1, i_size, h_file);
>> +  return MNG_TRUE;
>> +}
> 
> This always returns true.

Actually, the example code in libmng documentation does it that way. I 
guess libmng figures out if everything was written by comparing 
*i_written and i_size internally.

>> +  /* return allocated compressed data */
>> +  *p_out_ptr = out_ptr;
>> +  *p_out_len = (unsigned int)out_len;
> 
> Such casts make me nervous.

I think this cast is safe, due to the behavior of the compress2 call. 
I've added comments as explanation.

Regards,
Stefan

-------------- next part --------------
A non-text attachment was scrubbed...
Name: MPlayer-svn20110509-vomng.diff
Type: text/x-patch
Size: 23829 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/mplayer-dev-eng/attachments/20110509/f4089c48/attachment-0001.bin>