[MPlayer-dev-eng] [PATCH] DeckLink video output

[Reimar Döffinger]

On Mon, Aug 17, 2009 at 10:22:58PM +0200, Georg Lippitsch wrote:
> +IDeckLinkOutput *decklink_out = NULL;
> +BMDTimeValue decklink_frame_duration = 0;
> +BMDTimeScale decklink_frame_timescale = 0;
> +int decklink_frame_count = 0;
> +static int preroll = 10;
> +static int num_buffered = 0;
> +static IDeckLinkVideoFrame **dlvf = NULL;
> +static IDeckLinkVideoOutputCallback *vCallback = NULL;
> +static uint32_t in_width = 0;
> +static uint32_t in_height = 0;
> +static uint32_t in_format = 0;
> +static int bytes_per_frame = 0;
> +static bool running = false;

As for audio, the initializations are pointless.

> +        switch (result)
> +        {
> +        case bmdOutputFrameDropped:
> +            mp_msg(MSGT_VO, MSGL_ERR, "[decklink] Frame dropped\n");
> +            break;

I suspect this might get really ugly trying to play e.g. 60 fps content
on a 50 fps display with vsync...

> +    while (dmi->Next(&dm) == S_OK)
> +    {
> +        if (width == dm->GetWidth() &&
> +            height == dm->GetHeight())
> +        {

You are really failing unless the resolution _exactly_ matches a video
mode?
That very much limits what this can play, not to mention that it does
not really handle aspect (correctly).
Not to mention that MPlayer is not supposed to switch video modes unless
-vm is used (not that I can find a concept of "current mode" in the
DeckLink API).

> +                dlvf = new IDeckLinkVideoFrame*[preroll];
> +                memset(dlvf, 0, preroll * sizeof(IDeckLinkVideoFrame*));

In C I'd recommend calloc. In C++ I guess it is all on the same level of
ugly...

> +static void reset_preroll(void)
> +{    
> +    decklink_out->StopScheduledPlayback(0, NULL, decklink_frame_timescale);
> +    
> +    if (running)
> +    {
> +        pthread_mutex_lock(&sleep_mutex);
> +        while (num_buffered > 0)
> +        {
> +            pthread_cond_wait(&sleep_cond, &sleep_mutex);
> +        }
> +        pthread_mutex_unlock(&sleep_mutex);
> +    }
> +    else
> +    {
> +        num_buffered = 0;
> +    }

You can get rid of the "else" and just set it unconditionally.

> +static int draw_slice(uint8_t *src[], int stride[], int w, int h,
> +                      int x, int y)
> +{
> +    return 0;
> +}

draw_slice is not optional, see DOCS/tech/libvo.txt

> +static int draw_frame(uint8_t *src[])

VOCTRL_DRAW_IMAGE is much preferable, removing the stride might cause an
extra memcpy.

> +{
> +    uint8_t *buf;
> +    dlvf[decklink_frame_count % preroll]->GetBytes((void**)&buf);
> +
> +    memcpy(buf, src[0], bytes_per_frame);

And implementing VOCTRL_GET_IMAGE should eliminate this memcpy.

> +static void uninit(void)
> +{
> +    reset_preroll();
> +
> +    delete vCallback;
> +
> +    if (dlvf)
> +    {
> +        for (int i=0; i<preroll; i++)
> +        {
> +            if (dlvf[i])
> +            {
> +                dlvf[i]->Release();
> +                dlvf[i] = NULL;
> +            }
> +        }
> +    }

What about dlvf itself?

> +    const opt_t subopts[] =
> +    {
> +        { "preroll", OPT_ARG_INT, &preroll, (opt_test_f)int_pos }
> +    };
> +
> +    subopt_parse(arg, subopts);
> +
> +    if (preroll < 3)
> +    {
> +        mp_msg(MSGT_VO, MSGL_ERR,
> +               "[decklink] Minimum preroll: 3\n");
> +        return -1;
> +    }

That's exactly what the opt_test_f function is supposed to do.