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

[Clément Bœsch]

On Sun, May 15, 2011 at 09:31:54PM +0200, Stefan Schuermans wrote:
[...]
> As I'm sending a new version of the patch anyway, I also changed all
> standalone prefix increments to postfix increments.
> 

I just tested it and it seems to work, at least a file is generated.
Thought, I wasn't able to "play" it correctly… Maybe I'm missing
something, but since there is no documentation I'm a bit lost :)

If you can give me a hint on how to play it to show me it works, I'm OK
committing it. Currently I'm just able to have a static image with ffplay.

Thought, I'm attaching yet another notes on things I missed.

> +static vo_info_t info = {
> +    .name       = "MNG driver",

MNG file

> +    .short_name = "mng",
> +    .author     = "Stefan Schuermans <stefan blinkenarea org>"
> +};
> +
> +LIBVO_EXTERN(mng)
> +
> +/* a frame to be written to the MNG file */
> +struct vomng_frame {
> +    mng_ptr data;             /* deflate compressed data, malloc-ed */
> +    mng_uint32 len;           /* length of compressed data */
> +    unsigned int time_ms;     /* timestamp of frame (in ms) */
> +    struct vomng_frame *next; /* next frame */
> +};
> +

Can you use Doxygen comments instead?

///< deflate...

> +/* properties of MNG output */
> +struct vomng_properties {
> +    char               *out_file_name; /* name of output file, malloc-ed */

About the "malloc-ed", see below.

> +    unsigned int       width, height;  /* dimensions */
> +    unsigned char      *canvas;        /* canvas for frame
> +                                            canvas := row, ..., row
> +                                            row    := filter ID, pix, ..., pix
> +                                            pix    := red, green, blue */
> +    struct vomng_frame *frame_first;   /* list of frames */
> +    struct vomng_frame *frame_last;
> +    int                is_init;        /* if initialized */
> +};
> +
> +/* private data of MNG vo module */
> +static struct vomng_properties *vomng = NULL;
> +

Pointless init.

[...]

> +/**
> + * @brief set up properties of MNG vo module
> + * @return 0 on success, 1 on error
> + */
> +static int vomng_prop_setup(void)
> +{
> +    /* create properties structure */
> +    vomng = calloc(1, sizeof(struct vomng_properties));

sizeof(*vomng);

[...]

> +/**
> + * @brief pre-initialize MNG vo module
> + * @param[in] *arg arguments passed to MNG vo module (output file name)
> + * @return 0 on success, 1 on error
> + */
> +static int preinit(const char *arg)
> +{
> +    /* set up properties structure */
> +    if (vomng_prop_setup())
> +        return 1;
> +
> +    /* 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_prop_cleanup();
> +        return 1;
> +    }
> +    if (vomng->out_file_name)
> +        free(vomng->out_file_name);

Pointless if.

> +    vomng->out_file_name = strdup(arg);

Are you see you really need to reallocate the string?

Also, this is good practice to dynamically allocate a context but I wonder
if it was worst the effort. Anyway, doesn't matter much.

Maybe I'll port my pending mplayer2 patches for the images related libvo
so this code could be extended easily. For now, this looks good if you can
prove me it works (see beginning of the mail).

Regards,

-- 
Clément B.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 490 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/mplayer-dev-eng/attachments/20110515/f2bf5f1f/attachment-0001.asc>