[FFmpeg-devel] [PATCH] Implement NewTek NDI support

[Marton Balint]

On Tue, 8 Aug 2017, Maksym Veremeyenko wrote:

> [...]

>
> Check if *avahi* daemon is running and firewall turned off.

Thanks, it works now.

I have some final comments, after addressing those, I guess this can be 
applied, it's been discussed for quite a long time now.

[...]

> +static int ndi_set_video_packet(AVFormatContext *avctx, NDIlib_video_frame_t *v, AVPacket *pkt)
> +{
> +    int ret;
> +    struct NDIContext *ctx = avctx->priv_data;
> +
> +    ret = av_new_packet(pkt, v->yres * v->line_stride_in_bytes);
> +    if (ret < 0)
> +        return ret;

You are leaking the NDI video frame here on error. Maybe better to put
NDIlib_recv_free_video into the parent function.

> +
> +    pkt->dts = pkt->pts = av_rescale_q(v->timecode, NDI_TIME_BASE_Q, ctx->video_st->time_base);
> +    pkt->duration = av_rescale_q(1, (AVRational){v->frame_rate_D, v->frame_rate_N}, ctx->video_st->time_base);
> +
> +    av_log(avctx, AV_LOG_DEBUG, "%s: pkt->dts = pkt->pts = %"PRId64", duration=%"PRId64", timecode=%"PRId64"\n",
> +        __func__, pkt->dts, pkt->duration, v->timecode);
> +
> +    pkt->flags         |= AV_PKT_FLAG_KEY;
> +    pkt->stream_index   = ctx->video_st->index;
> +
> +    memcpy(pkt->data, v->p_data, pkt->size);
> +
> +    NDIlib_recv_free_video(ctx->recv, v);
> +
> +    return 0;
> +}
> +
> +static int ndi_set_audio_packet(AVFormatContext *avctx, NDIlib_audio_frame_t *a, AVPacket *pkt)
> +{
> +    int ret;
> +    struct NDIContext *ctx = avctx->priv_data;
> +
> +    NDIlib_audio_frame_interleaved_16s_t dst;
> +
> +    ret = av_new_packet(pkt, 2 * a->no_samples * a->no_channels);
> +    if (ret < 0)
> +        return ret;

Similar to the previous case, you are leaking the NDI audio frame here on 
error. Maybe better to put NDIlib_recv_free_audio into the parent 
function.

> +
> +    pkt->dts = pkt->pts = av_rescale_q(a->timecode, NDI_TIME_BASE_Q, ctx->audio_st->time_base);
> +    pkt->duration = av_rescale_q(1, (AVRational){a->no_samples, a->sample_rate}, ctx->audio_st->time_base);
> +
> +    av_log(avctx, AV_LOG_DEBUG, "%s: pkt->dts = pkt->pts = %"PRId64", duration=%"PRId64", timecode=%"PRId64"\n",
> +        __func__, pkt->dts, pkt->duration, a->timecode);
> +
> +    pkt->flags       |= AV_PKT_FLAG_KEY;
> +    pkt->stream_index = ctx->audio_st->index;
> +
> +    dst.reference_level = 0;
> +    dst.p_data = (short *)pkt->data;
> +    NDIlib_util_audio_to_interleaved_16s(a, &dst);
> +
> +    NDIlib_recv_free_audio(ctx->recv, a);
> +
> +    return 0;
> +}


[...]

