diff mbox series

[FFmpeg-devel,v7] avcodec/nvenc: write out user data unregistered SEI

Message ID 20210525111635.29482-2-bradh@frogmouth.net
State Accepted
Headers show
Series [FFmpeg-devel,v7] avcodec/nvenc: write out user data unregistered SEI | expand

Checks

Context Check Description
andriy/x86_make success Make finished
andriy/x86_make_fate success Make fate finished
andriy/PPC64_make success Make finished
andriy/PPC64_make_fate success Make fate finished

Commit Message

Brad Hards May 25, 2021, 11:16 a.m. UTC
Signed-off-by: Brad Hards <bradh@frogmouth.net>
---
 libavcodec/nvenc.c | 71 +++++++++++++++++++++++++++++++++++++---------
 libavcodec/nvenc.h |  2 ++
 2 files changed, 60 insertions(+), 13 deletions(-)

Comments

Brad Hards June 1, 2021, 9:51 a.m. UTC | #1
On Tuesday, 25 May 2021 9:16:35 PM AEST Brad Hards wrote:
> Signed-off-by: Brad Hards <bradh@frogmouth.net>
> ---
>  libavcodec/nvenc.c | 71 +++++++++++++++++++++++++++++++++++++---------
>  libavcodec/nvenc.h |  2 ++
>  2 files changed, 60 insertions(+), 13 deletions(-)
> 
> diff --git a/libavcodec/nvenc.c b/libavcodec/nvenc.c
> index 0dcd93a99c..8ca5cab4c8 100644
> --- a/libavcodec/nvenc.c
> +++ b/libavcodec/nvenc.c
> @@ -1610,6 +1610,8 @@ av_cold int ff_nvenc_encode_close(AVCodecContext
> *avctx)
> 
>      av_frame_free(&ctx->frame);
> 
> +    av_freep(&ctx->sei_data);
> +
>      if (ctx->nvencoder) {
>          p_nvenc->nvEncDestroyEncoder(ctx->nvencoder);
> 
> @@ -2170,9 +2172,9 @@ static int nvenc_send_frame(AVCodecContext *avctx,
> const AVFrame *frame) NVENCSTATUS nv_status;
>      NvencSurface *tmp_out_surf, *in_surf;
>      int res, res2;
> -    NV_ENC_SEI_PAYLOAD sei_data[8];
>      int sei_count = 0;
>      int i;
> +    int total_unregistered_sei = 0;
> 
>      NvencContext *ctx = avctx->priv_data;
>      NvencDynLoadFunctions *dl_fn = &ctx->nvenc_dload_funcs;
> @@ -2185,6 +2187,8 @@ static int nvenc_send_frame(AVCodecContext *avctx,
> const AVFrame *frame) return AVERROR(EINVAL);
> 
>      if (frame && frame->buf[0]) {
> +        void *a53_data = NULL;
> +        void *tc_data = NULL;
>          in_surf = get_free_frame(ctx);
>          if (!in_surf)
>              return AVERROR(EAGAIN);
> @@ -2230,7 +2234,6 @@ static int nvenc_send_frame(AVCodecContext *avctx,
> const AVFrame *frame) pic_params.inputTimeStamp = frame->pts;
> 
>          if (ctx->a53_cc && av_frame_get_side_data(frame,
> AV_FRAME_DATA_A53_CC)) { -            void *a53_data = NULL;
>              size_t a53_size = 0;
> 
>              if (ff_alloc_a53_sei(frame, 0, (void**)&a53_data, &a53_size) <
> 0) { @@ -2238,15 +2241,22 @@ static int nvenc_send_frame(AVCodecContext
> *avctx, const AVFrame *frame) }
> 
>              if (a53_data) {
> -                sei_data[sei_count].payloadSize = (uint32_t)a53_size;
> -                sei_data[sei_count].payloadType = 4;
> -                sei_data[sei_count].payload = (uint8_t*)a53_data;
> -                sei_count ++;
> +                void *tmp = av_fast_realloc(ctx->sei_data,
> +                                            &ctx->sei_data_size,
> +                                            ++sei_count *
> sizeof(NV_ENC_SEI_PAYLOAD)); +                if (!tmp) {
> +                    av_free(a53_data);
> +                    return AVERROR(ENOMEM);
> +                } else {
> +                    ctx->sei_data = tmp;
> +                    ctx->sei_data[sei_count-1].payloadSize =
> (uint32_t)a53_size; +                   
> ctx->sei_data[sei_count-1].payloadType = 4;
> +                    ctx->sei_data[sei_count-1].payload =
> (uint8_t*)a53_data; +                }
>              }
>          }
> 
>          if (ctx->s12m_tc && av_frame_get_side_data(frame,
> AV_FRAME_DATA_S12M_TIMECODE)) { -            void *tc_data = NULL;
>              size_t tc_size = 0;
> 
>              if (ff_alloc_timecode_sei(frame, avctx->framerate, 0,
> (void**)&tc_data, &tc_size) < 0) { @@ -2254,14 +2264,49 @@ static int
> nvenc_send_frame(AVCodecContext *avctx, const AVFrame *frame) }
> 
>              if (tc_data) {
> -                sei_data[sei_count].payloadSize = (uint32_t)tc_size;
> -                sei_data[sei_count].payloadType = SEI_TYPE_TIME_CODE;
> -                sei_data[sei_count].payload = (uint8_t*)tc_data;
> -                sei_count ++;
> +                void *tmp = av_fast_realloc(ctx->sei_data,
> +                                            &ctx->sei_data_size,
> +                                            ++sei_count *
> sizeof(NV_ENC_SEI_PAYLOAD)); +                if (!tmp) {
> +                    av_free(a53_data);
> +                    av_free(tc_data);
> +                    return AVERROR(ENOMEM);
> +                } else {
> +                    ctx->sei_data = tmp;
> +                    ctx->sei_data[sei_count-1].payloadSize =
> (uint32_t)tc_size; +                   
> ctx->sei_data[sei_count-1].payloadType = SEI_TYPE_TIME_CODE; +             
>       ctx->sei_data[sei_count-1].payload = (uint8_t*)tc_data; +            
>    }
> +            }
> +        }
> +
> +        for (int j = 0; j < frame->nb_side_data; j++)
> +            if (frame->side_data[j]->type ==
> AV_FRAME_DATA_SEI_UNREGISTERED) +                total_unregistered_sei++;
> +        if (total_unregistered_sei > 0) {
> +            void *tmp = av_fast_realloc(ctx->sei_data,
> +                                        &(ctx->sei_data_size),
> +                                        (sei_count +
> total_unregistered_sei) * sizeof(NV_ENC_SEI_PAYLOAD)); +            if
> (!tmp) {
> +                av_free(a53_data);
> +                av_free(tc_data);
> +                return AVERROR(ENOMEM);
> +            } else {
> +                ctx->sei_data = tmp;
> +                for (int j = 0; j < frame->nb_side_data; j++) {
> +                    AVFrameSideData *side_data = frame->side_data[j];
> +                    if (side_data->type == AV_FRAME_DATA_SEI_UNREGISTERED)
> { +                        ctx->sei_data[sei_count].payloadSize =
> side_data->size; +                       
> ctx->sei_data[sei_count].payloadType = SEI_TYPE_USER_DATA_UNREGISTERED; +  
>                      ctx->sei_data[sei_count].payload =
> av_memdup(side_data->data, side_data->size); +                        if
> (ctx->sei_data[sei_count].payload)
> +                            sei_count ++;
> +                    }
> +                }
>              }
>          }
> 
> -        nvenc_codec_specific_pic_params(avctx, &pic_params, sei_data,
> sei_count); +        nvenc_codec_specific_pic_params(avctx, &pic_params,
> ctx->sei_data, sei_count); } else {
>          pic_params.encodePicFlags = NV_ENC_PIC_FLAG_EOS;
>      }
> @@ -2273,7 +2318,7 @@ static int nvenc_send_frame(AVCodecContext *avctx,
> const AVFrame *frame) nv_status =
> p_nvenc->nvEncEncodePicture(ctx->nvencoder, &pic_params);
> 
>      for ( i = 0; i < sei_count; i++)
> -        av_freep(&sei_data[i].payload);
> +        av_freep(&ctx->sei_data[i].payload);
> 
>      res = nvenc_pop_context(avctx);
>      if (res < 0)
> diff --git a/libavcodec/nvenc.h b/libavcodec/nvenc.h
> index 314c270e74..d28f02b54f 100644
> --- a/libavcodec/nvenc.h
> +++ b/libavcodec/nvenc.h
> @@ -165,6 +165,8 @@ typedef struct NvencContext
>      AVFifoBuffer *output_surface_queue;
>      AVFifoBuffer *output_surface_ready_queue;
>      AVFifoBuffer *timestamp_list;
> +    NV_ENC_SEI_PAYLOAD *sei_data;
> +    int sei_data_size;
> 
>      struct {
>          void *ptr;
Anything more on this version?

Brad
Timo Rothenpieler June 1, 2021, 10:01 a.m. UTC | #2
On 01.06.2021 11:51, Brad Hards wrote:
> Anything more on this version?
> 
> Brad
> 

No, but we're investigating weird crashes that are caused by the s12m 
timecodes _somehow_, so I'm holding off on merging any other changes in 
that area until it's figured out.
Brad Hards June 1, 2021, 10:46 a.m. UTC | #3
On Tuesday, 1 June 2021 8:01:13 PM AEST Timo Rothenpieler wrote:
> No, but we're investigating weird crashes that are caused by the s12m
> timecodes _somehow_, so I'm holding off on merging any other changes in
> that area until it's figured out.
Do you have a repro? Obviously motivated to unblock this.

Brad
Timo Rothenpieler June 1, 2021, 12:11 p.m. UTC | #4
On 01.06.2021 12:46, Brad Hards wrote:
> On Tuesday, 1 June 2021 8:01:13 PM AEST Timo Rothenpieler wrote:
>> No, but we're investigating weird crashes that are caused by the s12m
>> timecodes _somehow_, so I'm holding off on merging any other changes in
>> that area until it's figured out.
> Do you have a repro? Obviously motivated to unblock this.
> 
> Brad

It's not related to this patch, it crashes on FFmpeg master.
Also already tested it with this patch applied, and it still crashes.
Deep in the Nvidia driver.

This sample: https://btbn.de/nvenc-segfault.mkv

And this command line:

> ./ffmpeg.exe -i nvenc-segfault.mkv -map v -c:v hevc_nvenc -rc-lookahead 16 -s12m_tc 1 -y out.mp4

 From what I gathered so far, this ONLY crashes if
a) ffmpeg was built with gcc. No matter if Windows or Linux. However, 
building it with MSVC on Windows works and does not crash.
b) s12m timecode data is present in the sample. But not every sample 
with s12m timecodes crashes.
c) rc-lookahead is enabled

