NextLevelSessionExporter icon indicating copy to clipboard operation
NextLevelSessionExporter copied to clipboard

Buffers in pool may not always point at the video compositors output

Open Developer-Ecosystem-Engineering opened this issue 3 years ago • 2 comments

We received a report of black frames in an applications video content once it was encoded with NextLevelSessionExporter.

Investigation revealed a similar issue as https://github.com/NextLevel/NextLevelSessionExporter/issues/38.

It appears NextLevelSessionExporter is pulling buffers from the pool that was created, and sending those buffers back to the API. The assumption a buffer pulled from that pool will contain the output of the Metal video compositor is not always correct.

This change will pass the pixelBuffer directly, or use the toBuffer if a renderHandler exists. It resolves black frames in video content in our testing, @GrandPoohBear you may want to give it a try.

@Developer-Ecosystem-Engineering thanks for the PR! could you explain how these code paths are different? when an optional statement is called, nothing happens, just as your code does. did you observe different results?

piemonte avatar Jun 03 '22 23:06 piemonte

Hi @piemonte!

@Developer-Ecosystem-Engineering thanks for the PR! could you explain how these code paths are different? when an optional statement is called, nothing happens, just as your code does. did you observe different results?

The difference is which buffer we are passing to pixelBufferAdaptor.append.

If the renderHandler is nil, the existing pixelBuffer should be passed to pixelBufferAdaptor.append. If the renderHandler is not nil, then the renderHandler needs to copy the contents of the pixelBuffer to the toBuffer.

Without this change, there is no default renderHandler, so the toBuffer is assumed to automatically contain the contents of the pixelBuffer, but that is not always the case. This usually works fine, except in some error cases where it doesn't!

The observable difference is sometimes video files re-encoded by NLSE had black frames prior to the change and now those same video files re-encode no longer black. Perhaps @GrandPoohBear could share one of their sample videos, we are unable to provide the reference video in this case.