diff mbox

[FFmpeg-devel,1/2] avdevice/decklink_dec: use a custom memory allocator

Message ID 20180604202131.6529-1-cus@passwd.hu
State Accepted
Commit 643123b29d4072f27be9b348d03069b648dbddf9
Headers show

Commit Message

Marton Balint June 4, 2018, 8:21 p.m. UTC
The default memory allocator is limited in the max number of frames available,
and therefore caused frame drops if the frames were not freed fast enough.

Signed-off-by: Marton Balint <cus@passwd.hu>
---
 libavdevice/decklink_dec.cpp | 50 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 50 insertions(+)

Comments

Dave Rice June 5, 2018, 3:06 p.m. UTC | #1
> On Jun 4, 2018, at 4:21 PM, Marton Balint <cus@passwd.hu> wrote:
> 
> The default memory allocator is limited in the max number of frames available,
> and therefore caused frame drops if the frames were not freed fast enough.

I’ve been testing this patchset today. Yesterday I was occasionally getting “Decklink input buffer overrun!” errors with this command:

/usr/local/opt/ffmpegdecklink/bin/ffmpeg-dl -v info -nostdin -nostats -t 1980 -f decklink -draw_bars 0 -audio_input embedded -video_input sdi -format_code ntsc -channels 8 -raw_format yuv422p10 -i "UltraStudio Express" -metadata:s:v:0 encoder="FFV1 version 3" -color_primaries smpte170m -color_trc bt709 -colorspace smpte170m -color_range mpeg -metadata creation_time=now -f_strict unofficial -c:v ffv1 -level 3 -g 1 -slices 16 -slicecrc 1 -c:a pcm_s24le -filter_complex "[0:v:0]setfield=bff,setsar=40/27,setdar=4/3; [0:a:0]pan=stereo| c0=c0 | c1=c1[stereo1];[0:a:0]pan=stereo| c0=c2 | c1=c3[stereo2]" -map "[stereo1]" -map "[stereo2]" -f matroska output.mkv -an -f framemd5 output.framemd5

With the patchset applied, I haven’t had that buffer overrun error re-occur. 

