diff mbox series

[FFmpeg-devel,v6] avutil: add AV_FRAME_DATA_USER_DATA_UNREGISTERED side data type

Message ID 20200106145406.9247-1-lance.lmwang@gmail.com
State Superseded
Headers show
Series [FFmpeg-devel,v6] avutil: add AV_FRAME_DATA_USER_DATA_UNREGISTERED side data type | expand

Checks

Context Check Description
andriy/ffmpeg-patchwork success Make fate finished

Commit Message

Lance Wang Jan. 6, 2020, 2:54 p.m. UTC 
From: Limin Wang <lance.lmwang@gmail.com>

Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
---
Fix for the v5 commit message change when copy(add avutil:)

 doc/APIchanges      | 3 +++
 libavutil/frame.c   | 1 +
 libavutil/frame.h   | 8 ++++++++
 libavutil/version.h | 2 +-
 4 files changed, 13 insertions(+), 1 deletion(-)

Comments

Mark Thompson Jan. 6, 2020, 10:03 p.m. UTC | #1 
On 06/01/2020 14:54, lance.lmwang@gmail.com wrote:
> From: Limin Wang <lance.lmwang@gmail.com>
> 
> Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
> ---
> Fix for the v5 commit message change when copy(add avutil:)
> 
>  doc/APIchanges      | 3 +++
>  libavutil/frame.c   | 1 +
>  libavutil/frame.h   | 8 ++++++++
>  libavutil/version.h | 2 +-
>  4 files changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/doc/APIchanges b/doc/APIchanges
> index 3c24dc6..6e1673e 100644
> --- a/doc/APIchanges
> +++ b/doc/APIchanges
> @@ -15,6 +15,9 @@ libavutil:     2017-10-21
>  
>  API changes, most recent first:
>  
> +2020-01-xx - xxxxxxxxxx - lavu 56.39.100 - frame.h
> +  Add AV_FRAME_DATA_USER_DATA_UNREGISTERED.
> +
>  2019-12-27 - xxxxxxxxxx - lavu 56.38.100 - eval.h
>    Add av_expr_count_func().
>  
> diff --git a/libavutil/frame.c b/libavutil/frame.c
> index e403809..1d0faec6 100644
> --- a/libavutil/frame.c
> +++ b/libavutil/frame.c
> @@ -842,6 +842,7 @@ const char *av_frame_side_data_name(enum AVFrameSideDataType type)
>  #endif
>      case AV_FRAME_DATA_DYNAMIC_HDR_PLUS: return "HDR Dynamic Metadata SMPTE2094-40 (HDR10+)";
>      case AV_FRAME_DATA_REGIONS_OF_INTEREST: return "Regions Of Interest";
> +    case AV_FRAME_DATA_USER_DATA_UNREGISTERED: return "User Data Unregistered";
>      }
>      return NULL;
>  }
> diff --git a/libavutil/frame.h b/libavutil/frame.h
> index b5afb58..9e8c3a9 100644
> --- a/libavutil/frame.h
> +++ b/libavutil/frame.h
> @@ -179,6 +179,14 @@ enum AVFrameSideDataType {
>       * array element is implied by AVFrameSideData.size / AVRegionOfInterest.self_size.
>       */
>      AV_FRAME_DATA_REGIONS_OF_INTEREST,
> +
> +    /**
> +     * User data unregistered metadata associated with a video frame.

IMO it should be stated directly that this is the H.26[45] UDU SEI message, and shouldn't be used for any other purpose.  Mention it here and reference the relevant sections of the standards.

> +     * This data payload is stored as uint8_t in AVFrameSideData.data.
> +     * The number of bytes of data payload is AVFrameSideData.size.
> +     * The data payload consists of 16 bytes UUID and real user data.

The ambiguity around the word payload is kindof confusing here.  I think either avoid using the word payload at all, or name the syntax elements explicitly (16 bytes of uuid_iso_iec_11578 followed by size-16 bytes of user_data_payload_byte).

> +     */
> +    AV_FRAME_DATA_USER_DATA_UNREGISTERED,
>  };
>  
>  enum AVActiveFormatDescription {
> diff --git a/libavutil/version.h b/libavutil/version.h
> index af8f614..2bc1b98 100644
> --- a/libavutil/version.h
> +++ b/libavutil/version.h
> @@ -79,7 +79,7 @@
>   */
>  
>  #define LIBAVUTIL_VERSION_MAJOR  56
> -#define LIBAVUTIL_VERSION_MINOR  38
> +#define LIBAVUTIL_VERSION_MINOR  39
>  #define LIBAVUTIL_VERSION_MICRO 100
>  
>  #define LIBAVUTIL_VERSION_INT   AV_VERSION_INT(LIBAVUTIL_VERSION_MAJOR, \
> 

Thanks,

- Mark
Lance Wang Jan. 7, 2020, 12:56 a.m. UTC | #2 
On Mon, Jan 06, 2020 at 10:03:42PM +0000, Mark Thompson wrote:
> On 06/01/2020 14:54, lance.lmwang@gmail.com wrote:
> > From: Limin Wang <lance.lmwang@gmail.com>
> > 
> > Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
> > ---
> > Fix for the v5 commit message change when copy(add avutil:)
> > 
> >  doc/APIchanges      | 3 +++
> >  libavutil/frame.c   | 1 +
> >  libavutil/frame.h   | 8 ++++++++
> >  libavutil/version.h | 2 +-
> >  4 files changed, 13 insertions(+), 1 deletion(-)
> > 
> > diff --git a/doc/APIchanges b/doc/APIchanges
> > index 3c24dc6..6e1673e 100644
> > --- a/doc/APIchanges
> > +++ b/doc/APIchanges
> > @@ -15,6 +15,9 @@ libavutil:     2017-10-21
> >  
> >  API changes, most recent first:
> >  
> > +2020-01-xx - xxxxxxxxxx - lavu 56.39.100 - frame.h
> > +  Add AV_FRAME_DATA_USER_DATA_UNREGISTERED.
> > +
> >  2019-12-27 - xxxxxxxxxx - lavu 56.38.100 - eval.h
> >    Add av_expr_count_func().
> >  
> > diff --git a/libavutil/frame.c b/libavutil/frame.c
> > index e403809..1d0faec6 100644
> > --- a/libavutil/frame.c
> > +++ b/libavutil/frame.c
> > @@ -842,6 +842,7 @@ const char *av_frame_side_data_name(enum AVFrameSideDataType type)
> >  #endif
> >      case AV_FRAME_DATA_DYNAMIC_HDR_PLUS: return "HDR Dynamic Metadata SMPTE2094-40 (HDR10+)";
> >      case AV_FRAME_DATA_REGIONS_OF_INTEREST: return "Regions Of Interest";
> > +    case AV_FRAME_DATA_USER_DATA_UNREGISTERED: return "User Data Unregistered";
> >      }
> >      return NULL;
> >  }
> > diff --git a/libavutil/frame.h b/libavutil/frame.h
> > index b5afb58..9e8c3a9 100644
> > --- a/libavutil/frame.h
> > +++ b/libavutil/frame.h
> > @@ -179,6 +179,14 @@ enum AVFrameSideDataType {
> >       * array element is implied by AVFrameSideData.size / AVRegionOfInterest.self_size.
> >       */
> >      AV_FRAME_DATA_REGIONS_OF_INTEREST,
> > +
> > +    /**
> > +     * User data unregistered metadata associated with a video frame.
> 
> IMO it should be stated directly that this is the H.26[45] UDU SEI message, and shouldn't be used for any other purpose.  Mention it here and reference the relevant sections of the standards.
OK

> 
> > +     * This data payload is stored as uint8_t in AVFrameSideData.data.
> > +     * The number of bytes of data payload is AVFrameSideData.size.
> > +     * The data payload consists of 16 bytes UUID and real user data.
> 
> The ambiguity around the word payload is kindof confusing here.  I think either avoid using the word payload at all, or name the syntax elements explicitly (16 bytes of uuid_iso_iec_11578 followed by size-16 bytes of user_data_payload_byte).

OK, will update by your suggestion.


> 
> > +     */
> > +    AV_FRAME_DATA_USER_DATA_UNREGISTERED,
> >  };
> >  
> >  enum AVActiveFormatDescription {
> > diff --git a/libavutil/version.h b/libavutil/version.h
> > index af8f614..2bc1b98 100644
> > --- a/libavutil/version.h
> > +++ b/libavutil/version.h
> > @@ -79,7 +79,7 @@
> >   */
> >  
> >  #define LIBAVUTIL_VERSION_MAJOR  56
> > -#define LIBAVUTIL_VERSION_MINOR  38
> > +#define LIBAVUTIL_VERSION_MINOR  39
> >  #define LIBAVUTIL_VERSION_MICRO 100
> >  
> >  #define LIBAVUTIL_VERSION_INT   AV_VERSION_INT(LIBAVUTIL_VERSION_MAJOR, \
> > 
> 
> Thanks,
> 
> - Mark
> _______________________________________________
> 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".
Lance Wang Jan. 7, 2020, 5:08 a.m. UTC | #3 
On Mon, Jan 06, 2020 at 10:03:42PM +0000, Mark Thompson wrote:
> On 06/01/2020 14:54, lance.lmwang@gmail.com wrote:
> > From: Limin Wang <lance.lmwang@gmail.com>
> > 
> > Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
> > ---
> > Fix for the v5 commit message change when copy(add avutil:)
> > 
> >  doc/APIchanges      | 3 +++
> >  libavutil/frame.c   | 1 +
> >  libavutil/frame.h   | 8 ++++++++
> >  libavutil/version.h | 2 +-
> >  4 files changed, 13 insertions(+), 1 deletion(-)
> > 
> > diff --git a/doc/APIchanges b/doc/APIchanges
> > index 3c24dc6..6e1673e 100644
> > --- a/doc/APIchanges
> > +++ b/doc/APIchanges
> > @@ -15,6 +15,9 @@ libavutil:     2017-10-21
> >  
> >  API changes, most recent first:
> >  
> > +2020-01-xx - xxxxxxxxxx - lavu 56.39.100 - frame.h
> > +  Add AV_FRAME_DATA_USER_DATA_UNREGISTERED.
> > +
> >  2019-12-27 - xxxxxxxxxx - lavu 56.38.100 - eval.h
> >    Add av_expr_count_func().
> >  
> > diff --git a/libavutil/frame.c b/libavutil/frame.c
> > index e403809..1d0faec6 100644
> > --- a/libavutil/frame.c
> > +++ b/libavutil/frame.c
> > @@ -842,6 +842,7 @@ const char *av_frame_side_data_name(enum AVFrameSideDataType type)
> >  #endif
> >      case AV_FRAME_DATA_DYNAMIC_HDR_PLUS: return "HDR Dynamic Metadata SMPTE2094-40 (HDR10+)";
> >      case AV_FRAME_DATA_REGIONS_OF_INTEREST: return "Regions Of Interest";
> > +    case AV_FRAME_DATA_USER_DATA_UNREGISTERED: return "User Data Unregistered";
> >      }
> >      return NULL;
> >  }
> > diff --git a/libavutil/frame.h b/libavutil/frame.h
> > index b5afb58..9e8c3a9 100644
> > --- a/libavutil/frame.h
> > +++ b/libavutil/frame.h
> > @@ -179,6 +179,14 @@ enum AVFrameSideDataType {
> >       * array element is implied by AVFrameSideData.size / AVRegionOfInterest.self_size.
> >       */
> >      AV_FRAME_DATA_REGIONS_OF_INTEREST,
> > +
> > +    /**
> > +     * User data unregistered metadata associated with a video frame.
> 
> IMO it should be stated directly that this is the H.26[45] UDU SEI message, and shouldn't be used for any other purpose.  Mention it here and reference the relevant sections of the standards.
> 
> > +     * This data payload is stored as uint8_t in AVFrameSideData.data.
> > +     * The number of bytes of data payload is AVFrameSideData.size.
> > +     * The data payload consists of 16 bytes UUID and real user data.
> 
> The ambiguity around the word payload is kindof confusing here.  I think either avoid using the word payload at all, or name the syntax elements explicitly (16 bytes of uuid_iso_iec_11578 followed by size-16 bytes of user_data_payload_byte).

Mark, I have updated the patch set by your comments, please help to review to
sure I have catch your points. If have any missing, please comments further.
In addition, I put other related patch into this thread for better review.

> 
> > +     */
> > +    AV_FRAME_DATA_USER_DATA_UNREGISTERED,
> >  };
> >  
> >  enum AVActiveFormatDescription {
> > diff --git a/libavutil/version.h b/libavutil/version.h
> > index af8f614..2bc1b98 100644
> > --- a/libavutil/version.h
> > +++ b/libavutil/version.h
> > @@ -79,7 +79,7 @@
> >   */
> >  
> >  #define LIBAVUTIL_VERSION_MAJOR  56
> > -#define LIBAVUTIL_VERSION_MINOR  38
> > +#define LIBAVUTIL_VERSION_MINOR  39
> >  #define LIBAVUTIL_VERSION_MICRO 100
> >  
> >  #define LIBAVUTIL_VERSION_INT   AV_VERSION_INT(LIBAVUTIL_VERSION_MAJOR, \
> > 
> 
> Thanks,
> 
> - Mark
> _______________________________________________
> 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/doc/APIchanges b/doc/APIchanges
index 3c24dc6..6e1673e 100644
--- a/doc/APIchanges
+++ b/doc/APIchanges
@@ -15,6 +15,9 @@  libavutil:     2017-10-21
 
 API changes, most recent first:
 
+2020-01-xx - xxxxxxxxxx - lavu 56.39.100 - frame.h
+  Add AV_FRAME_DATA_USER_DATA_UNREGISTERED.
+
 2019-12-27 - xxxxxxxxxx - lavu 56.38.100 - eval.h
   Add av_expr_count_func().
 
diff --git a/libavutil/frame.c b/libavutil/frame.c
index e403809..1d0faec6 100644
--- a/libavutil/frame.c
+++ b/libavutil/frame.c
@@ -842,6 +842,7 @@  const char *av_frame_side_data_name(enum AVFrameSideDataType type)
 #endif
     case AV_FRAME_DATA_DYNAMIC_HDR_PLUS: return "HDR Dynamic Metadata SMPTE2094-40 (HDR10+)";
     case AV_FRAME_DATA_REGIONS_OF_INTEREST: return "Regions Of Interest";
+    case AV_FRAME_DATA_USER_DATA_UNREGISTERED: return "User Data Unregistered";
     }
     return NULL;
 }
