gstreamer1.0-plugins-nvcompositor: update patches for 1.18 build

[kekiefer]

Work in progress. While the build works, tests against the plugin are not working.

[madisongh]

Don't worry about the autobuilder failure - the check shouldn't have triggered for a draft PR. Should clear up once you promote the PR to non-draft status.

[kekiefer]

FYR, the smoke test:

WIDTH=320
HEIGHT=240
CAPS="video/x-raw, width=$WIDTH, height=$HEIGHT"
POSITIONING="\
sink_0::xpos=0 sink_0::ypos=0 sink_0::width=$WIDTH sink_0::height=$HEIGHT \
sink_1::xpos=$WIDTH sink_1::ypos=0 sink_1::width=$WIDTH sink_1::height=$HEIGHT \
sink_2::xpos=0 sink_2::ypos=$HEIGHT sink_2::width=$WIDTH sink_2::height=$HEIGHT \
sink_3::xpos=$WIDTH sink_3::ypos=$HEIGHT sink_3::width=$WIDTH sink_3::height=$HEIGHT \
"

gst-launch-1.0 -v nvcompositor name=comp $POSITIONING ! fakesink \
videotestsrc is-live=true ! $CAPS ! nvvidconv ! comp. \
videotestsrc is-live=true ! $CAPS ! nvvidconv ! comp. \
videotestsrc is-live=true ! $CAPS ! nvvidconv ! comp. \
videotestsrc is-live=true ! $CAPS ! nvvidconv ! comp.

Fails with the following

WARNING: erroneous pipeline: no property "sink_0::xpos" in element "comp"

Removing the positioning stuff, it fails due to

NvDdkVicConfigure Failed
nvbuffer_composite Failed
ERROR: from element /GstPipeline:pipeline0/GstVideoTestSrc:videotestsrc0: Internal data stream error.
Got EOS from element "pipeline0".

[madisongh]

@kekiefer I took a stab at moving this further along, using the sources for the upstream gstreamer compositor plugin as a reference:
wip-update-for-gstreamer-1.16-patch.txt

Hopefully it's readable enough with all the #ifdefs I added to try and make it build-compatible for both 1.14 and 1.16. It clears up the problem with the properties with the addition of the child proxies. The memory pool that the nvcompositor plugin creates for the NvBuffers is the latest issue, since the NvBuffer size isn't the size for the image data and a sanity check elsewhere in gstreamer fails because it thinks the buffer in that pool is too small.

[madisongh]

@kekiefer I've tracked down the memory management issue and have patches on this branch, if you'd like to give them a try. The nvvidconv plugin also needed fixing; with both of those plugins udpated, your test pipeline ran without error.

[kekiefer]

Thanks for getting around to working on this. I'll make the time to review this today.

My initial reaction regarding nvvidconv is that a patch to it may not be required. Indeed, nvvidconv has worked fine with 1.18 for me for quite some time. Downstream consumers of nvbuffers do not expect the full buffer size, and they're the only ones that will map these buffers without conversion. Making the size bigger would incur a performance penalty if downstream elements attempt to map the entire buffer (which I think they do), and with large video streams is likely be non-negligible.

[madisongh]

My initial reaction regarding nvvidconv is that a patch to it may not be required.

Yeah, I wasn't 100% sure of it, but your test script was generating error messages (but not stopping the pipeline) because of the size mismatches in the memory pool configuration, so I went ahead with it.

Making the size bigger would incur a performance penalty if downstream elements attempt to map the entire buffer (which I think they do), and with large video streams is likely be non-negligible.

Maybe my read of the code was incorrect, but the pool looked like it was used only for dmabuf-backed NvBuffers, and the memory_map function provided in the allocator just retrieves the pointer. So I didn't think it would make a difference.

[kekiefer]

Maybe my read of the code was incorrect, but the pool looked like it was used only for dmabuf-backed NvBuffers, and the memory_map function provided in the allocator just retrieves the pointer. So I didn't think it would make a difference.

You're right about this. Looking back, I ran into this problem when I modified nvvidconv's allocator to subclass GstDmaBufAllocator. This class provided a memory_map_full implementation that was being called first and ran DMA_BUF_IOCTL_SYNC on the whole buffer, which was the problem there.

The changes to nvvidconv are working with existing code with no detectable regressions. Also I am able to get images out of the nvcompositor so that looks about as sane as any of these plugins are going to get as well.

I'll close this PR and you've got my thumbs up to merge the wip branch.