Re: [FFmpeg-devel] [PATCH v11 4/4] avcodec/h264: create user data unregistered SEI side data for H.264
On Wed, Jun 10, 2020 at 08:02:10PM +0200, Marton Balint wrote:
>
>
> On Wed, 10 Jun 2020, lance.lmw...@gmail.com wrote:
>
> > From: Limin Wang
> >
> > Signed-off-by: Limin Wang
> > ---
> > libavcodec/h264_sei.c | 18 -
> > libavcodec/h264_sei.h | 2 +
> > libavcodec/h264_slice.c | 14
> > tests/ref/fate/mov-zombie | 195
> > ++
> > 4 files changed, 161 insertions(+), 68 deletions(-)
> >
> > diff --git a/libavcodec/h264_sei.c b/libavcodec/h264_sei.c
> > index 870dd90..a903706 100644
> > --- a/libavcodec/h264_sei.c
> > +++ b/libavcodec/h264_sei.c
> > @@ -52,6 +52,10 @@ void ff_h264_sei_uninit(H264SEIContext *h)
> > h->afd.present = 0;
> >
> > av_buffer_unref(&h->a53_caption.buf_ref);
> > +for (int i = 0; i < h->unregistered.nb_buf_ref; i++)
> > +av_buffer_unref(&h->unregistered.buf_ref[i]);
> > +h->unregistered.nb_buf_ref = 0;
> > +av_freep(&h->unregistered.buf_ref);
> > }
> >
> > int ff_h264_sei_process_picture_timing(H264SEIPictureTiming *h, const SPS
> > *sps,
> > @@ -260,25 +264,33 @@ static int
> > decode_unregistered_user_data(H264SEIUnregistered *h, GetBitContext *
> > {
> > uint8_t *user_data;
> > int e, build, i;
> > +AVBufferRef *buf_ref, **tmp;
> >
> > if (size < 16 || size >= INT_MAX - 1)
> > return AVERROR_INVALIDDATA;
> >
> > -user_data = av_malloc(size + 1);
> > -if (!user_data)
> > +tmp = av_realloc_array(h->buf_ref, h->nb_buf_ref + 1,
> > sizeof(*h->buf_ref));
> > +if (!tmp)
> > return AVERROR(ENOMEM);
> > +h->buf_ref = tmp;
> > +
> > +buf_ref = av_buffer_alloc(size + 1);
> > +if (!buf_ref)
> > +return AVERROR(ENOMEM);
>
> You are supposed to set buf_ref->size to size, otherwise the side data will
> also have the extra 1 byte padding as far as I see.
thanks, I got your points now. I'll fix it
>
> Thanks,
> Marton
>
> > +user_data = buf_ref->data;
> >
> > for (i = 0; i < size; i++)
> > user_data[i] = get_bits(gb, 8);
> >
> > user_data[i] = 0;
> > +h->buf_ref[h->nb_buf_ref++] = buf_ref;
> > +
> > e = sscanf(user_data + 16, "x264 - core %d", &build);
> > if (e == 1 && build > 0)
> > h->x264_build = build;
> > if (e == 1 && build == 1 && !strncmp(user_data+16, "x264 - core ",
> > 16))
> > h->x264_build = 67;
> >
> > -av_free(user_data);
> > return 0;
> > }
> >
> > diff --git a/libavcodec/h264_sei.h b/libavcodec/h264_sei.h
> > index f07a505..4fdcf4e 100644
> > --- a/libavcodec/h264_sei.h
> > +++ b/libavcodec/h264_sei.h
> > @@ -126,6 +126,8 @@ typedef struct H264SEIA53Caption {
> >
> > typedef struct H264SEIUnregistered {
> > int x264_build;
> > +AVBufferRef **buf_ref;
> > +int nb_buf_ref;
> > } H264SEIUnregistered;
> >
> > typedef struct H264SEIRecoveryPoint {
> > diff --git a/libavcodec/h264_slice.c b/libavcodec/h264_slice.c
> > index 7139537..47f3917 100644
> > --- a/libavcodec/h264_slice.c
> > +++ b/libavcodec/h264_slice.c
> > @@ -1289,6 +1289,20 @@ static int h264_export_frame_props(H264Context *h)
> > h->avctx->properties |= FF_CODEC_PROPERTY_CLOSED_CAPTIONS;
> > }
> >
> > +for (int i = 0; i < h->sei.unregistered.nb_buf_ref; i++) {
> > +H264SEIUnregistered *unreg = &h->sei.unregistered;
> > +
> > +if (unreg->buf_ref[i]) {
> > +AVFrameSideData *sd = av_frame_new_side_data_from_buf(cur->f,
> > +AV_FRAME_DATA_SEI_UNREGISTERED,
> > +unreg->buf_ref[i]);
> > +if (!sd)
> > +av_buffer_unref(&unreg->buf_ref[i]);
> > +unreg->buf_ref[i] = NULL;
> > +}
> > +}
> > +h->sei.unregistered.nb_buf_ref = 0;
> > +
> > if (h->sei.picture_timing.timecode_cnt > 0) {
> > uint32_t tc = 0;
> > uint32_t *tc_sd;
> > diff --git a/tests/ref/fate/mov-zombie b/tests/ref/fate/mov-zombie
> > index 445f921..1a6625b 100644
> > --- a/tests/ref/fate/mov-zombie
> > +++ b/tests/ref/fate/mov-zombie
> > @@ -1,133 +1,198 @@
> > packet|codec_type=video|stream_index=0|pts=0|pts_time=0.00|dts=-3004|dts_time=-0.033378|duration=3003|duration_time=0.033367|convergence_duration=N/A|convergence_duration_time=N/A|size=4133|pos=11309|flags=K_
> > packet|codec_type=video|stream_index=0|pts=5440|pts_time=0.060444|dts=-567|dts_time=-0.006300|duration=3003|duration_time=0.033367|convergence_duration=N/A|convergence_duration_time=N/A|size=1077|pos=15442|flags=__
> > -frame|media_type=video|stream_index=0|key_frame=1|pkt_pts=0|pkt_pts_time=0.00|pkt_dts=-567|pkt_dts_time=-0.006300|best_effort_timestamp=0|best_effort_timestamp_time=0.00|pkt_duration=3003|pkt_duration_time=0.033367|pkt_pos=11309|pkt_size=4133|width=160|height=240|pix_fmt=yuv420p|sample_aspect_ratio=2:1|pict_type=I|coded_picture_number=0|display_picture_number=0|interlaced_frame=0|top_field_first=0|repeat_pict=0|color_range=tv|color_space=smpt
Re: [FFmpeg-devel] [PATCH v11 4/4] avcodec/h264: create user data unregistered SEI side data for H.264
On Wed, 10 Jun 2020, lance.lmw...@gmail.com wrote:
From: Limin Wang
Signed-off-by: Limin Wang
---
libavcodec/h264_sei.c | 18 -
libavcodec/h264_sei.h | 2 +
libavcodec/h264_slice.c | 14
tests/ref/fate/mov-zombie | 195 ++
4 files changed, 161 insertions(+), 68 deletions(-)
diff --git a/libavcodec/h264_sei.c b/libavcodec/h264_sei.c
index 870dd90..a903706 100644
--- a/libavcodec/h264_sei.c
+++ b/libavcodec/h264_sei.c
@@ -52,6 +52,10 @@ void ff_h264_sei_uninit(H264SEIContext *h)
h->afd.present = 0;
av_buffer_unref(&h->a53_caption.buf_ref);
+for (int i = 0; i < h->unregistered.nb_buf_ref; i++)
+av_buffer_unref(&h->unregistered.buf_ref[i]);
+h->unregistered.nb_buf_ref = 0;
+av_freep(&h->unregistered.buf_ref);
}
int ff_h264_sei_process_picture_timing(H264SEIPictureTiming *h, const SPS *sps,
@@ -260,25 +264,33 @@ static int
decode_unregistered_user_data(H264SEIUnregistered *h, GetBitContext *
{
uint8_t *user_data;
int e, build, i;
+AVBufferRef *buf_ref, **tmp;
if (size < 16 || size >= INT_MAX - 1)
return AVERROR_INVALIDDATA;
-user_data = av_malloc(size + 1);
-if (!user_data)
+tmp = av_realloc_array(h->buf_ref, h->nb_buf_ref + 1, sizeof(*h->buf_ref));
+if (!tmp)
return AVERROR(ENOMEM);
+h->buf_ref = tmp;
+
+buf_ref = av_buffer_alloc(size + 1);
+if (!buf_ref)
+return AVERROR(ENOMEM);
You are supposed to set buf_ref->size to size, otherwise the side
data will also have the extra 1 byte padding as far as I see.
Thanks,
Marton
+user_data = buf_ref->data;
for (i = 0; i < size; i++)
user_data[i] = get_bits(gb, 8);
user_data[i] = 0;
+h->buf_ref[h->nb_buf_ref++] = buf_ref;
+
e = sscanf(user_data + 16, "x264 - core %d", &build);
if (e == 1 && build > 0)
h->x264_build = build;
if (e == 1 && build == 1 && !strncmp(user_data+16, "x264 - core ", 16))
h->x264_build = 67;
-av_free(user_data);
return 0;
}
diff --git a/libavcodec/h264_sei.h b/libavcodec/h264_sei.h
index f07a505..4fdcf4e 100644
--- a/libavcodec/h264_sei.h
+++ b/libavcodec/h264_sei.h
@@ -126,6 +126,8 @@ typedef struct H264SEIA53Caption {
typedef struct H264SEIUnregistered {
int x264_build;
+AVBufferRef **buf_ref;
+int nb_buf_ref;
} H264SEIUnregistered;
typedef struct H264SEIRecoveryPoint {
diff --git a/libavcodec/h264_slice.c b/libavcodec/h264_slice.c
index 7139537..47f3917 100644
--- a/libavcodec/h264_slice.c
+++ b/libavcodec/h264_slice.c
@@ -1289,6 +1289,20 @@ static int h264_export_frame_props(H264Context *h)
h->avctx->properties |= FF_CODEC_PROPERTY_CLOSED_CAPTIONS;
}
+for (int i = 0; i < h->sei.unregistered.nb_buf_ref; i++) {
+H264SEIUnregistered *unreg = &h->sei.unregistered;
+
+if (unreg->buf_ref[i]) {
+AVFrameSideData *sd = av_frame_new_side_data_from_buf(cur->f,
+AV_FRAME_DATA_SEI_UNREGISTERED,
+unreg->buf_ref[i]);
+if (!sd)
+av_buffer_unref(&unreg->buf_ref[i]);
+unreg->buf_ref[i] = NULL;
+}
+}
+h->sei.unregistered.nb_buf_ref = 0;
+
if (h->sei.picture_timing.timecode_cnt > 0) {
uint32_t tc = 0;
uint32_t *tc_sd;
diff --git a/tests/ref/fate/mov-zombie b/tests/ref/fate/mov-zombie
index 445f921..1a6625b 100644
--- a/tests/ref/fate/mov-zombie
+++ b/tests/ref/fate/mov-zombie
@@ -1,133 +1,198 @@
packet|codec_type=video|stream_index=0|pts=0|pts_time=0.00|dts=-3004|dts_time=-0.033378|duration=3003|duration_time=0.033367|convergence_duration=N/A|convergence_duration_time=N/A|size=4133|pos=11309|flags=K_
packet|codec_type=video|stream_index=0|pts=5440|pts_time=0.060444|dts=-567|dts_time=-0.006300|duration=3003|duration_time=0.033367|convergence_duration=N/A|convergence_duration_time=N/A|size=1077|pos=15442|flags=__
-frame|media_type=video|stream_index=0|key_frame=1|pkt_pts=0|pkt_pts_time=0.00|pkt_dts=-567|pkt_dts_time=-0.006300|best_effort_timestamp=0|best_effort_timestamp_time=0.00|pkt_duration=3003|pkt_duration_time=0.033367|pkt_pos=11309|pkt_size=4133|width=160|height=240|pix_fmt=yuv420p|sample_aspect_ratio=2:1|pict_type=I|coded_picture_number=0|display_picture_number=0|interlaced_frame=0|top_field_first=0|repeat_pict=0|color_range=tv|color_space=smpte170m|color_primaries=smpte170m|color_transfer=bt709|chroma_location=topleft
+frame|media_type=video|stream_index=0|key_frame=1|pkt_pts=0|pkt_pts_time=0.00|pkt_dts=-567|pkt_dts_time=-0.006300|best_effort_timestamp=0|best_effort_timestamp_time=0.00|pkt_duration=3003|pkt_duration_time=0.033367|pkt_pos=11309|pkt_size=4133|width=160|height=240|pix_fmt=yuv420p|sample_aspect_ratio=2:1|pict_type=I|coded_picture_number=0|display_picture_number=0|interlaced_frame=0|top_field_first=0|repeat_pict=0|color_range=tv|color_space=smpte170m|color_primaries=smpte170m|color_transfer=bt
[FFmpeg-devel] [PATCH v11 4/4] avcodec/h264: create user data unregistered SEI side data for H.264
From: Limin Wang
Signed-off-by: Limin Wang
---
libavcodec/h264_sei.c | 18 -
libavcodec/h264_sei.h | 2 +
libavcodec/h264_slice.c | 14
tests/ref/fate/mov-zombie | 195 ++
4 files changed, 161 insertions(+), 68 deletions(-)
diff --git a/libavcodec/h264_sei.c b/libavcodec/h264_sei.c
index 870dd90..a903706 100644
--- a/libavcodec/h264_sei.c
+++ b/libavcodec/h264_sei.c
@@ -52,6 +52,10 @@ void ff_h264_sei_uninit(H264SEIContext *h)
h->afd.present = 0;
av_buffer_unref(&h->a53_caption.buf_ref);
+for (int i = 0; i < h->unregistered.nb_buf_ref; i++)
+av_buffer_unref(&h->unregistered.buf_ref[i]);
+h->unregistered.nb_buf_ref = 0;
+av_freep(&h->unregistered.buf_ref);
}
int ff_h264_sei_process_picture_timing(H264SEIPictureTiming *h, const SPS *sps,
@@ -260,25 +264,33 @@ static int
decode_unregistered_user_data(H264SEIUnregistered *h, GetBitContext *
{
uint8_t *user_data;
int e, build, i;
+AVBufferRef *buf_ref, **tmp;
if (size < 16 || size >= INT_MAX - 1)
return AVERROR_INVALIDDATA;
-user_data = av_malloc(size + 1);
-if (!user_data)
+tmp = av_realloc_array(h->buf_ref, h->nb_buf_ref + 1, sizeof(*h->buf_ref));
+if (!tmp)
return AVERROR(ENOMEM);
+h->buf_ref = tmp;
+
+buf_ref = av_buffer_alloc(size + 1);
+if (!buf_ref)
+return AVERROR(ENOMEM);
+user_data = buf_ref->data;
for (i = 0; i < size; i++)
user_data[i] = get_bits(gb, 8);
user_data[i] = 0;
+h->buf_ref[h->nb_buf_ref++] = buf_ref;
+
e = sscanf(user_data + 16, "x264 - core %d", &build);
if (e == 1 && build > 0)
h->x264_build = build;
if (e == 1 && build == 1 && !strncmp(user_data+16, "x264 - core ", 16))
h->x264_build = 67;
-av_free(user_data);
return 0;
}
diff --git a/libavcodec/h264_sei.h b/libavcodec/h264_sei.h
index f07a505..4fdcf4e 100644
--- a/libavcodec/h264_sei.h
+++ b/libavcodec/h264_sei.h
@@ -126,6 +126,8 @@ typedef struct H264SEIA53Caption {
typedef struct H264SEIUnregistered {
int x264_build;
+AVBufferRef **buf_ref;
+int nb_buf_ref;
} H264SEIUnregistered;
typedef struct H264SEIRecoveryPoint {
diff --git a/libavcodec/h264_slice.c b/libavcodec/h264_slice.c
index 7139537..47f3917 100644
--- a/libavcodec/h264_slice.c
+++ b/libavcodec/h264_slice.c
@@ -1289,6 +1289,20 @@ static int h264_export_frame_props(H264Context *h)
h->avctx->properties |= FF_CODEC_PROPERTY_CLOSED_CAPTIONS;
}
+for (int i = 0; i < h->sei.unregistered.nb_buf_ref; i++) {
+H264SEIUnregistered *unreg = &h->sei.unregistered;
+
+if (unreg->buf_ref[i]) {
+AVFrameSideData *sd = av_frame_new_side_data_from_buf(cur->f,
+AV_FRAME_DATA_SEI_UNREGISTERED,
+unreg->buf_ref[i]);
+if (!sd)
+av_buffer_unref(&unreg->buf_ref[i]);
+unreg->buf_ref[i] = NULL;
+}
+}
+h->sei.unregistered.nb_buf_ref = 0;
+
if (h->sei.picture_timing.timecode_cnt > 0) {
uint32_t tc = 0;
uint32_t *tc_sd;
diff --git a/tests/ref/fate/mov-zombie b/tests/ref/fate/mov-zombie
index 445f921..1a6625b 100644
--- a/tests/ref/fate/mov-zombie
+++ b/tests/ref/fate/mov-zombie
@@ -1,133 +1,198 @@
packet|codec_type=video|stream_index=0|pts=0|pts_time=0.00|dts=-3004|dts_time=-0.033378|duration=3003|duration_time=0.033367|convergence_duration=N/A|convergence_duration_time=N/A|size=4133|pos=11309|flags=K_
packet|codec_type=video|stream_index=0|pts=5440|pts_time=0.060444|dts=-567|dts_time=-0.006300|duration=3003|duration_time=0.033367|convergence_duration=N/A|convergence_duration_time=N/A|size=1077|pos=15442|flags=__
-frame|media_type=video|stream_index=0|key_frame=1|pkt_pts=0|pkt_pts_time=0.00|pkt_dts=-567|pkt_dts_time=-0.006300|best_effort_timestamp=0|best_effort_timestamp_time=0.00|pkt_duration=3003|pkt_duration_time=0.033367|pkt_pos=11309|pkt_size=4133|width=160|height=240|pix_fmt=yuv420p|sample_aspect_ratio=2:1|pict_type=I|coded_picture_number=0|display_picture_number=0|interlaced_frame=0|top_field_first=0|repeat_pict=0|color_range=tv|color_space=smpte170m|color_primaries=smpte170m|color_transfer=bt709|chroma_location=topleft
+frame|media_type=video|stream_index=0|key_frame=1|pkt_pts=0|pkt_pts_time=0.00|pkt_dts=-567|pkt_dts_time=-0.006300|best_effort_timestamp=0|best_effort_timestamp_time=0.00|pkt_duration=3003|pkt_duration_time=0.033367|pkt_pos=11309|pkt_size=4133|width=160|height=240|pix_fmt=yuv420p|sample_aspect_ratio=2:1|pict_type=I|coded_picture_number=0|display_picture_number=0|interlaced_frame=0|top_field_first=0|repeat_pict=0|color_range=tv|color_space=smpte170m|color_primaries=smpte170m|color_transfer=bt709|chroma_location=topleftside_data|side_data_type=H.26[45]
User Data Unregistered SEI message
+
packet|codec_type=video|stream_index=0|pts=2437|pts_time