> Signed-off-by: Marton Balint <cus@passwd.hu>
> ---
> libavdevice/decklink_dec.cpp | 50 ++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 50 insertions(+)
> 
> diff --git a/libavdevice/decklink_dec.cpp b/libavdevice/decklink_dec.cpp
> index 510637676c..897fca1003 100644
> --- a/libavdevice/decklink_dec.cpp
> +++ b/libavdevice/decklink_dec.cpp
> @@ -21,6 +21,9 @@
>  * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
>  */
> 
> +#include <atomic>
> +using std::atomic;
> +
> /* Include internal.h first to avoid conflict between winsock.h (used by
>  * DeckLink headers) and winsock2.h (used by libavformat) in MSVC++ builds */
> extern "C" {
> @@ -98,6 +101,44 @@ static VANCLineNumber vanc_line_numbers[] = {
>     {bmdModeUnknown, 0, -1, -1, -1}
> };
> 
> +class decklink_allocator : public IDeckLinkMemoryAllocator
> +{
> +public:
> +        decklink_allocator(): _refs(1) { }
> +        virtual ~decklink_allocator() { }
> +
> +        // IDeckLinkMemoryAllocator methods
> +        virtual HRESULT STDMETHODCALLTYPE AllocateBuffer(unsigned int bufferSize, void* *allocatedBuffer)
> +        {
> +            void *buf = av_malloc(bufferSize + AV_INPUT_BUFFER_PADDING_SIZE);
> +            if (!buf)
> +                return E_OUTOFMEMORY;
> +            *allocatedBuffer = buf;
> +            return S_OK;
> +        }
> +        virtual HRESULT STDMETHODCALLTYPE ReleaseBuffer(void* buffer)
> +        {
> +            av_free(buffer);
> +            return S_OK;
> +        }
> +        virtual HRESULT STDMETHODCALLTYPE Commit() { return S_OK; }
> +        virtual HRESULT STDMETHODCALLTYPE Decommit() { return S_OK; }
> +
> +        // IUnknown methods
> +        virtual HRESULT STDMETHODCALLTYPE QueryInterface(REFIID iid, LPVOID *ppv) { return E_NOINTERFACE; }
> +        virtual ULONG   STDMETHODCALLTYPE AddRef(void) { return ++_refs; }
> +        virtual ULONG   STDMETHODCALLTYPE Release(void)
> +        {
> +            int ret = --_refs;
> +            if (!ret)
> +                delete this;
> +            return ret;
> +        }
> +
> +private:
> +        std::atomic<int>  _refs;
> +};
> +
> extern "C" {
> static void decklink_object_free(void *opaque, uint8_t *data)
> {
> @@ -924,6 +965,7 @@ av_cold int ff_decklink_read_header(AVFormatContext *avctx)
> {
>     struct decklink_cctx *cctx = (struct decklink_cctx *)avctx->priv_data;
>     struct decklink_ctx *ctx;
> +    class decklink_allocator *allocator;
>     AVStream *st;
>     HRESULT result;
>     char fname[1024];
> @@ -1017,6 +1059,14 @@ av_cold int ff_decklink_read_header(AVFormatContext *avctx)
>     ctx->input_callback = new decklink_input_callback(avctx);
>     ctx->dli->SetCallback(ctx->input_callback);
> 
> +    allocator = new decklink_allocator();
> +    ret = (ctx->dli->SetVideoInputFrameMemoryAllocator(allocator) == S_OK ? 0 : AVERROR_EXTERNAL);
> +    allocator->Release();
> +    if (ret < 0) {
> +        av_log(avctx, AV_LOG_ERROR, "Cannot set custom memory allocator\n");
> +        goto error;
> +    }
> +
>     if (mode_num == 0 && !cctx->format_code) {
>         if (decklink_autodetect(cctx) < 0) {
>             av_log(avctx, AV_LOG_ERROR, "Cannot Autodetect input stream or No signal\n");
> -- 
> 2.16.3
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Marton Balint June 5, 2018, 5:17 p.m. UTC | #2
On Tue, 5 Jun 2018, Dave Rice wrote:

>
>> On Jun 4, 2018, at 4:21 PM, Marton Balint <cus@passwd.hu> wrote:
>>
>> The default memory allocator is limited in the max number of frames available,
>> and therefore caused frame drops if the frames were not freed fast enough.
>
> I’ve been testing this patchset today. Yesterday I was occasionally getting “Decklink input buffer overrun!” errors with this command:
>
> /usr/local/opt/ffmpegdecklink/bin/ffmpeg-dl -v info -nostdin -nostats -t 1980 -f decklink -draw_bars 0 -audio_input embedded -video_input sdi -format_code ntsc -channels 8 -raw_format yuv422p10 -i "UltraStudio Express" -metadata:s:v:0 encoder="FFV1 version 3" -color_primaries smpte170m -color_trc bt709 -colorspace smpte170m -color_range mpeg -metadata creation_time=now -f_strict unofficial -c:v ffv1 -level 3 -g 1 -slices 16 -slicecrc 1 -c:a pcm_s24le -filter_complex "[0:v:0]setfield=bff,setsar=40/27,setdar=4/3; [0:a:0]pan=stereo| c0=c0 | c1=c1[stereo1];[0:a:0]pan=stereo| c0=c2 | c1=c3[stereo2]" -map "[stereo1]" -map "[stereo2]" -f matroska output.mkv -an -f framemd5 output.framemd5
>
> With the patchset applied, I haven’t had that buffer overrun error re-occur.

That is very strange, it should work the opposite way. Without the patch, 
the decklink driver is dropping frames (silently), so you should never 
get a Decklink input buffer overrun error message, but silent frame drops 
instead if you don't release (transcode) the frames fast enough.

With the patch, you won't get silent frame drops, but you might fill the 
internal queue and therefore get Decklink input buffer overruns. On the 
other hand, if you get Decklink input buffer overruns, that typically 
means that your computer is too slow to handle transcoding in real time...

Regards,
Marton
Dave Rice June 5, 2018, 5:39 p.m. UTC | #3
> On Jun 5, 2018, at 1:17 PM, Marton Balint <cus@passwd.hu> wrote:
> 
> On Tue, 5 Jun 2018, Dave Rice wrote:
> 
>>> On Jun 4, 2018, at 4:21 PM, Marton Balint <cus@passwd.hu> wrote:
>>> 
>>> The default memory allocator is limited in the max number of frames available,
>>> and therefore caused frame drops if the frames were not freed fast enough.
>> 
>> I’ve been testing this patchset today. Yesterday I was occasionally getting “Decklink input buffer overrun!” errors with this command:
>> 
>> /usr/local/opt/ffmpegdecklink/bin/ffmpeg-dl -v info -nostdin -nostats -t 1980 -f decklink -draw_bars 0 -audio_input embedded -video_input sdi -format_code ntsc -channels 8 -raw_format yuv422p10 -i "UltraStudio Express" -metadata:s:v:0 encoder="FFV1 version 3" -color_primaries smpte170m -color_trc bt709 -colorspace smpte170m -color_range mpeg -metadata creation_time=now -f_strict unofficial -c:v ffv1 -level 3 -g 1 -slices 16 -slicecrc 1 -c:a pcm_s24le -filter_complex "[0:v:0]setfield=bff,setsar=40/27,setdar=4/3; [0:a:0]pan=stereo| c0=c0 | c1=c1[stereo1];[0:a:0]pan=stereo| c0=c2 | c1=c3[stereo2]" -map "[stereo1]" -map "[stereo2]" -f matroska output.mkv -an -f framemd5 output.framemd5
>> 
>> With the patchset applied, I haven’t had that buffer overrun error re-occur.
> 
> That is very strange, it should work the opposite way. Without the patch, the decklink driver is dropping frames (silently), so you should never get a Decklink input buffer overrun error message, but silent frame drops instead if you don't release (transcode) the frames fast enough.
> 
> With the patch, you won't get silent frame drops, but you might fill the internal queue and therefore get Decklink input buffer overruns. On the other hand, if you get Decklink input buffer overruns, that typically means that your computer is too slow to handle transcoding in real time…

Trying to detect unreported dropped frames is why I added the framemd5 output as a second output. After the command runs, I would use this command

grep -v "^#” output.framemd5 | awk -F',' '$2!=p+1{printf p+1"-"$2-1" "}{p=$2}'

to report the ranges of pts that weren’t incrementing the pts by 1 within the pts. I had presumed that getting a gap in the pts within the framemd5 was corresponding with the buffer overrun error in the terminal output. I’ve tested a few hours of recorded with your patch applied and haven’t gotten any pts discontinuity in the framemd5s yet.

Dave
Marton Balint June 12, 2018, 10:28 p.m. UTC | #4
On Tue, 5 Jun 2018, Dave Rice wrote:

>
>> On Jun 5, 2018, at 1:17 PM, Marton Balint <cus@passwd.hu> wrote:
>> 
>> On Tue, 5 Jun 2018, Dave Rice wrote:
>> 
>>>> On Jun 4, 2018, at 4:21 PM, Marton Balint <cus@passwd.hu> wrote:
>>>> 
>>>> The default memory allocator is limited in the max number of frames available,
>>>> and therefore caused frame drops if the frames were not freed fast enough.
>>> 
>>> I’ve been testing this patchset today. Yesterday I was occasionally getting “Decklink input buffer overrun!” errors with this command:
>>> 
>>> /usr/local/opt/ffmpegdecklink/bin/ffmpeg-dl -v info -nostdin -nostats -t 1980 -f decklink -draw_bars 0 -audio_input embedded -video_input sdi -format_code ntsc -channels 8 -raw_format yuv422p10 -i "UltraStudio Express" -metadata:s:v:0 encoder="FFV1 version 3" -color_primaries smpte170m -color_trc bt709 -colorspace smpte170m -color_range mpeg -metadata creation_time=now -f_strict unofficial -c:v ffv1 -level 3 -g 1 -slices 16 -slicecrc 1 -c:a pcm_s24le -filter_complex "[0:v:0]setfield=bff,setsar=40/27,setdar=4/3; [0:a:0]pan=stereo| c0=c0 | c1=c1[stereo1];[0:a:0]pan=stereo| c0=c2 | c1=c3[stereo2]" -map "[stereo1]" -map "[stereo2]" -f matroska output.mkv -an -f framemd5 output.framemd5
>>> 
>>> With the patchset applied, I haven’t had that buffer overrun error re-occur.
>> 
>> That is very strange, it should work the opposite way. Without the patch, the decklink driver is dropping frames (silently), so you should never get a Decklink input buffer overrun error message, but silent frame drops instead if you don't release (transcode) the frames fast enough.
>> 
>> With the patch, you won't get silent frame drops, but you might fill the internal queue and therefore get Decklink input buffer overruns. On the other hand, if you get Decklink input buffer overruns, that typically means that your computer is too slow to handle transcoding in real time…
>
> Trying to detect unreported dropped frames is why I added the framemd5 output as a second output. After the command runs, I would use this command
>
> grep -v "^#” output.framemd5 | awk -F',' '$2!=p+1{printf p+1"-"$2-1" "}{p=$2}'
>
> to report the ranges of pts that weren’t incrementing the pts by 1 within the pts. I had presumed that getting a gap in the pts within the framemd5 was corresponding with the buffer overrun error in the terminal output. I’ve tested a few hours of recorded with your patch applied and haven’t gotten any pts discontinuity in the framemd5s yet.

Pushed the series.

Regards,
Marton
diff mbox

Patch

diff --git a/libavdevice/decklink_dec.cpp b/libavdevice/decklink_dec.cpp
index 510637676c..897fca1003 100644
--- a/libavdevice/decklink_dec.cpp
+++ b/libavdevice/decklink_dec.cpp
@@ -21,6 +21,9 @@ 
  * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
  */
 
+#include <atomic>
+using std::atomic;
+
 /* Include internal.h first to avoid conflict between winsock.h (used by
  * DeckLink headers) and winsock2.h (used by libavformat) in MSVC++ builds */
 extern "C" {
@@ -98,6 +101,44 @@  static VANCLineNumber vanc_line_numbers[] = {
     {bmdModeUnknown, 0, -1, -1, -1}
 };
 
+class decklink_allocator : public IDeckLinkMemoryAllocator
+{
+public:
+        decklink_allocator(): _refs(1) { }
+        virtual ~decklink_allocator() { }
+
+        // IDeckLinkMemoryAllocator methods
+        virtual HRESULT STDMETHODCALLTYPE AllocateBuffer(unsigned int bufferSize, void* *allocatedBuffer)
+        {
+            void *buf = av_malloc(bufferSize + AV_INPUT_BUFFER_PADDING_SIZE);
+            if (!buf)
+                return E_OUTOFMEMORY;
+            *allocatedBuffer = buf;
+            return S_OK;
+        }
+        virtual HRESULT STDMETHODCALLTYPE ReleaseBuffer(void* buffer)
+        {
+            av_free(buffer);
+            return S_OK;
+        }
+        virtual HRESULT STDMETHODCALLTYPE Commit() { return S_OK; }
+        virtual HRESULT STDMETHODCALLTYPE Decommit() { return S_OK; }
+
+        // IUnknown methods
+        virtual HRESULT STDMETHODCALLTYPE QueryInterface(REFIID iid, LPVOID *ppv) { return E_NOINTERFACE; }
+        virtual ULONG   STDMETHODCALLTYPE AddRef(void) { return ++_refs; }
+        virtual ULONG   STDMETHODCALLTYPE Release(void)
+        {
+            int ret = --_refs;
+            if (!ret)
+                delete this;
+            return ret;
+        }
+
+private:
+        std::atomic<int>  _refs;
+};
+
 extern "C" {
 static void decklink_object_free(void *opaque, uint8_t *data)
 {
@@ -924,6 +965,7 @@  av_cold int ff_decklink_read_header(AVFormatContext *avctx)
 {
     struct decklink_cctx *cctx = (struct decklink_cctx *)avctx->priv_data;
     struct decklink_ctx *ctx;
+    class decklink_allocator *allocator;
     AVStream *st;
     HRESULT result;
     char fname[1024];
@@ -1017,6 +1059,14 @@  av_cold int ff_decklink_read_header(AVFormatContext *avctx)
     ctx->input_callback = new decklink_input_callback(avctx);
     ctx->dli->SetCallback(ctx->input_callback);
 
+    allocator = new decklink_allocator();
+    ret = (ctx->dli->SetVideoInputFrameMemoryAllocator(allocator) == S_OK ? 0 : AVERROR_EXTERNAL);
+    allocator->Release();
+    if (ret < 0) {
+        av_log(avctx, AV_LOG_ERROR, "Cannot set custom memory allocator\n");
+        goto error;
+    }
+
     if (mode_num == 0 && !cctx->format_code) {
         if (decklink_autodetect(cctx) < 0) {
             av_log(avctx, AV_LOG_ERROR, "Cannot Autodetect input stream or No signal\n");