diff mbox series

[FFmpeg-devel,v9,2/4] avcodec/hevc_sei: add support for user data unregistered SEI message

Message ID 20200317105409.2795-2-lance.lmwang@gmail.com
State New
Headers show
Series [FFmpeg-devel,v9,1/4] avutil: add AV_FRAME_DATA_SEI_UNREGISTERED side data type
Related show

Checks

Context Check Description
andriy/ffmpeg-patchwork pending
andriy/ffmpeg-patchwork success Applied patch
andriy/ffmpeg-patchwork success Configure finished
andriy/ffmpeg-patchwork success Make finished
andriy/ffmpeg-patchwork success Make fate finished

Commit Message

Limin Wang March 17, 2020, 10:54 a.m. UTC
From: Limin Wang <lance.lmwang@gmail.com>

Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
---
 libavcodec/hevc_sei.c               | 31 +++++++++++++++++++++++++++++++
 libavcodec/hevc_sei.h               |  6 ++++++
 libavcodec/hevcdec.c                | 14 ++++++++++++++
 tests/ref/fate/hevc-monochrome-crop |  3 +++
 4 files changed, 54 insertions(+)

Comments

Derek Buitenhuis March 17, 2020, 5:25 p.m. UTC | #1
On 17/03/2020 10:54, lance.lmwang@gmail.com wrote:
> From: Limin Wang <lance.lmwang@gmail.com>
> 
> Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
> ---
>  libavcodec/hevc_sei.c               | 31 +++++++++++++++++++++++++++++++
>  libavcodec/hevc_sei.h               |  6 ++++++
>  libavcodec/hevcdec.c                | 14 ++++++++++++++
>  tests/ref/fate/hevc-monochrome-crop |  3 +++
>  4 files changed, 54 insertions(+)

Apologies if I missed this in previous iterations.

What is the proposed usecase?

- Derek
Limin Wang March 17, 2020, 11:11 p.m. UTC | #2
On Tue, Mar 17, 2020 at 05:25:29PM +0000, Derek Buitenhuis wrote:
> On 17/03/2020 10:54, lance.lmwang@gmail.com wrote:
> > From: Limin Wang <lance.lmwang@gmail.com>
> > 
> > Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
> > ---
> >  libavcodec/hevc_sei.c               | 31 +++++++++++++++++++++++++++++++
> >  libavcodec/hevc_sei.h               |  6 ++++++
> >  libavcodec/hevcdec.c                | 14 ++++++++++++++
> >  tests/ref/fate/hevc-monochrome-crop |  3 +++
> >  4 files changed, 54 insertions(+)
> 
> Apologies if I missed this in previous iterations.
> 
> What is the proposed usecase?
The user data unregistered allows arbitrary data to be carried in the bitstream,
for example, ROI info, time info etc. For the real support patch, please refer to
the pending patch series 7, 8 in below link:
https://patchwork.ffmpeg.org/project/ffmpeg/patch/20200107050355.17503-1-lance.lmwang@gmail.com/


> 
> - Derek
> _______________________________________________
> 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".
Derek Buitenhuis March 18, 2020, 12:13 a.m. UTC | #3
On 17/03/2020 23:11, Limin Wang wrote:
> The user data unregistered allows arbitrary data to be carried in the bitstream,
> for example, ROI info, time info etc. For the real support patch, please refer to
> the pending patch series 7, 8 in below link:
> https://patchwork.ffmpeg.org/project/ffmpeg/patch/20200107050355.17503-1-lance.lmwang@gmail.com/

I understand what the SEI does. I was inquiring as to what the usecase you had
in mind for it specifically. I don't like the idea of adding APIs that are only
theorically useful - that is, what data is intended to be shoved in there?

- Derek
Limin Wang March 18, 2020, 2:37 a.m. UTC | #4
On Wed, Mar 18, 2020 at 12:13:33AM +0000, Derek Buitenhuis wrote:
> On 17/03/2020 23:11, Limin Wang wrote:
> > The user data unregistered allows arbitrary data to be carried in the bitstream,
> > for example, ROI info, time info etc. For the real support patch, please refer to
> > the pending patch series 7, 8 in below link:
> > https://patchwork.ffmpeg.org/project/ffmpeg/patch/20200107050355.17503-1-lance.lmwang@gmail.com/
> 
> I understand what the SEI does. I was inquiring as to what the usecase you had
> in mind for it specifically. I don't like the idea of adding APIs that are only
> theorically useful - that is, what data is intended to be shoved in there?

In order to identify the source of time used in the live event, the encoder system can
insert a time mode-source code along with each embedded precision time stamp. 
Each time mode-source code will be inserted into the H.264/H.265 bitstream as an SEI
message of type User Data (Unregistered). The format is self-defined json string 


> 
> - Derek
> _______________________________________________
> 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".
Kieran Kunhya March 18, 2020, 10:42 a.m. UTC | #5
On Wed, 18 Mar 2020 at 02:38, Limin Wang <lance.lmwang@gmail.com> wrote:

> On Wed, Mar 18, 2020 at 12:13:33AM +0000, Derek Buitenhuis wrote:
> > On 17/03/2020 23:11, Limin Wang wrote:
> > > The user data unregistered allows arbitrary data to be carried in the
> bitstream,
> > > for example, ROI info, time info etc. For the real support patch,
> please refer to
> > > the pending patch series 7, 8 in below link:
> > >
> https://patchwork.ffmpeg.org/project/ffmpeg/patch/20200107050355.17503-1-lance.lmwang@gmail.com/
> >
> > I understand what the SEI does. I was inquiring as to what the usecase
> you had
> > in mind for it specifically. I don't like the idea of adding APIs that
> are only
> > theorically useful - that is, what data is intended to be shoved in
> there?
>
> In order to identify the source of time used in the live event, the
> encoder system can
> insert a time mode-source code along with each embedded precision time
> stamp.
> Each time mode-source code will be inserted into the H.264/H.265 bitstream
> as an SEI
> message of type User Data (Unregistered). The format is self-defined json
> string
>

And the timecode field is not enough?
You do know that the time at source will not match the rate at which frames
arrive, there will be a drift?

Kieran
Limin Wang March 19, 2020, 1:42 a.m. UTC | #6
On Wed, Mar 18, 2020 at 10:42:12AM +0000, Kieran Kunhya wrote:
> On Wed, 18 Mar 2020 at 02:38, Limin Wang <lance.lmwang@gmail.com> wrote:
> 
> > On Wed, Mar 18, 2020 at 12:13:33AM +0000, Derek Buitenhuis wrote:
> > > On 17/03/2020 23:11, Limin Wang wrote:
> > > > The user data unregistered allows arbitrary data to be carried in the
> > bitstream,
> > > > for example, ROI info, time info etc. For the real support patch,
> > please refer to
> > > > the pending patch series 7, 8 in below link:
> > > >
> > https://patchwork.ffmpeg.org/project/ffmpeg/patch/20200107050355.17503-1-lance.lmwang@gmail.com/
> > >
> > > I understand what the SEI does. I was inquiring as to what the usecase
> > you had
> > > in mind for it specifically. I don't like the idea of adding APIs that
> > are only
> > > theorically useful - that is, what data is intended to be shoved in
> > there?
> >
> > In order to identify the source of time used in the live event, the
> > encoder system can
> > insert a time mode-source code along with each embedded precision time
> > stamp.
> > Each time mode-source code will be inserted into the H.264/H.265 bitstream
> > as an SEI
> > message of type User Data (Unregistered). The format is self-defined json
> > string
> >
> 
> And the timecode field is not enough?
I just give one use case for the unregistered user data. Actual We have more other private info
in the json string. timecode SEI is helpful, but it's not enough..

> You do know that the time at source will not match the rate at which frames
> arrive, there will be a drift?

Yes, we inserted the live time in the second level device, it's not require to be frame accurate.

> 
> Kieran
> _______________________________________________
> 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".
Limin Wang March 19, 2020, 1:46 a.m. UTC | #7
On Wed, Mar 18, 2020 at 11:12:54AM -0300, James Almer wrote:
> On 3/17/2020 11:37 PM, Limin Wang wrote:
> > On Wed, Mar 18, 2020 at 12:13:33AM +0000, Derek Buitenhuis wrote:
> >> On 17/03/2020 23:11, Limin Wang wrote:
> >>> The user data unregistered allows arbitrary data to be carried in the bitstream,
> >>> for example, ROI info, time info etc. For the real support patch, please refer to
> >>> the pending patch series 7, 8 in below link:
> >>> https://patchwork.ffmpeg.org/project/ffmpeg/patch/20200107050355.17503-1-lance.lmwang@gmail.com/
> >>
> >> I understand what the SEI does. I was inquiring as to what the usecase you had
> >> in mind for it specifically. I don't like the idea of adding APIs that are only
> >> theorically useful - that is, what data is intended to be shoved in there?
> > 
> > In order to identify the source of time used in the live event, the encoder system can
> > insert a time mode-source code along with each embedded precision time stamp. 
> > Each time mode-source code will be inserted into the H.264/H.265 bitstream as an SEI
> > message of type User Data (Unregistered). The format is self-defined json string 
> 
> You may want to look at the Producer Reference Time side data, then.

Thanks, I'll check it. But we have more private user data which I didn't want
to make them public..

> _______________________________________________
> 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/hevc_sei.c b/libavcodec/hevc_sei.c
index 6057069..e3d6bc9 100644
--- a/libavcodec/hevc_sei.c
+++ b/libavcodec/hevc_sei.c
@@ -213,6 +213,30 @@  static int decode_registered_user_data_closed_caption(HEVCSEIA53Caption *s, GetB
     return 0;
 }
 
