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

[Marton Balint]

On Thu, 3 Aug 2017, Maksym Veremeyenko wrote:

> On 30.07.2017 17:19, Marton Balint wrote:
> [...]
>>> Subject: [PATCH] lavd: implement NewTek NDI muxer/demuxer support

Hi,

About the warnings for the bottom field order and the negative linesize, I 
still think it is better to reject them (return an error), this way the 
user will be forced to fix the input, by changing the field order (e.g by 
using the fieldorder filter before passing video) or getting rid of the 
negative linesize.

To support negative linesizes, there is one more thing you might do: If the
source frame has a negative linesize, then instead of cloning, allocate a new
avframe with av_frame_alloc, set widht/height/format, use av_frame_get_buffer
to allocate a buffer, then use av_frame_copy to copy the data from the source
frame to the newly allocated one which should have a positive linesize.
A bit too much work for a rarely used feature, but if you are interested, it is
doable. You can generate a negative linesize source with the vflip filter.

I did some further tests, and I still cannot see my own ffmpeg generated
source, I am running this command:

./ffmpeg -nostdin -f libndi_newtek -wait_sources 10 -find_sources 1 -i none & ./ffmpeg -f lavfi -i mptestsrc,format=pix_fmts=uyvy422 -f libndi_newtek dummy

Even after 10 seconds, the first process finds 0 sources, the second process is
running happily. Does this work for you?

I also have some additional comments for the patch:

[..]

> + at subsection Examples
> +
> + at itemize
> +
> + at item
> +Play video clip:
> + at example
> +ffmpeg -i "udp://@239.1.1.1:10480?fifo_size=1000000&overrun_nonfatal=1" -vf "scale=720:576,fps=fps=25,setdar=dar=16/9,format=pix_fmts=uyvy422" -f libndi_newtek NEW_NDI1

@ needs escaping into @@

[..]

> +    while (1)
> +    {
> +        int f, t = ctx->wait_sources / 1000;
> +        av_log(avctx, AV_LOG_DEBUG, "Waiting for sources %d miliseconds\n", t);
> +        f = NDIlib_find_wait_for_sources(ctx->ndi_find, t);
> +        av_log(avctx, AV_LOG_DEBUG, "NDIlib_find_wait_for_sources returns %d\n", f);
> +        if (!f)
> +            break;
> +    };

You should not loop indefinitely, give up after a few tries.

[..]

> +static int ndi_write_video_packet(AVFormatContext *avctx, AVStream *st, AVPacket *pkt)
> +{
> +    struct NDIContext *ctx = avctx->priv_data;
> +    AVFrame *avframe, *tmp = (AVFrame *)pkt->data;
> +
> +    if (tmp->format != AV_PIX_FMT_UYVY422 ||
> +        tmp->width  != ctx->video->xres ||
> +        tmp->height != ctx->video->yres) {
> +        av_log(avctx, AV_LOG_ERROR, "Got a frame with invalid pixel format or dimension.\n");
> +        av_log(avctx, AV_LOG_ERROR, "tmp->width=%d, tmp->height=%d, ctx->video->xres=%d, ctx->video->yres=%d\n", tmp->width, tmp->height, ctx->video->xres, ctx->video->yres);
> +        return AVERROR(EINVAL);
> +    }
> +    avframe = av_frame_clone(tmp);
> +    if (!avframe) {
> +        av_log(avctx, AV_LOG_ERROR, "Could not clone video frame.\n");

For ENOMEM we typically don't use an error message.

[..]


> +static int ndi_write_header(AVFormatContext *avctx)
> +{
> +    int ret = 0;
> +    unsigned int n;
> +    struct NDIContext *ctx = avctx->priv_data;
> +    const NDIlib_send_create_t ndi_send_desc = { .p_ndi_name = avctx->filename,
> +        .p_groups = NULL, .clock_video = false, .clock_audio = false };

Setting both clock_video and clock_audio to false is intentional? Maybe better
to clock based on the stream types you get? (E.g. clock to video if you have a
video stream, clock to audio otherwise?)

Regards,
Marton