I'm very confused by this, and can't see anything obviously wrong in the 
FFmpeg code. But FFmpeg built with MSVC not crashing is indicating that 
the issue IS in ffmpeg, somehow.
Brad Hards June 1, 2021, 10:29 p.m. UTC | #5
On Tuesday, 1 June 2021 10:11:08 PM AEST Timo Rothenpieler wrote:
> On 01.06.2021 12:46, Brad Hards wrote:
> > On Tuesday, 1 June 2021 8:01:13 PM AEST Timo Rothenpieler wrote:
> >> No, but we're investigating weird crashes that are caused by the s12m
> >> timecodes _somehow_, so I'm holding off on merging any other changes in
> >> that area until it's figured out.
> > 
> > Do you have a repro? Obviously motivated to unblock this.
> > 
> > Brad
> 
> It's not related to this patch, it crashes on FFmpeg master.
> Also already tested it with this patch applied, and it still crashes.
> Deep in the Nvidia driver.
> 
> This sample: https://btbn.de/nvenc-segfault.mkv
> 
> And this command line:
> > ./ffmpeg.exe -i nvenc-segfault.mkv -map v -c:v hevc_nvenc -rc-lookahead 16
> > -s12m_tc 1 -y out.mp4
>  From what I gathered so far, this ONLY crashes if
> a) ffmpeg was built with gcc. No matter if Windows or Linux. However,
> building it with MSVC on Windows works and does not crash.
> b) s12m timecode data is present in the sample. But not every sample
> with s12m timecodes crashes.
> c) rc-lookahead is enabled
> 
> I'm very confused by this, and can't see anything obviously wrong in the
> FFmpeg code. But FFmpeg built with MSVC not crashing is indicating that
> the issue IS in ffmpeg, somehow.

