CasparCG / server

CasparCG Server is a Windows and Linux software used to play out professional graphics, audio and video to multiple outputs. It has been in 24/7 broadcast production since 2006. Ready-to-use downloads are available under the Releases tab https://casparcg.com.
GNU General Public License v3.0
914 stars 269 forks source link

NDI Consumer Field Order Incorrect #1577

Closed silid closed 1 month ago

silid commented 1 month ago

Description

Add an option to the NDI consumer config to allow change of field order.

Solution suggestion

The flag would have the effect of reversing the output from this line:

https://github.com/CasparCG/server/blob/master/src/modules/newtek/consumer/newtek_ndi_consumer.cpp#L192

silid commented 1 month ago

Actually - this might be a bug and not need to be configurable.

Docs say that NDI is always top field first, but it looks like I'm receiving them bottom field first. https://docs.ndi.video/docs/sdk/frame-types#video-frames-ndilib_video_frame_v2_t

silid commented 1 month ago

I think this is backwards.

The header says that:

    // A fielded frame with the field 0 being on the even lines and field 1 being
    // on the odd lines.
    NDIlib_frame_format_type_interleaved = 0,

    // Individual fields.
    NDIlib_frame_format_type_field_0 = 2,
    NDIlib_frame_format_type_field_1 = 3,

If NDIlib_frame_format_type_field_0 is the even lines - then it is the lower field and should some second, and NDIlib_frame_format_type_field_1 is the upper field and should come first.

The consumer code initialises frame-no_ at 0 so I think the even frame nos are the upper field and the odd are the lower field.

But this conditional reverses that.

ndi_video_frame_.frame_format_type =
    (frame_no_ % 2 ? NDIlib_frame_format_type_field_1 : NDIlib_frame_format_type_field_0);

I think the fix will be to either initialise frame_no_ at 1 or to reverse the outcome of the conditional.

Julusian commented 1 month ago

I understand your logic, and it does make sense, but I'm not sure that there is a problem in this line. I wouldn't be surprised if that comment for the interleaved type is a mistake, I think calling them odd and even is a bit ambiguous as that depends on whether you count from 0 or 1.

To test this out, I have taken the NDIlib_Send_video.cpp sample, and NDIlib_Recv.cpp, and modified the send to do:

#include <cstdio>
#include <cstdlib>
#include <cstring>
#include <chrono>
#include <Processing.NDI.Lib.h>

#ifdef _WIN32
#ifdef _WIN64
#pragma comment(lib, "Processing.NDI.Lib.x64.lib")
#else // _WIN64
#pragma comment(lib, "Processing.NDI.Lib.x86.lib")
#endif // _WIN64
#endif // _WIN32

int main(int argc, char *argv[])
{
    // Not required, but "correct" (see the SDK documentation).
    if (!NDIlib_initialize())
        return 0;

    // We create the NDI sender
    NDIlib_send_instance_t pNDI_send = NDIlib_send_create();
    if (!pNDI_send)
        return 0;

    // We are going to create a 1920x1080 interlaced frame at 29.97Hz.
    NDIlib_video_frame_v2_t NDI_video_frame;
    NDI_video_frame.xres = 1920;
    NDI_video_frame.yres = 1080;
    NDI_video_frame.FourCC = NDIlib_FourCC_type_BGRX;
    NDI_video_frame.p_data = (uint8_t *)malloc(NDI_video_frame.xres * NDI_video_frame.yres * 4);

    // Run for one minute
    using namespace std::chrono;
    for (const auto start = high_resolution_clock::now(); high_resolution_clock::now() - start < minutes(5);)
    {
        // Get the current time
        const auto start_send = high_resolution_clock::now();

        // Send 200 frames
        for (int idx = 0; idx < 200; idx++)
        {
            // Fill in the buffer. It is likely that you would do something much smarter than this.
            memset((void *)NDI_video_frame.p_data, (idx % 2) ? 255 : 0, NDI_video_frame.xres * NDI_video_frame.yres * 4);

            NDI_video_frame.frame_format_type = (idx % 2 ? NDIlib_frame_format_type_field_0 : NDIlib_frame_format_type_field_1);
            // NDI_video_frame.frame_format_type = NDIlib_frame_format_type_interleaved;

            // We now submit the frame. Note that this call will be clocked so that we end up submitting at exactly 29.97fps.
            NDIlib_send_send_video_v2(pNDI_send, &NDI_video_frame);
        }

        // Just display something helpful
        printf("200 frames sent, at %1.2ffps\n", 200.0f / duration_cast<duration<float>>(high_resolution_clock::now() - start_send).count());
    }

    // Free the video frame
    free(NDI_video_frame.p_data);

    // Destroy the NDI sender
    NDIlib_send_destroy(pNDI_send);

    // Not required, but nice
    NDIlib_destroy();

    // Success
    return 0;
}

