diff mbox series

[FFmpeg-devel] avcodec/nvenc: add udu_sei option to import user data unregistered SEIs

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

Checks

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

Commit Message

Limin Wang Dec. 24, 2021, 4:32 a.m. UTC
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).

Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
---
 libavcodec/nvenc.c      | 2 +-
 libavcodec/nvenc.h      | 1 +
 libavcodec/nvenc_h264.c | 2 ++
 libavcodec/nvenc_hevc.c | 2 ++
 4 files changed, 6 insertions(+), 1 deletion(-)

Comments

Timo Rothenpieler Dec. 24, 2021, 2 p.m. UTC | #1
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?
Limin Wang Dec. 24, 2021, 2:47 p.m. UTC | #2
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".
Timo Rothenpieler Dec. 24, 2021, 3:52 p.m. UTC | #3
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.
Limin Wang Dec. 25, 2021, 12:43 a.m. UTC | #4
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".
Timo Rothenpieler Dec. 25, 2021, 1:15 p.m. UTC | #5
Applied a slightly modified version.
Just so it avoids cycling through the side data loop entirely when it's 
never going to be needed.
Limin Wang Dec. 25, 2021, 2:12 p.m. UTC | #6
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 mbox series

Patch

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 }
 };