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

[Clément Bœsch]

On Tue, May 10, 2011 at 09:36:15PM +0200, Stefan Schuermans wrote:
> [...]
> +static vo_info_t info = {
> +    "MNG driver",
> +    "mng",
> +    "Stefan Schuermans <stefan blinkenarea org>"
> +};
> +

Do you mind adding attribut specifiers?

> +/**
> + * @brief close all files and free all memory of MNG vo module
> + */
> +static void vomng_prop_reset(void)
> +{
> +    struct vomng_frame *p_frame, *p_next;
> +
> +    if (vomng) {
> +
> +        /* we are initialized properly */
> +        if (vomng->b_is_init) {
> +            /* write buffered frames to MNG file */
> +            if (vomng_write_file())
> +                mp_msg(MSGT_VO, MSGL_ERR,
> +                       "vomng: writing output file failed\n");
> +        }
> +
> +        /* reset state */
> +        vomng->b_is_init = 0;
> +        if (vomng->p_frame_first) {
> +            p_frame = vomng->p_frame_first;
> +            while (p_frame) {
> +                p_next = p_frame->p_next;
> +                if (p_frame->p_data)
> +                    free(p_frame->p_data);

The if is pointless, or wrong.

> +                free(p_frame);
> +                p_frame = p_next;
> +            }
> +            vomng->p_frame_first = NULL;
> +            vomng->p_frame_last  = NULL;
> +        }
> +        if (vomng->p_canvas) {
> +            free(vomng->p_canvas);
> +            vomng->p_canvas = NULL;
> +        }
> +        vomng->i_width  = 0;
> +        vomng->i_height = 0;
> +    }
> +}
> +
> +/**
> + * @brief close files, free memory and delete private data of MNG von module
> + */
> +static void vomng_prop_cleanup(void)
> +{
> +    if (vomng) {
> +        vomng_prop_reset();
> +        if (vomng->out_file_name)
> +            free(vomng->out_file_name);

Ditto.

> +        free(vomng);
> +        vomng = NULL;
> +    }
> +}
> +
> +/**
> + * @brief configure MNG vo module
> + * @param[in] width video width
> + * @param[in] height video height
> + * @param[in] d_width (unused)
> + * @param[in] d_height (unused)
> + * @param[in] flags (unused)
> + * @param[in] title (unused)
> + * @param[in] format video frame format
> + * @return 0 on success, 1 on error
> + */
> +static int config(uint32_t width, uint32_t height,
> +                  uint32_t d_width, uint32_t d_height,
> +                  uint32_t flags, char *title, uint32_t format)
> +{
> +    uint32_t row_stride, y;
> +

Are the uint32_t really mandatory for the width/height or unsigned would
be fine? Specifying specific may supposed you're going to use the that
exact size.

> +    /* reset state */
> +    vomng_prop_reset();
> +
> +    /* check format */
> +    if (format != IMGFMT_RGB24) {
> +        mp_msg(MSGT_VO, MSGL_ERR,
> +               "vomng: config with invalid format (!= IMGFMT_RGB24)\n");
> +        return 1;
> +    }
> +
> +    /* allocate canvas */
> +    vomng->i_width  = width;
> +    vomng->i_height = height;
> +    row_stride      = 1 + width * 3; /* rows contain filter IDs */
> +    vomng->p_canvas = calloc(height * row_stride, 1);
> +    if (!vomng->p_canvas) {
> +        free(vomng->p_canvas);
> +        vomng->p_canvas = NULL;
> +    }
> +    /* fill in filter IDs for rows */
> +    for (y = 0; y < height; ++y)
> +        *(vomng->p_canvas + row_stride * y) = MNG_FILTER_NONE;
> +
> +    /* we are initialized */
> +    vomng->b_is_init = 1;
> +

It took me quiet a few time to realize what the b meant… please avoid that
notation.

> +/**
> + * @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 == NULL || strlen(arg) == 0) {

if (!arg || !*arg)

Not much to say. The code looks pretty clean, but I didn't test it. I'm
not sure this would be useful for MPlayer, but I hope it may have some
benefit when you'll port it to FFmpeg/Libav ;)

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/20110510/d45d3667/attachment.asc>