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

Ali KIZIL alikizil at gmail.com
Thu Aug 17 11:42:11 EEST 2017


2017-08-14 0:00 GMT+03:00 Marton Balint <cus at passwd.hu>:

>
> 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
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>

As general infomation, NDI v3 SDK is released at:
http://pages.newtek.com/NDI-Developers-SDK-Download-Link.html


More information about the ffmpeg-devel mailing list