| Message ID | 1640320367-30993-1-git-send-email-lance.lmwang@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel] avcodec/nvenc: add udu_sei option to import user data unregistered SEIs | expand |
| Context | Check | Description |
|---|---|---|
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
| andriy/make_ppc | success | Make finished |
| andriy/make_fate_ppc | fail | Make fate failed |
On 24.12.2021 05:32, lance.lmwang@gmail.com wrote: > From: Limin Wang <lance.lmwang@gmail.com> > > Note: > nvenc sdk will truncated user data unregistered SEI if the size > 503. > for example, hardcode its size to 504, trace_headers will report below error: > Invalid SEI message: payload_size too large (504 504 bytes). I'm amazed it even does that now. Last time I tested this, encoding straight up failed if the SEI data was too large, since the allocated packet output buffer has a fixed size, and no API to influence that size exists. Disabling unregistered SEIs by default is probably a good call, given 99% of the time it'll lead to unexpected failures and is not actually intended or used by the user. It IS technically an API break though, so not 100% sure on this. Ideally it'd be possible to just increase the buffer size, so the SEI data fits. Where exactly did you see this 503 byte truncation happening?
On Fri, Dec 24, 2021 at 03:00:10PM +0100, Timo Rothenpieler wrote: > On 24.12.2021 05:32, lance.lmwang@gmail.com wrote: > > From: Limin Wang <lance.lmwang@gmail.com> > > > > Note: > > nvenc sdk will truncated user data unregistered SEI if the size > 503. > > for example, hardcode its size to 504, trace_headers will report below error: > > Invalid SEI message: payload_size too large (504 504 bytes). > > I'm amazed it even does that now. > Last time I tested this, encoding straight up failed if the SEI data was too > large, since the allocated packet output buffer has a fixed size, and no API > to influence that size exists. I'm not sure whether it's for the sdk version, I'm using nv-codec-headers(sdk/11.0) and encoding is OK, but I can't find any user data unregistered SEIs although my testing is created by libx264(default the data has encoding information by the UDU SEIs, its size > 503 always). So I using trace_headers to trace the output and find out it's truncated. > > Disabling unregistered SEIs by default is probably a good call, given 99% of > the time it'll lead to unexpected failures and is not actually intended or > used by the user. Yes, if it's not truncated, you'll see the x264 encoding setting after using h264_nvenc encoding with libx264 encoding file, it's very misleading in fact. > > It IS technically an API break though, so not 100% sure on this. It's introduced by commit: cee9f9628fb, it's not correct to import unregistered SEIs to output default and the patch try to fix that. We can bump the minor version for the API change. > Ideally it'd be possible to just increase the buffer size, so the SEI data > fits. > > Where exactly did you see this 503 byte truncation happening? You can tested with libx264 output file, the size of unregistered SEIs(for encoding setting) will > 503 always, if you don't make the size <= 503, you'll see below error by trace_eaders filter. Invalid SEI message: payload_size too large (xxx 504 bytes). Note, libavcodec/cbs_sei_syntax_template.c line 262, I print out "get_bits_left(rw) / 8". I think the buffer size is OK, but nvenc sdk truncate the data after encoding. > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
On 24.12.2021 15:47, lance.lmwang@gmail.com wrote: > > You can tested with libx264 output file, the size of unregistered SEIs(for encoding > setting) will > 503 always, if you don't make the size <= 503, you'll see below > error by trace_eaders filter. > Invalid SEI message: payload_size too large (xxx 504 bytes). > > Note, libavcodec/cbs_sei_syntax_template.c line 262, I print out "get_bits_left(rw) / 8". > > I think the buffer size is OK, but nvenc sdk truncate the data after encoding. What exactly are you referring to by nvenc sdk? Are you looking at some Nvidia sample that does this truncating? Cause if I try this with ffmpeg right now, encoding straight up fails if a too large SEI segment is attached, since nvenc runs out of space in the output buffer.
On Fri, Dec 24, 2021 at 04:52:35PM +0100, Timo Rothenpieler wrote: > On 24.12.2021 15:47, lance.lmwang@gmail.com wrote: > > > > You can tested with libx264 output file, the size of unregistered SEIs(for encoding > > setting) will > 503 always, if you don't make the size <= 503, you'll see below > > error by trace_eaders filter. > > Invalid SEI message: payload_size too large (xxx 504 bytes). > > > > Note, libavcodec/cbs_sei_syntax_template.c line 262, I print out "get_bits_left(rw) / 8". > > > > I think the buffer size is OK, but nvenc sdk truncate the data after encoding. > > What exactly are you referring to by nvenc sdk? Are you looking at some > Nvidia sample that does this truncating? I guess it's caused by nv sdk library failed to forward the SEI data output completely. It's not related to the patch, so I tested with small size SEIs and veirfy the option working as expected. > Cause if I try this with ffmpeg right now, encoding straight up fails if a > too large SEI segment is attached, since nvenc runs out of space in the > output buffer. I haven created 5 frame by libx264 and test it, so maybe it's too short to cause encoding failed. I have used bsf filter to add user unregistered before, it haven't such limitation or issue. > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
Applied a slightly modified version. Just so it avoids cycling through the side data loop entirely when it's never going to be needed.
On Sat, Dec 25, 2021 at 02:15:36PM +0100, Timo Rothenpieler wrote: > Applied a slightly modified version. > Just so it avoids cycling through the side data loop entirely when it's > never going to be needed. Thanks, it's better, I'll move the check out of loop for libx264 and libx265 to avoid the unnecessary cycling. > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
diff --git a/libavcodec/nvenc.c b/libavcodec/nvenc.c index edc46ed..12ffcf0 100644 --- a/libavcodec/nvenc.c +++ b/libavcodec/nvenc.c @@ -2221,7 +2221,7 @@ static int prepare_sei_data_array(AVCodecContext *avctx, const AVFrame *frame) AVFrameSideData *side_data = frame->side_data[i]; void *tmp; - if (side_data->type != AV_FRAME_DATA_SEI_UNREGISTERED) + if (!(ctx->udu_sei && side_data->type == AV_FRAME_DATA_SEI_UNREGISTERED)) continue; tmp = av_fast_realloc(ctx->sei_data, diff --git a/libavcodec/nvenc.h b/libavcodec/nvenc.h index 08531e1..b42a156 100644 --- a/libavcodec/nvenc.h +++ b/libavcodec/nvenc.h @@ -235,6 +235,7 @@ typedef struct NvencContext int intra_refresh; int single_slice_intra_refresh; int constrained_encoding; + int udu_sei; } NvencContext; int ff_nvenc_encode_init(AVCodecContext *avctx); diff --git a/libavcodec/nvenc_h264.c b/libavcodec/nvenc_h264.c index 7d78aa0..79b7aca 100644 --- a/libavcodec/nvenc_h264.c +++ b/libavcodec/nvenc_h264.c @@ -194,6 +194,8 @@ static const AVOption options[] = { OFFSET(single_slice_intra_refresh), AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, VE }, { "constrained-encoding", "Enable constrainedFrame encoding where each slice in the constrained picture is independent of other slices", OFFSET(constrained_encoding), AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, VE }, + { "udu_sei", "Use user data unregistered SEI if available", + OFFSET(udu_sei), AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, VE }, { NULL } }; diff --git a/libavcodec/nvenc_hevc.c b/libavcodec/nvenc_hevc.c index a13ac5a..12b4e11 100644 --- a/libavcodec/nvenc_hevc.c +++ b/libavcodec/nvenc_hevc.c @@ -175,6 +175,8 @@ static const AVOption options[] = { OFFSET(single_slice_intra_refresh), AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, VE }, { "constrained-encoding", "Enable constrainedFrame encoding where each slice in the constrained picture is independent of other slices", OFFSET(constrained_encoding), AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, VE }, + { "udu_sei", "Use user data unregistered SEI if available", + OFFSET(udu_sei), AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, VE }, { NULL } };