[MPlayer-dev-eng] [PATCH] MNG default output filename

[Stefan Schuermans]

Clément Bœsch wrote on 11.06.2011 14:11:
> On Mon, May 30, 2011 at 08:23:43PM +0200, Stefan Schuermans wrote:
>> Index: libvo/vo_mng.c
>> ===================================================================
>> --- libvo/vo_mng.c	(Revision 33526)
>> +++ libvo/vo_mng.c	(Arbeitskopie)
>> [...]
>> +/** list of suboptions */
>> +static const opt_t subopts[] = {
>> +    {   .name = "output",
>> +        .type = OPT_ARG_MSTRZ,
>> +        .valp =&vomng.out_file_name },
>> +    {   .name = NULL }
>> +};
>> +
>
> Looks a bit overkill to specify attributes in this case, and style looks
> weird, but I'm ok with it. Please stick to what you see in the rest of the
> code next time.

I remember that someone requested those attributes for the vo_info_t 
structure although most other vo modules do not use them, so I thought 
to add them here, too. I changed that to the style of vo_gif89a.

>>   /**
>>    * @brief pre-initialize MNG vo module
>>    * @param[in] *arg arguments passed to MNG vo module (output file name)
>> @@ -573,19 +582,14 @@
>>    */
>>   static int preinit(const char *arg)
>>   {
>> -    /* get name of output file */
>> -    if (!arg || !*arg) {
>> -        mp_msg(MSGT_VO, MSGL_ERR, "vomng: MNG output file must be given,"
>> -                                  " example: -vo mng:output.mng\n");
>> +    vomng.out_file_name = strdup("output.mng");
>> +
>
> Why don't you alloc out_file_name in case it is NULL after the
> subopt_parse? It doesn't really matter since the buffer is free because of
> OPT_ARG_MSTRZ, but the code doesn't look obvious that way IMO.

Good point, this also saves a malloc/free in case the suboption is supplied.

> Also, please use "out.mng" just like vo gif.

Done.

>> +    if (subopt_parse(arg, subopts)) {
>> +        mp_msg(MSGT_VO, MSGL_ERR, "vomng: invalid MNG suboptions,"
>> +                                  " example: -vo mng:output=output.mng\n");
>
> Do you mind using the same output as vo_gif? "Example: mplayer ..."

I changed it to match gif.

> Looks good enough for me otherwise. I let Diego take over.

New patch for review by Diego is attached.

Best regards,
Stefan
-------------- next part --------------
A non-text attachment was scrubbed...
Name: MPlayer-svn20110611-mng-def.diff
Type: text/x-patch
Size: 2764 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/mplayer-dev-eng/attachments/20110611/ec26448d/attachment-0001.bin>