Message ID | 20210430113432.308139-5-bradh@frogmouth.net |
---|---|
State | New |
Headers | show |
Series | [FFmpeg-devel,1/5] libavutil: add convenience accessors for frame side data | expand |
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 |
On 30.04.2021 13:34, Brad Hards wrote: > Signed-off-by: Brad Hards <bradh@frogmouth.net> > --- > libavcodec/nvenc.c | 16 ++++++++++++++++ > 1 file changed, 16 insertions(+) > > diff --git a/libavcodec/nvenc.c b/libavcodec/nvenc.c > index 0dcd93a99c..1a895a64f4 100644 > --- a/libavcodec/nvenc.c > +++ b/libavcodec/nvenc.c > @@ -2173,6 +2173,8 @@ static int nvenc_send_frame(AVCodecContext *avctx, const AVFrame *frame) > NV_ENC_SEI_PAYLOAD sei_data[8]; > int sei_count = 0; > int i; > + AVFrameSideData *sd; > + int num_unregistered_sei; > > NvencContext *ctx = avctx->priv_data; > NvencDynLoadFunctions *dl_fn = &ctx->nvenc_dload_funcs; > @@ -2261,6 +2263,20 @@ static int nvenc_send_frame(AVCodecContext *avctx, const AVFrame *frame) > } > } > > + num_unregistered_sei = av_frame_num_side_data(frame, AV_FRAME_DATA_SEI_UNREGISTERED); > + for (int i = 0; i < num_unregistered_sei; i++) { redefines i from above. Not sure if that's even valid. > + sd = av_frame_get_side_data_n(frame, AV_FRAME_DATA_SEI_UNREGISTERED, i); > + if (sd) { > + sei_data[sei_count].payloadSize = sd->size; > + sei_data[sei_count].payloadType = SEI_TYPE_USER_DATA_UNREGISTERED; > + sei_data[sei_count].payload = av_memdup(sd->data, sd->size); > + sei_count ++; > + if (sei_count >= 8) { > + break; > + } > + } > + } I'm not at all a fan of writing an arbitrary amount of stuff into a small fixed size array. > nvenc_codec_specific_pic_params(avctx, &pic_params, sei_data, sei_count); > } else { > pic_params.encodePicFlags = NV_ENC_PIC_FLAG_EOS; >
On Saturday, 1 May 2021 1:27:44 AM AEST Timo Rothenpieler wrote: > On 30.04.2021 13:34, Brad Hards wrote: > > + num_unregistered_sei = av_frame_num_side_data(frame, > > AV_FRAME_DATA_SEI_UNREGISTERED); + for (int i = 0; i < > > num_unregistered_sei; i++) { > > redefines i from above. Not sure if that's even valid. Will fix. > > + sd = av_frame_get_side_data_n(frame, > > AV_FRAME_DATA_SEI_UNREGISTERED, i); + if (sd) { > > + sei_data[sei_count].payloadSize = sd->size; > > + sei_data[sei_count].payloadType = > > SEI_TYPE_USER_DATA_UNREGISTERED; + > > sei_data[sei_count].payload = av_memdup(sd->data, sd->size); + > > sei_count ++; > > + if (sei_count >= 8) { > > + break; > > + } > > + } > > + } > > I'm not at all a fan of writing an arbitrary amount of stuff into a > small fixed size array. I don't understand this comment. The existing code defines a fixed size array, and I don't want to change it because I couldn't find an authoritative source as to why that size or approach. I assume it could be a hardware limitation on at least some versions. So I've tried to follow the existing approach, and ensure that the loop never exceeds the available space by tracking the count. Am I missing something? Brad
On 01.05.2021 01:30, Brad Hards wrote: > I don't understand this comment. The existing code defines a fixed size array, > and I don't want to change it because I couldn't find an authoritative source > as to why that size or approach. I assume it could be a hardware limitation on > at least some versions. > So I've tried to follow the existing approach, and ensure that the loop never > exceeds the available space by tracking the count. > Am I missing something? Pretty sure the 8 is just an arbitrary number picked when originally writing this code. It now no longer seems appropriate, so a dynamic array might be in order.
diff --git a/libavcodec/nvenc.c b/libavcodec/nvenc.c index 0dcd93a99c..1a895a64f4 100644 --- a/libavcodec/nvenc.c +++ b/libavcodec/nvenc.c @@ -2173,6 +2173,8 @@ static int nvenc_send_frame(AVCodecContext *avctx, const AVFrame *frame) NV_ENC_SEI_PAYLOAD sei_data[8]; int sei_count = 0; int i; + AVFrameSideData *sd; + int num_unregistered_sei; NvencContext *ctx = avctx->priv_data; NvencDynLoadFunctions *dl_fn = &ctx->nvenc_dload_funcs; @@ -2261,6 +2263,20 @@ static int nvenc_send_frame(AVCodecContext *avctx, const AVFrame *frame) } } + num_unregistered_sei = av_frame_num_side_data(frame, AV_FRAME_DATA_SEI_UNREGISTERED); + for (int i = 0; i < num_unregistered_sei; i++) { + sd = av_frame_get_side_data_n(frame, AV_FRAME_DATA_SEI_UNREGISTERED, i); + if (sd) { + sei_data[sei_count].payloadSize = sd->size; + sei_data[sei_count].payloadType = SEI_TYPE_USER_DATA_UNREGISTERED; + sei_data[sei_count].payload = av_memdup(sd->data, sd->size); + sei_count ++; + if (sei_count >= 8) { + break; + } + } + } + nvenc_codec_specific_pic_params(avctx, &pic_params, sei_data, sei_count); } else { pic_params.encodePicFlags = NV_ENC_PIC_FLAG_EOS;
Signed-off-by: Brad Hards <bradh@frogmouth.net> --- libavcodec/nvenc.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)