[MPlayer-dev-eng] [PATCH] mencoder vfw codecs configuration data support

[Reimar Döffinger]

Hello,
On Tue, Oct 24, 2006 at 09:49:43AM +0200, Gianluigi Tiesi wrote:
> This patch adds a new option to mencoder when used with -ovc vfw
> like -xvfwopts codecs=vp6vfw.dll:compdata=settings.mcf
> 
> also using compdata=dialog, on win32 shows the codec
> configure dialog before starting the encoding.
> Attached also a small program that can be compiled
> with mingw, msvc2005 and with wingcc (yes it works also
> on linux) that can generate settings files by
> displaying the settings dialog.
> The patch follows the same code style of the original file.

Rather late reply, still...

>    HRESULT ret;
>    BITMAPINFOHEADER* output_bih=NULL;
>    int temp_len;
> +  FILE *fd=NULL;
> +  char *drvdata=NULL;
> +  struct stat st;

IMO move them into the block where they are used.

> +      if (stat(compdatafile, &st) < 0){
> +        mp_msg(MSGT_WIN32,MSGL_ERR,"Compressor data file not found!\n");
> +        return NULL;
> +      }

Useless clutter I'd say, if it says it can't be opened users will
hopefully be bright enough to check if it even exists...

> +      fd = fopen(compdatafile, "rb");
> +      if (!fd){
> +        mp_msg(MSGT_WIN32,MSGL_ERR,"Cannot open Compressor data file!\n");
> +        return NULL;
> +      }
> +      drvdata = (char *) malloc(st.st_size);
> +      if (fread(drvdata, st.st_size, 1, fd) != 1) {
> +        mp_msg(MSGT_WIN32,MSGL_ERR,"Cannot read Compressor data file!\n");
> +        fclose(fd);
> +        free(drvdata);
> +        return NULL;
> +      }
> +      fclose(fd);
> +      mp_msg(MSGT_WIN32,MSGL_ERR,"Compressor data %d bytes\n", st.st_size);

Maybe bad style but I would just set a fixed upper limit and malloc and
fread that and see how much we actually got...
Or if you prefer leaving it like this I'd say consider moving the file
reading into a separate function.

Greetings,
Reimar Döffinger