> +static int ndi_create_video_stream(AVFormatContext *avctx, NDIlib_video_frame_t *v)
> +{
> +    AVStream *st;
> +    AVRational tmp;
> +    struct NDIContext *ctx = avctx->priv_data;
> +
> +    st = avformat_new_stream(avctx, NULL);
> +    if (!st) {
> +        av_log(avctx, AV_LOG_ERROR, "Cannot add video stream\n");
> +        return AVERROR(ENOMEM);
> +    }
> +
> +    st->time_base                   = NDI_TIME_BASE_Q;
> +    av_stream_set_r_frame_rate(st, av_make_q(v->frame_rate_N, v->frame_rate_D));
> +
> +    tmp = av_mul_q(av_d2q(v->picture_aspect_ratio, INT_MAX), (AVRational){v->yres, v->xres});
> +    av_reduce(&st->sample_aspect_ratio.num, &st->sample_aspect_ratio.den, tmp.num, tmp.den, 1000);
> +    st->codecpar->sample_aspect_ratio = st->sample_aspect_ratio;
> +
> +    st->codecpar->codec_type        = AVMEDIA_TYPE_VIDEO;
> +    st->codecpar->width             = v->xres;
> +    st->codecpar->height            = v->yres;
> +    st->codecpar->codec_id          = AV_CODEC_ID_RAWVIDEO;
> +    st->codecpar->bit_rate          = av_rescale(v->xres * v->yres * 16, v->frame_rate_N, v->frame_rate_D);
> +    st->codecpar->field_order       = v->frame_format_type == NDIlib_frame_format_type_progressive
> +        ? AV_FIELD_PROGRESSIVE : AV_FIELD_TT;
> +
> +    if (NDIlib_FourCC_type_UYVY == v->FourCC || NDIlib_FourCC_type_UYVA == v->FourCC) {

You should give the user a warning here in case of UYVA format, because you are
going to skip the alpha channel.

> +        st->codecpar->format        = AV_PIX_FMT_UYVY422;
> +        st->codecpar->codec_tag     = MKTAG('U', 'Y', 'V', 'Y');
> +    } else if (NDIlib_FourCC_type_BGRA == v->FourCC) {
> +        st->codecpar->format        = AV_PIX_FMT_BGRA;
> +        st->codecpar->codec_tag     = MKTAG('B', 'G', 'R', 'A');
> +    } else if (NDIlib_FourCC_type_BGRX == v->FourCC) {
> +        st->codecpar->format        = AV_PIX_FMT_BGR0;
> +        st->codecpar->codec_tag     = MKTAG('B', 'G', 'R', '0');
> +    } else if (NDIlib_FourCC_type_RGBA == v->FourCC) {
> +        st->codecpar->format        = AV_PIX_FMT_RGBA;
> +        st->codecpar->codec_tag     = MKTAG('R', 'G', 'B', 'A');
> +    } else if (NDIlib_FourCC_type_RGBX == v->FourCC) {
> +        st->codecpar->format        = AV_PIX_FMT_RGB0;
> +        st->codecpar->codec_tag     = MKTAG('R', 'G', 'B', '0');
> +    } else {
> +        av_log(avctx, AV_LOG_ERROR, "Unsupported video stream format, v->FourCC=%d\n", v->FourCC);
> +        return AVERROR(EINVAL);
> +    }
> +
> +    avpriv_set_pts_info(st, 64, 1, NDI_TIME_BASE);
> +
> +    ctx->video_st = st;
> +
> +    return 0;
> +}

[...]

> +static int ndi_read_packet(AVFormatContext *avctx, AVPacket *pkt)
> +{
> +    int ret = 0;
> +    struct NDIContext *ctx = avctx->priv_data;
> +
> +    while (1) {
> +        NDIlib_video_frame_t v;
> +        NDIlib_audio_frame_t a;
> +        NDIlib_metadata_frame_t m;
> +        NDIlib_frame_type_e t;
> +
> +        t = NDIlib_recv_capture(ctx->recv, &v, &a, &m, 40);
> +
> +        if (NDIlib_frame_type_video == t) {
> +            if (!ctx->video_st) {
> +                ret = ndi_create_video_stream(avctx, &v);
> +                if (ret < 0)
> +                    return ret;

You are leaking the video frame here on error.

> +            }
> +            ret = ndi_set_video_packet(avctx, &v, pkt);
> +            break;
> +        }
> +        else if (NDIlib_frame_type_audio == t) {
> +            if (!ctx->audio_st) {
> +                ret = ndi_create_audio_stream(avctx, &a);
> +                if (ret < 0)
> +                    return ret;

You are leaking the audio frame here on error.

> +            }
> +            ret = ndi_set_audio_packet(avctx, &a, pkt);
> +            break;
> +        }
> +        else if (NDIlib_frame_type_metadata == t)
> +            NDIlib_recv_free_metadata(ctx->recv, &m);
> +        else if (NDIlib_frame_type_none != t){
> +            av_log(avctx, AV_LOG_ERROR, "NDIlib_recv_capture returns %d\n", t);
> +        }
> +    };
> +
> +    return ret;
> +}
> +

[...]

> +static int ndi_write_trailer(AVFormatContext *avctx)
> +{
> +    struct NDIContext *ctx = avctx->priv_data;
> +
> +    if (ctx->ndi_send) {
> +        NDIlib_send_destroy(ctx->ndi_send);
> +
> +        if (ctx->last_avframe)
> +            av_frame_free(&ctx->last_avframe);

You might spare the 'if' here, av_frame_free handles the NULL pointer as well.

> +    }
> +
> +    if (ctx->video)
> +        av_freep(&ctx->video);

Same here.

> +
> +    if (ctx->audio)
> +        av_freep(&ctx->audio);

And here.

> +
> +    return 0;
> +}
> +

Regards,
Marton