On linux with valgrind, I see:

$ valgrind ./ffmpeg_g -i ~/nvenc-segfault.mkv -map v -c:v hevc_nvenc -rc-lookahead 16 -s12m_tc 1 -y out.mp4
==4234== Memcheck, a memory error detector
==4234== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==4234== Using Valgrind-3.16.1 and LibVEX; rerun with -h for copyright info
==4234== Command: ./ffmpeg_g -i /home/bradh/nvenc-segfault.mkv -map v -c:v hevc_nvenc -rc-lookahead 16 -s12m_tc 1 -y out.mp4
==4234== 
ffmpeg version N-102622-g9c1b6e0f09 Copyright (c) 2000-2021 the FFmpeg developers
  built with gcc 10 (Ubuntu 10.2.0-13ubuntu1)
  configuration: --enable-libx265 --enable-gpl --enable-libx264 --enable-libmfx
  libavutil      57.  0.100 / 57.  0.100
  libavcodec     59.  1.100 / 59.  1.100
  libavformat    59.  2.101 / 59.  2.101
  libavdevice    59.  0.100 / 59.  0.100
  libavfilter     8.  0.101 /  8.  0.101
  libswscale      6.  0.100 /  6.  0.100
  libswresample   4.  0.100 /  4.  0.100
  libpostproc    56.  0.100 / 56.  0.100