+static int decode_nal_sei_user_data_unregistered(HEVCSEIUnregistered *s, GetBitContext *gb,
+                                                      int size)
+{
+    AVBufferRef *buf_ref, **tmp;
+
+    if (size < 16)
+       return AVERROR(EINVAL);
+
+    tmp = av_realloc_array(s->buf_ref, s->nb_buf_ref + 1, sizeof(*s->buf_ref));
+    if (!tmp)
+        return AVERROR(ENOMEM);
+    s->buf_ref = tmp;
+
+    buf_ref = av_buffer_alloc(size);
+    if (!buf_ref)
+        return AVERROR(ENOMEM);
+
+    for (int i = 0; i < size; i++)
+        buf_ref->data[i] = get_bits(gb, 8);
+    s->buf_ref[s->nb_buf_ref++] = buf_ref;
+
+    return 0;
+}
+
 static int decode_nal_sei_user_data_registered_itu_t_t35(HEVCSEI *s, GetBitContext *gb,
                                                          int size)
 {
@@ -300,6 +324,8 @@  static int decode_nal_sei_prefix(GetBitContext *gb, void *logctx, HEVCSEI *s,
         return decode_nal_sei_active_parameter_sets(s, gb, logctx);
     case HEVC_SEI_TYPE_USER_DATA_REGISTERED_ITU_T_T35:
         return decode_nal_sei_user_data_registered_itu_t_t35(s, gb, size);
+    case HEVC_SEI_TYPE_USER_DATA_UNREGISTERED:
+        return decode_nal_sei_user_data_unregistered(&s->unregistered, gb, size);
     case HEVC_SEI_TYPE_ALTERNATIVE_TRANSFER_CHARACTERISTICS:
         return decode_nal_sei_alternative_transfer(&s->alternative_transfer, gb);
     default:
@@ -371,4 +397,9 @@  int ff_hevc_decode_nal_sei(GetBitContext *gb, void *logctx, HEVCSEI *s,
 void ff_hevc_reset_sei(HEVCSEI *s)
 {
     av_buffer_unref(&s->a53_caption.buf_ref);
+
+    for (int i = 0; i < s->unregistered.nb_buf_ref; i++)
+        av_buffer_unref(&s->unregistered.buf_ref[i]);
+    s->unregistered.nb_buf_ref = 0;
+    av_freep(&s->unregistered.buf_ref);
 }
diff --git a/libavcodec/hevc_sei.h b/libavcodec/hevc_sei.h
index a44ccca..3618d16 100644
--- a/libavcodec/hevc_sei.h
+++ b/libavcodec/hevc_sei.h
@@ -91,6 +91,11 @@  typedef struct HEVCSEIA53Caption {
     AVBufferRef *buf_ref;
 } HEVCSEIA53Caption;
 
+typedef struct HEVCSEIUnregistered {
+    AVBufferRef **buf_ref;
+    int nb_buf_ref;
+} HEVCSEIUnregistered;
+
 typedef struct HEVCSEIMasteringDisplay {
     int present;
     uint16_t display_primaries[3][2];
@@ -116,6 +121,7 @@  typedef struct HEVCSEI {
     HEVCSEIDisplayOrientation display_orientation;
     HEVCSEIPictureTiming picture_timing;
     HEVCSEIA53Caption a53_caption;
+    HEVCSEIUnregistered unregistered;
     HEVCSEIMasteringDisplay mastering_display;
     HEVCSEIContentLight content_light;
     int active_seq_parameter_set_id;
diff --git a/libavcodec/hevcdec.c b/libavcodec/hevcdec.c
index 7448be4..8909aca 100644
--- a/libavcodec/hevcdec.c
+++ b/libavcodec/hevcdec.c
@@ -2795,6 +2795,20 @@  static int set_side_data(HEVCContext *s)
         s->avctx->properties |= FF_CODEC_PROPERTY_CLOSED_CAPTIONS;
     }
 
+    for (int i = 0; i < s->sei.unregistered.nb_buf_ref; i++) {
+        HEVCSEIUnregistered *unreg = &s->sei.unregistered;
+
+        if (unreg->buf_ref[i]) {
+            AVFrameSideData *sd = av_frame_new_side_data_from_buf(out,
+                    AV_FRAME_DATA_SEI_UNREGISTERED,
+                    unreg->buf_ref[i]);
+            if (!sd)
+                av_buffer_unref(&unreg->buf_ref[i]);
+            unreg->buf_ref[i] = NULL;
+        }
+    }
+    s->sei.unregistered.nb_buf_ref = 0;
+
     return 0;
 }
 
diff --git a/tests/ref/fate/hevc-monochrome-crop b/tests/ref/fate/hevc-monochrome-crop
index 4e45412..384404d 100644
--- a/tests/ref/fate/hevc-monochrome-crop
+++ b/tests/ref/fate/hevc-monochrome-crop
@@ -1,6 +1,9 @@ 
 [FRAME]
 width=384
 height=240
+[SIDE_DATA]
+side_data_type=H.26[45] User Data Unregistered SEI message
+[/SIDE_DATA]
 [/FRAME]
 [STREAM]
 width=384