with the recieve doing

#include <cstdio>
#include <chrono>
#include <Processing.NDI.Lib.h>

#ifdef _WIN32
#ifdef _WIN64
#pragma comment(lib, "Processing.NDI.Lib.x64.lib")
#else // _WIN64
#pragma comment(lib, "Processing.NDI.Lib.x86.lib")
#endif // _WIN64
#endif // _WIN32

int main(int argc, char *argv[])
{
    // Not required, but "correct" (see the SDK documentation).
    if (!NDIlib_initialize())
        return 0;

    // Create a finder
    NDIlib_find_instance_t pNDI_find = NDIlib_find_create_v2();
    if (!pNDI_find)
        return 0;

    // Wait until there is one source
    uint32_t no_sources = 0;
    const NDIlib_source_t *p_sources = NULL;
    while (!no_sources)
    {
        // Wait until the sources on the network have changed
        printf("Looking for sources ...\n");
        NDIlib_find_wait_for_sources(pNDI_find, 1000 /* One second */);
        p_sources = NDIlib_find_get_current_sources(pNDI_find, &no_sources);
    }

    // We now have at least one source, so we create a receiver to look at it.
    NDIlib_recv_create_v3_t opts;
    opts.color_format = NDIlib_recv_color_format_RGBX_RGBA;

    NDIlib_recv_instance_t pNDI_recv = NDIlib_recv_create_v3(&opts);
    if (!pNDI_recv)
        return 0;

    // Connect to our sources
    NDIlib_recv_connect(pNDI_recv, p_sources + 0);

    // Destroy the NDI finder. We needed to have access to the pointers to p_sources[0]
    NDIlib_find_destroy(pNDI_find);

    // Run for one minute
    using namespace std::chrono;
    for (const auto start = high_resolution_clock::now(); high_resolution_clock::now() - start < minutes(5);)
    {
        // The descriptors
        NDIlib_video_frame_v2_t video_frame;
        NDIlib_audio_frame_v2_t audio_frame;

        switch (NDIlib_recv_capture_v2(pNDI_recv, &video_frame, &audio_frame, nullptr, 5000))
        {
        // No data
        case NDIlib_frame_type_none:
            printf("No data received.\n");
            break;

            // Video data
        case NDIlib_frame_type_video:
            printf("Video data received (%dx%d). %d (%d-%d)\n", video_frame.xres, video_frame.yres, video_frame.frame_format_type, video_frame.p_data[0], video_frame.p_data[1920 * 4]);
            NDIlib_recv_free_video_v2(pNDI_recv, &video_frame);
            break;

            // Audio data
        case NDIlib_frame_type_audio:
            printf("Audio data received (%d samples).\n", audio_frame.no_samples);
            NDIlib_recv_free_audio_v2(pNDI_recv, &audio_frame);
            break;
        }
    }

    // Destroy the receiver
    NDIlib_recv_destroy(pNDI_recv);

    // Not required, but nice
    NDIlib_destroy();

    // Finished
    return 0;
}

Running these, I am receiving interleaved frames, with the logging including 0-253, which matches how the sender is filling the first field of type field_0 with 0, and the field_1 with 255. Which unless I have made a mistake along the way, shows field_0 being the upper field

I don't have any other ideas on why you are seeing what you are. To clarify, are you using some other receiving library or somehow configuring the default sdk to be able to receive individual fields, or are you referring to the content looking like it is packed with the field order flipped?

silid commented 1 month ago

Hi @Julusian

Thanks for looking at this.

I've spent a while looking at this with different media and clients and agree - I think Caspar server is probably right - and my receiving software is mangling it.

I've switched to running the channel and ndi feed in progressive and allowing the receiver to convert to interlaced and that seems to work.