Input #0, matroska,webm, from '/home/bradh/nvenc-segfault.mkv':
  Metadata:
    ENCODER         : Lavf59.2.101
  Duration: 00:00:03.06, start: 0.140000, bitrate: 32869 kb/s
  Chapters:
    Chapter #0:0: start 0.000000, end 3.000000
  Stream #0:0: Video: h264 (High), yuv420p(progressive), 1920x1080 [SAR 1:1 DAR 16:9], 23.98 fps, 23.98 tbr, 1k tbn (default)
    Metadata:
      DURATION        : 00:00:03.059000000
Stream mapping:
  Stream #0:0 -> #0:0 (h264 (native) -> hevc (hevc_nvenc))
Press [q] to stop, [?] for help
==4234== Warning: noted but unhandled ioctl 0x30000001 with no size/direction hints.
==4234==    This could cause spurious value errors to appear.
==4234==    See README_MISSING_SYSCALL_OR_IOCTL for guidance on writing a proper wrapper.
==4234== Warning: noted but unhandled ioctl 0x27 with no size/direction hints.
==4234==    This could cause spurious value errors to appear.
==4234==    See README_MISSING_SYSCALL_OR_IOCTL for guidance on writing a proper wrapper.
==4234== Warning: noted but unhandled ioctl 0x25 with no size/direction hints.
==4234==    This could cause spurious value errors to appear.
==4234==    See README_MISSING_SYSCALL_OR_IOCTL for guidance on writing a proper wrapper.
==4234== Warning: noted but unhandled ioctl 0x17 with no size/direction hints.
==4234==    This could cause spurious value errors to appear.
==4234==    See README_MISSING_SYSCALL_OR_IOCTL for guidance on writing a proper wrapper.
==4234== Warning: set address range perms: large range [0x200000000, 0x300200000) (noaccess)
==4234== Warning: set address range perms: large range [0x1c620000, 0x3c61f000) (noaccess)
==4234== Warning: noted but unhandled ioctl 0x19 with no size/direction hints.
==4234==    This could cause spurious value errors to appear.
==4234==    See README_MISSING_SYSCALL_OR_IOCTL for guidance on writing a proper wrapper.
==4234== Warning: noted but unhandled ioctl 0x49 with no size/direction hints.
==4234==    This could cause spurious value errors to appear.
==4234==    See README_MISSING_SYSCALL_OR_IOCTL for guidance on writing a proper wrapper.
==4234== Warning: noted but unhandled ioctl 0x21 with no size/direction hints.
==4234==    This could cause spurious value errors to appear.
==4234==    See README_MISSING_SYSCALL_OR_IOCTL for guidance on writing a proper wrapper.
==4234== Warning: noted but unhandled ioctl 0x1b with no size/direction hints.
==4234==    This could cause spurious value errors to appear.
==4234==    See README_MISSING_SYSCALL_OR_IOCTL for guidance on writing a proper wrapper.
==4234== Warning: noted but unhandled ioctl 0x44 with no size/direction hints.
==4234==    This could cause spurious value errors to appear.
==4234==    See README_MISSING_SYSCALL_OR_IOCTL for guidance on writing a proper wrapper.
==4234== Warning: noted but unhandled ioctl 0x48 with no size/direction hints.
==4234==    This could cause spurious value errors to appear.
==4234==    See README_MISSING_SYSCALL_OR_IOCTL for guidance on writing a proper wrapper.
==4234== Conditional jump or move depends on uninitialised value(s)
==4234==    at 0x1BB861CF: ??? (in /usr/lib/x86_64-linux-gnu/libnvcuvid.so.460.73.01)
==4234==    by 0x1BB83377: ??? (in /usr/lib/x86_64-linux-gnu/libnvcuvid.so.460.73.01)
==4234==    by 0x1BBD85D1: ??? (in /usr/lib/x86_64-linux-gnu/libnvcuvid.so.460.73.01)
==4234==    by 0x1B940063: ??? (in /usr/lib/x86_64-linux-gnu/libnvidia-encode.so.460.73.01)
==4234==    by 0x1B94DD9A: ??? (in /usr/lib/x86_64-linux-gnu/libnvidia-encode.so.460.73.01)
==4234==    by 0x29BCE9: nvenc_open_session (nvenc.c:353)
==4234==    by 0x29F0F7: nvenc_check_device (nvenc.c:576)
==4234==    by 0x29F0F7: nvenc_setup_device (nvenc.c:699)
==4234==    by 0x29F0F7: ff_nvenc_encode_init (nvenc.c:1675)
==4234==    by 0x6DFE37: avcodec_open2 (avcodec.c:325)
==4234==    by 0x2D532C: init_output_stream.constprop.0 (ffmpeg.c:3592)
==4234==    by 0x2D8134: init_output_stream_wrapper (ffmpeg.c:992)
==4234==    by 0x2D8134: do_video_out (ffmpeg.c:1161)
==4234==    by 0x2D87E4: reap_filters (ffmpeg.c:1562)
==4234==    by 0x2DBD78: transcode_step (ffmpeg.c:4764)
==4234==    by 0x2DBD78: transcode (ffmpeg.c:4808)
==4234== 
Output #0, mp4, to 'out.mp4':
  Metadata:
    encoder         : Lavf59.2.101
  Chapters:
    Chapter #0:0: start 0.000000, end 2.860000
  Stream #0:0: Video: hevc (Main) (hev1 / 0x31766568), yuv420p(progressive), 1920x1080 [SAR 1:1 DAR 16:9], q=2-31, 2000 kb/s, 23.98 fps, 24k tbn (default)
    Metadata:
      DURATION        : 00:00:03.059000000
      encoder         : Lavc59.1.100 hevc_nvenc
    Side data:
      cpb: bitrate max/min/avg: 0/0/2000000 buffer size: 4000000 vbv_delay: N/A
