[MPlayer-dev-eng] [PATCH] Capture feature

[Reimar Döffinger]

On Mon, Sep 06, 2010 at 05:12:50PM +0200, Pásztor Szilárd wrote:
> +    case M_PROPERTY_GET:
> +        if (arg) {
> +	    *(int *)arg = !!mpctx->stream->capture_file;
> +	    return M_PROPERTY_OK;
> +	} else
> +            return M_PROPERTY_ERROR;

That's a wild mix of tabs and spaces, leading to inconsistent indentation
for anyone with a different tab length.

> +    case M_PROPERTY_SET:
> +        if (!arg) {
> +            ret = M_PROPERTY_ERROR;
> +	    break;
> +	}
> +	M_PROPERTY_CLAMP(prop, *(int *) arg);
> +	capturing = *(int *)arg;
> +	// fall through
> +    case M_PROPERTY_STEP_UP:
> +    case M_PROPERTY_STEP_DOWN:
> +	if (capturing < 0)
> +	    capturing = !mpctx->stream->capture_file;
> +
> +	if (capturing) {
> +	    if (capture_dump_name && !mpctx->stream->capture_file &&
> +		!(mpctx->stream->capture_file = fopen(capture_dump_name, "wb"))) {
> +		mp_msg(MSGT_GLOBAL, MSGL_ERR, "Error opening capture file: %s\n", strerror(errno));
> +		ret = M_PROPERTY_ERROR;
> +	    } else
> +		ret = M_PROPERTY_OK;
> +	} else {
> +	    if (mpctx->stream->capture_file) {
> +		fclose(mpctx->stream->capture_file);
> +		mpctx->stream->capture_file = NULL;
> +		ret = M_PROPERTY_OK;
> +	    }
> +	    else
> +		return M_PROPERTY_OK;
> +	}
> +    }
> +
> +    switch (ret) {
> +    case M_PROPERTY_ERROR:
> +	set_osd_msg(OSD_MSG_SPEED, 1, osd_duration, MSGTR_OSDCapturingFailure);
> +	break;
> +    case M_PROPERTY_OK:
> +	set_osd_msg(OSD_MSG_SPEED, 1, osd_duration, MSGTR_OSDCapturing,
> +	    mpctx->stream->capture_file ? MSGTR_Enabled : MSGTR_Disabled);
> +	break;
> +    default:
> +	break;
> +    }

I think this needs more thinking.
For example it prints "enabled" when you set from enabled to enabled but
not when it is set from "disabled" to "disabled".
I think something like below should be close to working and much less complex
(and also support M_PROPERTY_PRINT)

int capturing = !!mpctx->stream->capture_file;
int ret = m_property_flag(prop, action, arg, &capturing);
if (ret == M_PROPERTY_OK && capturing != !!mpctx->stream->capture_file) {
    if (capturing) {
        if (capture_dump_name)
            mpctx->stream->capture_file = fopen(....);
        if (!mpctx->stream->capture_file)
            ret = M_PROPERTY_ERROR;
    } else {
        fclose(mpctx->stream->capture_file);
        mpctx->stream->capture_file = NULL;
    }
<OSD handling as before, but I'd make the error message the "default" case
 - also you could use M_PROPERTY_PRINT to get the string, but that may be overkill>
}

> diff -NurpabB mplayer-export-2010-09-06/mplayer.c mplayer-export-2010-09-06-capture/mplayer.c
> --- mplayer-export-2010-09-06/mplayer.c	2010-09-04 21:24:34.000000000 +0200
> +++ mplayer-export-2010-09-06-capture/mplayer.c	2010-09-06 17:09:25.156193405 +0200
> @@ -4086,6 +4086,11 @@ goto_next_file:  // don't jump here afte
>  
>  mp_msg(MSGT_CPLAYER,MSGL_INFO,"\n");
>  
> +if (mpctx->stream && mpctx->stream->capture_file) {
> +    fclose(mpctx->stream->capture_file);
> +    mpctx->stream->capture_file = NULL;
> +}

Both approaches have issues, but I think this would be
nicer to do in free_stream.