diff --git a/libavutil/frame.h b/libavutil/frame.h
index b5afb58..9e8c3a9 100644
--- a/libavutil/frame.h
+++ b/libavutil/frame.h
@@ -179,6 +179,14 @@  enum AVFrameSideDataType {
      * array element is implied by AVFrameSideData.size / AVRegionOfInterest.self_size.
      */
     AV_FRAME_DATA_REGIONS_OF_INTEREST,
+
+    /**
+     * User data unregistered metadata associated with a video frame.
+     * This data payload is stored as uint8_t in AVFrameSideData.data.
+     * The number of bytes of data payload is AVFrameSideData.size.
+     * The data payload consists of 16 bytes UUID and real user data.
+     */
+    AV_FRAME_DATA_USER_DATA_UNREGISTERED,
 };
 
 enum AVActiveFormatDescription {
diff --git a/libavutil/version.h b/libavutil/version.h
index af8f614..2bc1b98 100644
--- a/libavutil/version.h
+++ b/libavutil/version.h
@@ -79,7 +79,7 @@ 
  */
 
 #define LIBAVUTIL_VERSION_MAJOR  56
-#define LIBAVUTIL_VERSION_MINOR  38
+#define LIBAVUTIL_VERSION_MINOR  39
 #define LIBAVUTIL_VERSION_MICRO 100
 
 #define LIBAVUTIL_VERSION_INT   AV_VERSION_INT(LIBAVUTIL_VERSION_MAJOR, \