==4234== Invalid read of size 8=       0kB time=00:00:00.12 bitrate=   2.8kbits/s speed=0.00854x     
==4234==    at 0x48411E6: memcpy@GLIBC_2.2.5 (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==4234==    by 0x1B947BD1: ??? (in /usr/lib/x86_64-linux-gnu/libnvidia-encode.so.460.73.01)
==4234==    by 0x1B947EEB: ??? (in /usr/lib/x86_64-linux-gnu/libnvidia-encode.so.460.73.01)
==4234==    by 0x1B940FE3: ??? (in /usr/lib/x86_64-linux-gnu/libnvidia-encode.so.460.73.01)
==4234==    by 0x1B9411C8: ??? (in /usr/lib/x86_64-linux-gnu/libnvidia-encode.so.460.73.01)
==4234==    by 0x1B9419DA: ??? (in /usr/lib/x86_64-linux-gnu/libnvidia-encode.so.460.73.01)
==4234==    by 0x1B93F3F8: ??? (in /usr/lib/x86_64-linux-gnu/libnvidia-encode.so.460.73.01)
==4234==    by 0x1B94D73C: ??? (in /usr/lib/x86_64-linux-gnu/libnvidia-encode.so.460.73.01)
==4234==    by 0xEF3F47: nvenc_send_frame (nvenc.c:2318)
==4234==    by 0xEF5187: ff_nvenc_receive_packet (nvenc.c:2365)
==4234==    by 0x7ACC43: encode_receive_packet_internal (encode.c:301)
==4234==    by 0x7AD2B0: avcodec_send_frame (encode.c:387)
==4234==  Address 0x0 is not stack'd, malloc'd or (recently) free'd
==4234== 
==4234== 
==4234== Process terminating with default action of signal 11 (SIGSEGV)
==4234==  Access not within mapped region at address 0x0
==4234==    at 0x48411E6: memcpy@GLIBC_2.2.5 (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==4234==    by 0x1B947BD1: ??? (in /usr/lib/x86_64-linux-gnu/libnvidia-encode.so.460.73.01)
==4234==    by 0x1B947EEB: ??? (in /usr/lib/x86_64-linux-gnu/libnvidia-encode.so.460.73.01)
==4234==    by 0x1B940FE3: ??? (in /usr/lib/x86_64-linux-gnu/libnvidia-encode.so.460.73.01)
==4234==    by 0x1B9411C8: ??? (in /usr/lib/x86_64-linux-gnu/libnvidia-encode.so.460.73.01)
==4234==    by 0x1B9419DA: ??? (in /usr/lib/x86_64-linux-gnu/libnvidia-encode.so.460.73.01)
==4234==    by 0x1B93F3F8: ??? (in /usr/lib/x86_64-linux-gnu/libnvidia-encode.so.460.73.01)
==4234==    by 0x1B94D73C: ??? (in /usr/lib/x86_64-linux-gnu/libnvidia-encode.so.460.73.01)
==4234==    by 0xEF3F47: nvenc_send_frame (nvenc.c:2318)
==4234==    by 0xEF5187: ff_nvenc_receive_packet (nvenc.c:2365)
==4234==    by 0x7ACC43: encode_receive_packet_internal (encode.c:301)
==4234==    by 0x7AD2B0: avcodec_send_frame (encode.c:387)
==4234==  If you believe this happened as a result of a stack
==4234==  overflow in your program's main thread (unlikely but
==4234==  possible), you can try to increase the size of the
==4234==  main thread stack using the --main-stacksize= flag.
==4234==  The main thread stack size used in this run was 8388608.
==4234== 
==4234== HEAP SUMMARY:
==4234==     in use at exit: 111,658,730 bytes in 16,658 blocks
==4234==   total heap usage: 26,438 allocs, 9,780 frees, 215,037,672 bytes allocated
==4234== 
==4234== LEAK SUMMARY:
==4234==    definitely lost: 72 bytes in 1 blocks
==4234==    indirectly lost: 0 bytes in 0 blocks
==4234==      possibly lost: 100,032 bytes in 1,167 blocks
==4234==    still reachable: 111,558,626 bytes in 15,490 blocks
==4234==         suppressed: 0 bytes in 0 blocks
==4234== Rerun with --leak-check=full to see details of leaked memory
==4234== 
==4234== Use --track-origins=yes to see where uninitialised values come from
==4234== For lists of detected and suppressed errors, rerun with: -s
==4234== ERROR SUMMARY: 2 errors from 2 contexts (suppressed: 0 from 0)
Segmentation fault (core dumped)

Is that consistent with the problem you are seeing?

Brad
Timo Rothenpieler June 1, 2021, 11:04 p.m. UTC | #6
On 02.06.2021 00:29, Brad Hards wrote:
> Is that consistent with the problem you are seeing?
> 
> Brad
> 

It matches the backtrace on both linux and Windows.
Something deep inside the nvidia driver is doing an invalid read.
Timo Rothenpieler June 4, 2021, 4:49 p.m. UTC | #7
Pushed a very much refactored version of this patch and some more 
refactoring on top of it.

It still shows the crash, but both the previous code and this code seem 
fine to me in that regard.
Brad Hards June 5, 2021, 2:36 a.m. UTC | #8
On Saturday, 5 June 2021 2:49:01 AM AEST Timo Rothenpieler wrote:
> Pushed a very much refactored version of this patch and some more
> refactoring on top of it.
> 
> It still shows the crash, but both the previous code and this code seem
> fine to me in that regard.
Thanks for the work, and the guidance along the journey.

Brad
diff mbox series

Patch

diff --git a/libavcodec/nvenc.c b/libavcodec/nvenc.c
index 0dcd93a99c..8ca5cab4c8 100644
--- a/libavcodec/nvenc.c
+++ b/libavcodec/nvenc.c
@@ -1610,6 +1610,8 @@  av_cold int ff_nvenc_encode_close(AVCodecContext *avctx)
 
     av_frame_free(&ctx->frame);
 
+    av_freep(&ctx->sei_data);
+
     if (ctx->nvencoder) {
         p_nvenc->nvEncDestroyEncoder(ctx->nvencoder);
 
@@ -2170,9 +2172,9 @@  static int nvenc_send_frame(AVCodecContext *avctx, const AVFrame *frame)
     NVENCSTATUS nv_status;
     NvencSurface *tmp_out_surf, *in_surf;
     int res, res2;
-    NV_ENC_SEI_PAYLOAD sei_data[8];
     int sei_count = 0;
     int i;
+    int total_unregistered_sei = 0;
 
     NvencContext *ctx = avctx->priv_data;
     NvencDynLoadFunctions *dl_fn = &ctx->nvenc_dload_funcs;
@@ -2185,6 +2187,8 @@  static int nvenc_send_frame(AVCodecContext *avctx, const AVFrame *frame)
         return AVERROR(EINVAL);
 
     if (frame && frame->buf[0]) {
+        void *a53_data = NULL;
+        void *tc_data = NULL;
         in_surf = get_free_frame(ctx);
         if (!in_surf)
             return AVERROR(EAGAIN);
@@ -2230,7 +2234,6 @@  static int nvenc_send_frame(AVCodecContext *avctx, const AVFrame *frame)
         pic_params.inputTimeStamp = frame->pts;
 
         if (ctx->a53_cc && av_frame_get_side_data(frame, AV_FRAME_DATA_A53_CC)) {
-            void *a53_data = NULL;
             size_t a53_size = 0;
 
             if (ff_alloc_a53_sei(frame, 0, (void**)&a53_data, &a53_size) < 0) {
@@ -2238,15 +2241,22 @@  static int nvenc_send_frame(AVCodecContext *avctx, const AVFrame *frame)
             }
 
             if (a53_data) {
-                sei_data[sei_count].payloadSize = (uint32_t)a53_size;
-                sei_data[sei_count].payloadType = 4;
-                sei_data[sei_count].payload = (uint8_t*)a53_data;
-                sei_count ++;
+                void *tmp = av_fast_realloc(ctx->sei_data,
+                                            &ctx->sei_data_size,
+                                            ++sei_count * sizeof(NV_ENC_SEI_PAYLOAD));
+                if (!tmp) {
+                    av_free(a53_data);
+                    return AVERROR(ENOMEM);
+                } else {
+                    ctx->sei_data = tmp;
+                    ctx->sei_data[sei_count-1].payloadSize = (uint32_t)a53_size;
+                    ctx->sei_data[sei_count-1].payloadType = 4;
+                    ctx->sei_data[sei_count-1].payload = (uint8_t*)a53_data;
+                }
             }
         }
 
         if (ctx->s12m_tc && av_frame_get_side_data(frame, AV_FRAME_DATA_S12M_TIMECODE)) {
-            void *tc_data = NULL;
             size_t tc_size = 0;
 
             if (ff_alloc_timecode_sei(frame, avctx->framerate, 0, (void**)&tc_data, &tc_size) < 0) {
@@ -2254,14 +2264,49 @@  static int nvenc_send_frame(AVCodecContext *avctx, const AVFrame *frame)
             }
 
             if (tc_data) {
-                sei_data[sei_count].payloadSize = (uint32_t)tc_size;
-                sei_data[sei_count].payloadType = SEI_TYPE_TIME_CODE;
-                sei_data[sei_count].payload = (uint8_t*)tc_data;
-                sei_count ++;
+                void *tmp = av_fast_realloc(ctx->sei_data,
+                                            &ctx->sei_data_size,
+                                            ++sei_count * sizeof(NV_ENC_SEI_PAYLOAD));
+                if (!tmp) {
+                    av_free(a53_data);
+                    av_free(tc_data);
+                    return AVERROR(ENOMEM);
+                } else {
+                    ctx->sei_data = tmp;
+                    ctx->sei_data[sei_count-1].payloadSize = (uint32_t)tc_size;
+                    ctx->sei_data[sei_count-1].payloadType = SEI_TYPE_TIME_CODE;
+                    ctx->sei_data[sei_count-1].payload = (uint8_t*)tc_data;
+                }
+            }
+        }
+
+        for (int j = 0; j < frame->nb_side_data; j++)
+            if (frame->side_data[j]->type == AV_FRAME_DATA_SEI_UNREGISTERED)
+                total_unregistered_sei++;
+        if (total_unregistered_sei > 0) {
+            void *tmp = av_fast_realloc(ctx->sei_data,
+                                        &(ctx->sei_data_size),
+                                        (sei_count + total_unregistered_sei) * sizeof(NV_ENC_SEI_PAYLOAD));
+            if (!tmp) {
+                av_free(a53_data);
+                av_free(tc_data);
+                return AVERROR(ENOMEM);
+            } else {
+                ctx->sei_data = tmp;
+                for (int j = 0; j < frame->nb_side_data; j++) {
+                    AVFrameSideData *side_data = frame->side_data[j];
+                    if (side_data->type == AV_FRAME_DATA_SEI_UNREGISTERED) {
+                        ctx->sei_data[sei_count].payloadSize = side_data->size;
+                        ctx->sei_data[sei_count].payloadType = SEI_TYPE_USER_DATA_UNREGISTERED;
+                        ctx->sei_data[sei_count].payload = av_memdup(side_data->data, side_data->size);
+                        if (ctx->sei_data[sei_count].payload)
+                            sei_count ++;
+                    }
+                }
             }
         }
 
-        nvenc_codec_specific_pic_params(avctx, &pic_params, sei_data, sei_count);
+        nvenc_codec_specific_pic_params(avctx, &pic_params, ctx->sei_data, sei_count);
     } else {
         pic_params.encodePicFlags = NV_ENC_PIC_FLAG_EOS;
     }
@@ -2273,7 +2318,7 @@  static int nvenc_send_frame(AVCodecContext *avctx, const AVFrame *frame)
     nv_status = p_nvenc->nvEncEncodePicture(ctx->nvencoder, &pic_params);
 
     for ( i = 0; i < sei_count; i++)
-        av_freep(&sei_data[i].payload);
+        av_freep(&ctx->sei_data[i].payload);
 
     res = nvenc_pop_context(avctx);
     if (res < 0)
diff --git a/libavcodec/nvenc.h b/libavcodec/nvenc.h
index 314c270e74..d28f02b54f 100644
--- a/libavcodec/nvenc.h
+++ b/libavcodec/nvenc.h
@@ -165,6 +165,8 @@  typedef struct NvencContext
     AVFifoBuffer *output_surface_queue;
     AVFifoBuffer *output_surface_ready_queue;
     AVFifoBuffer *timestamp_list;
+    NV_ENC_SEI_PAYLOAD *sei_data;
+    int sei_data_size;
 
     struct {
         void *ptr;