Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
 help / color / mirror / Atom feed
* [FFmpeg-devel] [PATCH 1/4] lavu/frame: add side data descriptors
@ 2024-03-23 13:04 Anton Khirnov
  2024-03-23 13:04 ` [FFmpeg-devel] [PATCH 2/4] fftools/ffmpeg_enc: only promote first frame side data to global when meaningful Anton Khirnov
                   ` (3 more replies)
  0 siblings, 4 replies; 8+ messages in thread
From: Anton Khirnov @ 2024-03-23 13:04 UTC (permalink / raw)
  To: ffmpeg-devel

They allow exporting extended information about side data types.
---
 doc/APIchanges      |  4 +++
 libavutil/frame.c   | 70 ++++++++++++++++++++++++++-------------------
 libavutil/frame.h   | 27 +++++++++++++++++
 libavutil/version.h |  2 +-
 4 files changed, 72 insertions(+), 31 deletions(-)

diff --git a/doc/APIchanges b/doc/APIchanges
index a025f1df14..8794380e11 100644
--- a/doc/APIchanges
+++ b/doc/APIchanges
@@ -2,6 +2,10 @@ The last version increases of all libraries were on 2024-03-07
 
 API changes, most recent first:
 
+2024-03-xx - xxxxxxxxxx - lavu 59.4.100 - frame.h
+  Add AVSideDataDescriptor, enum AVSideDataProps, and
+  av_frame_side_data_desc().
+
 2024-03-xx - xxxxxxxxxx - lavc 61.2.100 - avcodec.h
   Add AVCodecContext.[nb_]decoded_side_data.
 
diff --git a/libavutil/frame.c b/libavutil/frame.c
index 89db687d9c..cb9af6326d 100644
--- a/libavutil/frame.c
+++ b/libavutil/frame.c
@@ -28,6 +28,36 @@
 #include "samplefmt.h"
 #include "hwcontext.h"
 
+static const AVSideDataDescriptor sd_props[] = {
+    [AV_FRAME_DATA_PANSCAN]                     = { "AVPanScan" },
+    [AV_FRAME_DATA_A53_CC]                      = { "ATSC A53 Part 4 Closed Captions" },
+    [AV_FRAME_DATA_MATRIXENCODING]              = { "AVMatrixEncoding" },
+    [AV_FRAME_DATA_DOWNMIX_INFO]                = { "Metadata relevant to a downmix procedure" },
+    [AV_FRAME_DATA_AFD]                         = { "Active format description" },
+    [AV_FRAME_DATA_MOTION_VECTORS]              = { "Motion vectors" },
+    [AV_FRAME_DATA_SKIP_SAMPLES]                = { "Skip samples" },
+    [AV_FRAME_DATA_GOP_TIMECODE]                = { "GOP timecode" },
+    [AV_FRAME_DATA_S12M_TIMECODE]               = { "SMPTE 12-1 timecode" },
+    [AV_FRAME_DATA_DYNAMIC_HDR_PLUS]            = { "HDR Dynamic Metadata SMPTE2094-40 (HDR10+)" },
+    [AV_FRAME_DATA_DYNAMIC_HDR_VIVID]           = { "HDR Dynamic Metadata CUVA 005.1 2021 (Vivid)" },
+    [AV_FRAME_DATA_REGIONS_OF_INTEREST]         = { "Regions Of Interest" },
+    [AV_FRAME_DATA_VIDEO_ENC_PARAMS]            = { "Video encoding parameters" },
+    [AV_FRAME_DATA_FILM_GRAIN_PARAMS]           = { "Film grain parameters" },
+    [AV_FRAME_DATA_DETECTION_BBOXES]            = { "Bounding boxes for object detection and classification" },
+    [AV_FRAME_DATA_DOVI_RPU_BUFFER]             = { "Dolby Vision RPU Data" },
+    [AV_FRAME_DATA_DOVI_METADATA]               = { "Dolby Vision Metadata" },
+    [AV_FRAME_DATA_STEREO3D]                    = { "Stereo 3D",                                    AV_SIDE_DATA_PROP_GLOBAL },
+    [AV_FRAME_DATA_REPLAYGAIN]                  = { "AVReplayGain",                                 AV_SIDE_DATA_PROP_GLOBAL },
+    [AV_FRAME_DATA_DISPLAYMATRIX]               = { "3x3 displaymatrix",                            AV_SIDE_DATA_PROP_GLOBAL },
+    [AV_FRAME_DATA_AUDIO_SERVICE_TYPE]          = { "Audio service type",                           AV_SIDE_DATA_PROP_GLOBAL },
+    [AV_FRAME_DATA_MASTERING_DISPLAY_METADATA]  = { "Mastering display metadata",                   AV_SIDE_DATA_PROP_GLOBAL },
+    [AV_FRAME_DATA_CONTENT_LIGHT_LEVEL]         = { "Content light level metadata",                 AV_SIDE_DATA_PROP_GLOBAL },
+    [AV_FRAME_DATA_AMBIENT_VIEWING_ENVIRONMENT] = { "Ambient viewing environment",                  AV_SIDE_DATA_PROP_GLOBAL },
+    [AV_FRAME_DATA_SPHERICAL]                   = { "Spherical Mapping",                            AV_SIDE_DATA_PROP_GLOBAL },
+    [AV_FRAME_DATA_ICC_PROFILE]                 = { "ICC profile",                                  AV_SIDE_DATA_PROP_GLOBAL },
+    [AV_FRAME_DATA_SEI_UNREGISTERED]            = { "H.26[45] User Data Unregistered SEI message",  AV_SIDE_DATA_PROP_MULTI },
+};
+
 static void get_frame_defaults(AVFrame *frame)
 {
     memset(frame, 0, sizeof(*frame));
@@ -895,38 +925,18 @@ void av_frame_remove_side_data(AVFrame *frame, enum AVFrameSideDataType type)
     remove_side_data(&frame->side_data, &frame->nb_side_data, type);
 }
 
+const AVSideDataDescriptor *av_frame_side_data_desc(enum AVFrameSideDataType type)
+{
+    unsigned t = type;
+    if (t < FF_ARRAY_ELEMS(sd_props) && sd_props[t].name)
+        return &sd_props[t];
+    return NULL;
+}
+
 const char *av_frame_side_data_name(enum AVFrameSideDataType type)
 {
-    switch(type) {
-    case AV_FRAME_DATA_PANSCAN:         return "AVPanScan";
-    case AV_FRAME_DATA_A53_CC:          return "ATSC A53 Part 4 Closed Captions";
-    case AV_FRAME_DATA_STEREO3D:        return "Stereo 3D";
-    case AV_FRAME_DATA_MATRIXENCODING:  return "AVMatrixEncoding";
-    case AV_FRAME_DATA_DOWNMIX_INFO:    return "Metadata relevant to a downmix procedure";
-    case AV_FRAME_DATA_REPLAYGAIN:      return "AVReplayGain";
-    case AV_FRAME_DATA_DISPLAYMATRIX:   return "3x3 displaymatrix";
-    case AV_FRAME_DATA_AFD:             return "Active format description";
-    case AV_FRAME_DATA_MOTION_VECTORS:  return "Motion vectors";
-    case AV_FRAME_DATA_SKIP_SAMPLES:    return "Skip samples";
-    case AV_FRAME_DATA_AUDIO_SERVICE_TYPE:          return "Audio service type";
-    case AV_FRAME_DATA_MASTERING_DISPLAY_METADATA:  return "Mastering display metadata";
-    case AV_FRAME_DATA_CONTENT_LIGHT_LEVEL:         return "Content light level metadata";
-    case AV_FRAME_DATA_GOP_TIMECODE:                return "GOP timecode";
-    case AV_FRAME_DATA_S12M_TIMECODE:               return "SMPTE 12-1 timecode";
-    case AV_FRAME_DATA_SPHERICAL:                   return "Spherical Mapping";
-    case AV_FRAME_DATA_ICC_PROFILE:                 return "ICC profile";
-    case AV_FRAME_DATA_DYNAMIC_HDR_PLUS: return "HDR Dynamic Metadata SMPTE2094-40 (HDR10+)";
-    case AV_FRAME_DATA_DYNAMIC_HDR_VIVID: return "HDR Dynamic Metadata CUVA 005.1 2021 (Vivid)";
-    case AV_FRAME_DATA_REGIONS_OF_INTEREST: return "Regions Of Interest";
-    case AV_FRAME_DATA_VIDEO_ENC_PARAMS:            return "Video encoding parameters";
-    case AV_FRAME_DATA_SEI_UNREGISTERED:            return "H.26[45] User Data Unregistered SEI message";
-    case AV_FRAME_DATA_FILM_GRAIN_PARAMS:           return "Film grain parameters";
-    case AV_FRAME_DATA_DETECTION_BBOXES:            return "Bounding boxes for object detection and classification";
-    case AV_FRAME_DATA_DOVI_RPU_BUFFER:             return "Dolby Vision RPU Data";
-    case AV_FRAME_DATA_DOVI_METADATA:               return "Dolby Vision Metadata";
-    case AV_FRAME_DATA_AMBIENT_VIEWING_ENVIRONMENT: return "Ambient viewing environment";
-    }
-    return NULL;
+    const AVSideDataDescriptor *desc = av_frame_side_data_desc(type);
+    return desc ? desc->name : NULL;
 }
 
 static int calc_cropping_offsets(size_t offsets[4], const AVFrame *frame,
diff --git a/libavutil/frame.h b/libavutil/frame.h
index a7fc909ad8..aa614c3dc6 100644
--- a/libavutil/frame.h
+++ b/libavutil/frame.h
@@ -251,6 +251,27 @@ typedef struct AVFrameSideData {
     AVBufferRef *buf;
 } AVFrameSideData;
 
+enum AVSideDataProps {
+    AV_SIDE_DATA_PROP_GLOBAL = (1 << 0),
+    AV_SIDE_DATA_PROP_MULTI  = (1 << 1),
+};
+
+/**
+ * This struct describes the properties of a side data type. Its instance
+ * corresponding to a given type can be obtained from av_frame_side_data_desc().
+ */
+typedef struct AVSideDataDescriptor {
+    /**
+     * Human-readable side data description.
+     */
+    const char      *name;
+
+    /**
+     * Side data property flags, a combination of AVSideDataProps values.
+     */
+    unsigned         props;
+} AVSideDataDescriptor;
+
 /**
  * Structure describing a single Region Of Interest.
  *
@@ -988,6 +1009,12 @@ int av_frame_apply_cropping(AVFrame *frame, int flags);
  */
 const char *av_frame_side_data_name(enum AVFrameSideDataType type);
 
+/**
+ * @return side data descriptor corresponding to a given side data type, NULL
+ *         when not available.
+ */
+const AVSideDataDescriptor *av_frame_side_data_desc(enum AVFrameSideDataType type);
+
 /**
  * Free all side data entries and their contents, then zeroes out the
  * values which the pointers are pointing to.
diff --git a/libavutil/version.h b/libavutil/version.h
index 5027b025be..882003f719 100644
--- a/libavutil/version.h
+++ b/libavutil/version.h
@@ -79,7 +79,7 @@
  */
 
 #define LIBAVUTIL_VERSION_MAJOR  59
-#define LIBAVUTIL_VERSION_MINOR   3
+#define LIBAVUTIL_VERSION_MINOR   4
 #define LIBAVUTIL_VERSION_MICRO 100
 
 #define LIBAVUTIL_VERSION_INT   AV_VERSION_INT(LIBAVUTIL_VERSION_MAJOR, \
-- 
2.43.0

_______________________________________________
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".

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [FFmpeg-devel] [PATCH 2/4] fftools/ffmpeg_enc: only promote first frame side data to global when meaningful
  2024-03-23 13:04 [FFmpeg-devel] [PATCH 1/4] lavu/frame: add side data descriptors Anton Khirnov
@ 2024-03-23 13:04 ` Anton Khirnov
  2024-03-23 13:15   ` Andreas Rheinhardt
  2024-03-23 13:04 ` [FFmpeg-devel] [PATCH 3/4] fftools/ffmpeg_enc: move decoded_side_data setup out of video-only block Anton Khirnov
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 8+ messages in thread
From: Anton Khirnov @ 2024-03-23 13:04 UTC (permalink / raw)
  To: ffmpeg-devel

Skip those side data types that do not make sense as global side data.
---
 fftools/ffmpeg_enc.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/fftools/ffmpeg_enc.c b/fftools/ffmpeg_enc.c
index f01be1c22f..6a91fd0398 100644
--- a/fftools/ffmpeg_enc.c
+++ b/fftools/ffmpeg_enc.c
@@ -247,6 +247,11 @@ int enc_open(void *opaque, const AVFrame *frame)
         enc_ctx->chroma_sample_location = frame->chroma_location;
 
         for (int i = 0; i < frame->nb_side_data; i++) {
+            const AVSideDataDescriptor *desc = av_frame_side_data_desc(frame->side_data[i]->type);
+
+            if (!desc || !(desc->props & AV_SIDE_DATA_PROP_GLOBAL))
+                continue;
+
             ret = av_frame_side_data_clone(
                 &enc_ctx->decoded_side_data, &enc_ctx->nb_decoded_side_data,
                 frame->side_data[i], AV_FRAME_SIDE_DATA_FLAG_UNIQUE);
-- 
2.43.0

_______________________________________________
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".

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [FFmpeg-devel] [PATCH 3/4] fftools/ffmpeg_enc: move decoded_side_data setup out of video-only block
  2024-03-23 13:04 [FFmpeg-devel] [PATCH 1/4] lavu/frame: add side data descriptors Anton Khirnov
  2024-03-23 13:04 ` [FFmpeg-devel] [PATCH 2/4] fftools/ffmpeg_enc: only promote first frame side data to global when meaningful Anton Khirnov
@ 2024-03-23 13:04 ` Anton Khirnov
  2024-03-23 13:04 ` [FFmpeg-devel] [PATCH 4/4] fftools/ffmpeg_enc: simplify error handling for decoded_side_data setup Anton Khirnov
  2024-03-26 21:38 ` [FFmpeg-devel] [PATCH 1/4] lavu/frame: add side data descriptors James Almer
  3 siblings, 0 replies; 8+ messages in thread
From: Anton Khirnov @ 2024-03-23 13:04 UTC (permalink / raw)
  To: ffmpeg-devel

Nothing about this code is video-specific.
---
 fftools/ffmpeg_enc.c | 41 +++++++++++++++++++++--------------------
 1 file changed, 21 insertions(+), 20 deletions(-)

diff --git a/fftools/ffmpeg_enc.c b/fftools/ffmpeg_enc.c
index 6a91fd0398..61310c67f1 100644
--- a/fftools/ffmpeg_enc.c
+++ b/fftools/ffmpeg_enc.c
@@ -187,6 +187,27 @@ int enc_open(void *opaque, const AVFrame *frame)
     if (frame) {
         av_assert0(frame->opaque_ref);
         fd = (FrameData*)frame->opaque_ref->data;
+
+        for (int i = 0; i < frame->nb_side_data; i++) {
+            const AVSideDataDescriptor *desc = av_frame_side_data_desc(frame->side_data[i]->type);
+
+            if (!desc || !(desc->props & AV_SIDE_DATA_PROP_GLOBAL))
+                continue;
+
+            ret = av_frame_side_data_clone(&enc_ctx->decoded_side_data,
+                                           &enc_ctx->nb_decoded_side_data,
+                                           frame->side_data[i],
+                                           AV_FRAME_SIDE_DATA_FLAG_UNIQUE);
+            if (ret < 0) {
+                av_frame_side_data_free(
+                    &enc_ctx->decoded_side_data,
+                    &enc_ctx->nb_decoded_side_data);
+                av_log(NULL, AV_LOG_ERROR,
+                        "failed to configure video encoder: %s!\n",
+                        av_err2str(ret));
+                return ret;
+            }
+        }
     }
 
     ret = set_encoder_id(of, ost);
@@ -246,26 +267,6 @@ int enc_open(void *opaque, const AVFrame *frame)
         enc_ctx->colorspace             = frame->colorspace;
         enc_ctx->chroma_sample_location = frame->chroma_location;
 
-        for (int i = 0; i < frame->nb_side_data; i++) {
-            const AVSideDataDescriptor *desc = av_frame_side_data_desc(frame->side_data[i]->type);
-
-            if (!desc || !(desc->props & AV_SIDE_DATA_PROP_GLOBAL))
-                continue;
-
-            ret = av_frame_side_data_clone(
-                &enc_ctx->decoded_side_data, &enc_ctx->nb_decoded_side_data,
-                frame->side_data[i], AV_FRAME_SIDE_DATA_FLAG_UNIQUE);
-            if (ret < 0) {
-                av_frame_side_data_free(
-                    &enc_ctx->decoded_side_data,
-                    &enc_ctx->nb_decoded_side_data);
-                av_log(NULL, AV_LOG_ERROR,
-                        "failed to configure video encoder: %s!\n",
-                        av_err2str(ret));
-                return ret;
-            }
-        }
-
         if (enc_ctx->flags & (AV_CODEC_FLAG_INTERLACED_DCT | AV_CODEC_FLAG_INTERLACED_ME) ||
             (frame->flags & AV_FRAME_FLAG_INTERLACED)
 #if FFMPEG_OPT_TOP
-- 
2.43.0

_______________________________________________
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".

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [FFmpeg-devel] [PATCH 4/4] fftools/ffmpeg_enc: simplify error handling for decoded_side_data setup
  2024-03-23 13:04 [FFmpeg-devel] [PATCH 1/4] lavu/frame: add side data descriptors Anton Khirnov
  2024-03-23 13:04 ` [FFmpeg-devel] [PATCH 2/4] fftools/ffmpeg_enc: only promote first frame side data to global when meaningful Anton Khirnov
  2024-03-23 13:04 ` [FFmpeg-devel] [PATCH 3/4] fftools/ffmpeg_enc: move decoded_side_data setup out of video-only block Anton Khirnov
@ 2024-03-23 13:04 ` Anton Khirnov
  2024-03-26 21:38 ` [FFmpeg-devel] [PATCH 1/4] lavu/frame: add side data descriptors James Almer
  3 siblings, 0 replies; 8+ messages in thread
From: Anton Khirnov @ 2024-03-23 13:04 UTC (permalink / raw)
  To: ffmpeg-devel

There is no need to free the already-added items, they will be freed
alongside the codec context. There is also little point in an error
message, as the only reason this can fail is malloc failure.
---
 fftools/ffmpeg_enc.c | 9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/fftools/ffmpeg_enc.c b/fftools/ffmpeg_enc.c
index 61310c67f1..9103fb74c0 100644
--- a/fftools/ffmpeg_enc.c
+++ b/fftools/ffmpeg_enc.c
@@ -198,15 +198,8 @@ int enc_open(void *opaque, const AVFrame *frame)
                                            &enc_ctx->nb_decoded_side_data,
                                            frame->side_data[i],
                                            AV_FRAME_SIDE_DATA_FLAG_UNIQUE);
-            if (ret < 0) {
-                av_frame_side_data_free(
-                    &enc_ctx->decoded_side_data,
-                    &enc_ctx->nb_decoded_side_data);
-                av_log(NULL, AV_LOG_ERROR,
-                        "failed to configure video encoder: %s!\n",
-                        av_err2str(ret));
+            if (ret < 0)
                 return ret;
-            }
         }
     }
 
-- 
2.43.0

_______________________________________________
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".

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [FFmpeg-devel] [PATCH 2/4] fftools/ffmpeg_enc: only promote first frame side data to global when meaningful
  2024-03-23 13:04 ` [FFmpeg-devel] [PATCH 2/4] fftools/ffmpeg_enc: only promote first frame side data to global when meaningful Anton Khirnov
@ 2024-03-23 13:15   ` Andreas Rheinhardt
  2024-03-23 13:19     ` Anton Khirnov
  0 siblings, 1 reply; 8+ messages in thread
From: Andreas Rheinhardt @ 2024-03-23 13:15 UTC (permalink / raw)
  To: ffmpeg-devel

Anton Khirnov:
> Skip those side data types that do not make sense as global side data.
> ---
>  fftools/ffmpeg_enc.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/fftools/ffmpeg_enc.c b/fftools/ffmpeg_enc.c
> index f01be1c22f..6a91fd0398 100644
> --- a/fftools/ffmpeg_enc.c
> +++ b/fftools/ffmpeg_enc.c
> @@ -247,6 +247,11 @@ int enc_open(void *opaque, const AVFrame *frame)
>          enc_ctx->chroma_sample_location = frame->chroma_location;
>  
>          for (int i = 0; i < frame->nb_side_data; i++) {
> +            const AVSideDataDescriptor *desc = av_frame_side_data_desc(frame->side_data[i]->type);
> +
> +            if (!desc || !(desc->props & AV_SIDE_DATA_PROP_GLOBAL))
> +                continue;

Why the first check? Is it intended that a defined side data type
doesn't have a descriptor? Because IIRC all side data types that can
occur here are defined and have not been created by letting the user
pass a number via options.

> +
>              ret = av_frame_side_data_clone(
>                  &enc_ctx->decoded_side_data, &enc_ctx->nb_decoded_side_data,
>                  frame->side_data[i], AV_FRAME_SIDE_DATA_FLAG_UNIQUE);

_______________________________________________
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".

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [FFmpeg-devel] [PATCH 2/4] fftools/ffmpeg_enc: only promote first frame side data to global when meaningful
  2024-03-23 13:15   ` Andreas Rheinhardt
@ 2024-03-23 13:19     ` Anton Khirnov
  2024-03-23 13:21       ` Andreas Rheinhardt
  0 siblings, 1 reply; 8+ messages in thread
From: Anton Khirnov @ 2024-03-23 13:19 UTC (permalink / raw)
  To: FFmpeg development discussions and patches

Quoting Andreas Rheinhardt (2024-03-23 14:15:06)
> Anton Khirnov:
> > Skip those side data types that do not make sense as global side data.
> > ---
> >  fftools/ffmpeg_enc.c | 5 +++++
> >  1 file changed, 5 insertions(+)
> > 
> > diff --git a/fftools/ffmpeg_enc.c b/fftools/ffmpeg_enc.c
> > index f01be1c22f..6a91fd0398 100644
> > --- a/fftools/ffmpeg_enc.c
> > +++ b/fftools/ffmpeg_enc.c
> > @@ -247,6 +247,11 @@ int enc_open(void *opaque, const AVFrame *frame)
> >          enc_ctx->chroma_sample_location = frame->chroma_location;
> >  
> >          for (int i = 0; i < frame->nb_side_data; i++) {
> > +            const AVSideDataDescriptor *desc = av_frame_side_data_desc(frame->side_data[i]->type);
> > +
> > +            if (!desc || !(desc->props & AV_SIDE_DATA_PROP_GLOBAL))
> > +                continue;
> 
> Why the first check? Is it intended that a defined side data type
> doesn't have a descriptor? Because IIRC all side data types that can
> occur here are defined and have not been created by letting the user
> pass a number via options.

It shouldn't happen, but it's not inconceivable that e.g. a filter could
attach side data with an unknown type to a frame. I can remove the check
if you prefer it.

-- 
Anton Khirnov
_______________________________________________
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".

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [FFmpeg-devel] [PATCH 2/4] fftools/ffmpeg_enc: only promote first frame side data to global when meaningful
  2024-03-23 13:19     ` Anton Khirnov
@ 2024-03-23 13:21       ` Andreas Rheinhardt
  0 siblings, 0 replies; 8+ messages in thread
From: Andreas Rheinhardt @ 2024-03-23 13:21 UTC (permalink / raw)
  To: ffmpeg-devel

Anton Khirnov:
> Quoting Andreas Rheinhardt (2024-03-23 14:15:06)
>> Anton Khirnov:
>>> Skip those side data types that do not make sense as global side data.
>>> ---
>>>  fftools/ffmpeg_enc.c | 5 +++++
>>>  1 file changed, 5 insertions(+)
>>>
>>> diff --git a/fftools/ffmpeg_enc.c b/fftools/ffmpeg_enc.c
>>> index f01be1c22f..6a91fd0398 100644
>>> --- a/fftools/ffmpeg_enc.c
>>> +++ b/fftools/ffmpeg_enc.c
>>> @@ -247,6 +247,11 @@ int enc_open(void *opaque, const AVFrame *frame)
>>>          enc_ctx->chroma_sample_location = frame->chroma_location;
>>>  
>>>          for (int i = 0; i < frame->nb_side_data; i++) {
>>> +            const AVSideDataDescriptor *desc = av_frame_side_data_desc(frame->side_data[i]->type);
>>> +
>>> +            if (!desc || !(desc->props & AV_SIDE_DATA_PROP_GLOBAL))
>>> +                continue;
>>
>> Why the first check? Is it intended that a defined side data type
>> doesn't have a descriptor? Because IIRC all side data types that can
>> occur here are defined and have not been created by letting the user
>> pass a number via options.
> 
> It shouldn't happen, but it's not inconceivable that e.g. a filter could
> attach side data with an unknown type to a frame. I can remove the check
> if you prefer it.
> 

Yes. I'd consider such a filter buggy and that bug should not be covered up.

- Andreas

_______________________________________________
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".

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [FFmpeg-devel] [PATCH 1/4] lavu/frame: add side data descriptors
  2024-03-23 13:04 [FFmpeg-devel] [PATCH 1/4] lavu/frame: add side data descriptors Anton Khirnov
                   ` (2 preceding siblings ...)
  2024-03-23 13:04 ` [FFmpeg-devel] [PATCH 4/4] fftools/ffmpeg_enc: simplify error handling for decoded_side_data setup Anton Khirnov
@ 2024-03-26 21:38 ` James Almer
  3 siblings, 0 replies; 8+ messages in thread
From: James Almer @ 2024-03-26 21:38 UTC (permalink / raw)
  To: ffmpeg-devel

On 3/23/2024 10:04 AM, Anton Khirnov wrote:
> They allow exporting extended information about side data types.
> ---
>   doc/APIchanges      |  4 +++
>   libavutil/frame.c   | 70 ++++++++++++++++++++++++++-------------------
>   libavutil/frame.h   | 27 +++++++++++++++++
>   libavutil/version.h |  2 +-
>   4 files changed, 72 insertions(+), 31 deletions(-)
> 
> diff --git a/doc/APIchanges b/doc/APIchanges
> index a025f1df14..8794380e11 100644
> --- a/doc/APIchanges
> +++ b/doc/APIchanges
> @@ -2,6 +2,10 @@ The last version increases of all libraries were on 2024-03-07
>   
>   API changes, most recent first:
>   
> +2024-03-xx - xxxxxxxxxx - lavu 59.4.100 - frame.h
> +  Add AVSideDataDescriptor, enum AVSideDataProps, and
> +  av_frame_side_data_desc().
> +
>   2024-03-xx - xxxxxxxxxx - lavc 61.2.100 - avcodec.h
>     Add AVCodecContext.[nb_]decoded_side_data.
>   
> diff --git a/libavutil/frame.c b/libavutil/frame.c
> index 89db687d9c..cb9af6326d 100644
> --- a/libavutil/frame.c
> +++ b/libavutil/frame.c
> @@ -28,6 +28,36 @@
>   #include "samplefmt.h"
>   #include "hwcontext.h"
>   
> +static const AVSideDataDescriptor sd_props[] = {
> +    [AV_FRAME_DATA_PANSCAN]                     = { "AVPanScan" },
> +    [AV_FRAME_DATA_A53_CC]                      = { "ATSC A53 Part 4 Closed Captions" },
> +    [AV_FRAME_DATA_MATRIXENCODING]              = { "AVMatrixEncoding" },
> +    [AV_FRAME_DATA_DOWNMIX_INFO]                = { "Metadata relevant to a downmix procedure" },
> +    [AV_FRAME_DATA_AFD]                         = { "Active format description" },
> +    [AV_FRAME_DATA_MOTION_VECTORS]              = { "Motion vectors" },
> +    [AV_FRAME_DATA_SKIP_SAMPLES]                = { "Skip samples" },
> +    [AV_FRAME_DATA_GOP_TIMECODE]                = { "GOP timecode" },
> +    [AV_FRAME_DATA_S12M_TIMECODE]               = { "SMPTE 12-1 timecode" },
> +    [AV_FRAME_DATA_DYNAMIC_HDR_PLUS]            = { "HDR Dynamic Metadata SMPTE2094-40 (HDR10+)" },
> +    [AV_FRAME_DATA_DYNAMIC_HDR_VIVID]           = { "HDR Dynamic Metadata CUVA 005.1 2021 (Vivid)" },
> +    [AV_FRAME_DATA_REGIONS_OF_INTEREST]         = { "Regions Of Interest" },
> +    [AV_FRAME_DATA_VIDEO_ENC_PARAMS]            = { "Video encoding parameters" },
> +    [AV_FRAME_DATA_FILM_GRAIN_PARAMS]           = { "Film grain parameters" },
> +    [AV_FRAME_DATA_DETECTION_BBOXES]            = { "Bounding boxes for object detection and classification" },
> +    [AV_FRAME_DATA_DOVI_RPU_BUFFER]             = { "Dolby Vision RPU Data" },
> +    [AV_FRAME_DATA_DOVI_METADATA]               = { "Dolby Vision Metadata" },
> +    [AV_FRAME_DATA_STEREO3D]                    = { "Stereo 3D",                                    AV_SIDE_DATA_PROP_GLOBAL },
> +    [AV_FRAME_DATA_REPLAYGAIN]                  = { "AVReplayGain",                                 AV_SIDE_DATA_PROP_GLOBAL },
> +    [AV_FRAME_DATA_DISPLAYMATRIX]               = { "3x3 displaymatrix",                            AV_SIDE_DATA_PROP_GLOBAL },
> +    [AV_FRAME_DATA_AUDIO_SERVICE_TYPE]          = { "Audio service type",                           AV_SIDE_DATA_PROP_GLOBAL },
> +    [AV_FRAME_DATA_MASTERING_DISPLAY_METADATA]  = { "Mastering display metadata",                   AV_SIDE_DATA_PROP_GLOBAL },
> +    [AV_FRAME_DATA_CONTENT_LIGHT_LEVEL]         = { "Content light level metadata",                 AV_SIDE_DATA_PROP_GLOBAL },
> +    [AV_FRAME_DATA_AMBIENT_VIEWING_ENVIRONMENT] = { "Ambient viewing environment",                  AV_SIDE_DATA_PROP_GLOBAL },
> +    [AV_FRAME_DATA_SPHERICAL]                   = { "Spherical Mapping",                            AV_SIDE_DATA_PROP_GLOBAL },
> +    [AV_FRAME_DATA_ICC_PROFILE]                 = { "ICC profile",                                  AV_SIDE_DATA_PROP_GLOBAL },
> +    [AV_FRAME_DATA_SEI_UNREGISTERED]            = { "H.26[45] User Data Unregistered SEI message",  AV_SIDE_DATA_PROP_MULTI },
> +};
> +
>   static void get_frame_defaults(AVFrame *frame)
>   {
>       memset(frame, 0, sizeof(*frame));
> @@ -895,38 +925,18 @@ void av_frame_remove_side_data(AVFrame *frame, enum AVFrameSideDataType type)
>       remove_side_data(&frame->side_data, &frame->nb_side_data, type);
>   }
>   
> +const AVSideDataDescriptor *av_frame_side_data_desc(enum AVFrameSideDataType type)
> +{
> +    unsigned t = type;
> +    if (t < FF_ARRAY_ELEMS(sd_props) && sd_props[t].name)
> +        return &sd_props[t];
> +    return NULL;
> +}
> +
>   const char *av_frame_side_data_name(enum AVFrameSideDataType type)
>   {
> -    switch(type) {
> -    case AV_FRAME_DATA_PANSCAN:         return "AVPanScan";
> -    case AV_FRAME_DATA_A53_CC:          return "ATSC A53 Part 4 Closed Captions";
> -    case AV_FRAME_DATA_STEREO3D:        return "Stereo 3D";
> -    case AV_FRAME_DATA_MATRIXENCODING:  return "AVMatrixEncoding";
> -    case AV_FRAME_DATA_DOWNMIX_INFO:    return "Metadata relevant to a downmix procedure";
> -    case AV_FRAME_DATA_REPLAYGAIN:      return "AVReplayGain";
> -    case AV_FRAME_DATA_DISPLAYMATRIX:   return "3x3 displaymatrix";
> -    case AV_FRAME_DATA_AFD:             return "Active format description";
> -    case AV_FRAME_DATA_MOTION_VECTORS:  return "Motion vectors";
> -    case AV_FRAME_DATA_SKIP_SAMPLES:    return "Skip samples";
> -    case AV_FRAME_DATA_AUDIO_SERVICE_TYPE:          return "Audio service type";
> -    case AV_FRAME_DATA_MASTERING_DISPLAY_METADATA:  return "Mastering display metadata";
> -    case AV_FRAME_DATA_CONTENT_LIGHT_LEVEL:         return "Content light level metadata";
> -    case AV_FRAME_DATA_GOP_TIMECODE:                return "GOP timecode";
> -    case AV_FRAME_DATA_S12M_TIMECODE:               return "SMPTE 12-1 timecode";
> -    case AV_FRAME_DATA_SPHERICAL:                   return "Spherical Mapping";
> -    case AV_FRAME_DATA_ICC_PROFILE:                 return "ICC profile";
> -    case AV_FRAME_DATA_DYNAMIC_HDR_PLUS: return "HDR Dynamic Metadata SMPTE2094-40 (HDR10+)";
> -    case AV_FRAME_DATA_DYNAMIC_HDR_VIVID: return "HDR Dynamic Metadata CUVA 005.1 2021 (Vivid)";
> -    case AV_FRAME_DATA_REGIONS_OF_INTEREST: return "Regions Of Interest";
> -    case AV_FRAME_DATA_VIDEO_ENC_PARAMS:            return "Video encoding parameters";
> -    case AV_FRAME_DATA_SEI_UNREGISTERED:            return "H.26[45] User Data Unregistered SEI message";
> -    case AV_FRAME_DATA_FILM_GRAIN_PARAMS:           return "Film grain parameters";
> -    case AV_FRAME_DATA_DETECTION_BBOXES:            return "Bounding boxes for object detection and classification";
> -    case AV_FRAME_DATA_DOVI_RPU_BUFFER:             return "Dolby Vision RPU Data";
> -    case AV_FRAME_DATA_DOVI_METADATA:               return "Dolby Vision Metadata";
> -    case AV_FRAME_DATA_AMBIENT_VIEWING_ENVIRONMENT: return "Ambient viewing environment";
> -    }
> -    return NULL;
> +    const AVSideDataDescriptor *desc = av_frame_side_data_desc(type);
> +    return desc ? desc->name : NULL;
>   }
>   
>   static int calc_cropping_offsets(size_t offsets[4], const AVFrame *frame,
> diff --git a/libavutil/frame.h b/libavutil/frame.h
> index a7fc909ad8..aa614c3dc6 100644
> --- a/libavutil/frame.h
> +++ b/libavutil/frame.h
> @@ -251,6 +251,27 @@ typedef struct AVFrameSideData {
>       AVBufferRef *buf;
>   } AVFrameSideData;
>   
> +enum AVSideDataProps {
> +    AV_SIDE_DATA_PROP_GLOBAL = (1 << 0),
> +    AV_SIDE_DATA_PROP_MULTI  = (1 << 1),
> +};
> +
> +/**
> + * This struct describes the properties of a side data type. Its instance
> + * corresponding to a given type can be obtained from av_frame_side_data_desc().
> + */
> +typedef struct AVSideDataDescriptor {
> +    /**
> +     * Human-readable side data description.
> +     */
> +    const char      *name;
> +
> +    /**
> +     * Side data property flags, a combination of AVSideDataProps values.
> +     */
> +    unsigned         props;
> +} AVSideDataDescriptor;
> +
>   /**
>    * Structure describing a single Region Of Interest.
>    *
> @@ -988,6 +1009,12 @@ int av_frame_apply_cropping(AVFrame *frame, int flags);
>    */
>   const char *av_frame_side_data_name(enum AVFrameSideDataType type);
>   
> +/**
> + * @return side data descriptor corresponding to a given side data type, NULL
> + *         when not available.
> + */
> +const AVSideDataDescriptor *av_frame_side_data_desc(enum AVFrameSideDataType type);
> +
>   /**
>    * Free all side data entries and their contents, then zeroes out the
>    * values which the pointers are pointing to.
> diff --git a/libavutil/version.h b/libavutil/version.h
> index 5027b025be..882003f719 100644
> --- a/libavutil/version.h
> +++ b/libavutil/version.h
> @@ -79,7 +79,7 @@
>    */
>   
>   #define LIBAVUTIL_VERSION_MAJOR  59
> -#define LIBAVUTIL_VERSION_MINOR   3
> +#define LIBAVUTIL_VERSION_MINOR   4
>   #define LIBAVUTIL_VERSION_MICRO 100
>   
>   #define LIBAVUTIL_VERSION_INT   AV_VERSION_INT(LIBAVUTIL_VERSION_MAJOR, \

Should be ok.
_______________________________________________
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".

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2024-03-26 21:38 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-03-23 13:04 [FFmpeg-devel] [PATCH 1/4] lavu/frame: add side data descriptors Anton Khirnov
2024-03-23 13:04 ` [FFmpeg-devel] [PATCH 2/4] fftools/ffmpeg_enc: only promote first frame side data to global when meaningful Anton Khirnov
2024-03-23 13:15   ` Andreas Rheinhardt
2024-03-23 13:19     ` Anton Khirnov
2024-03-23 13:21       ` Andreas Rheinhardt
2024-03-23 13:04 ` [FFmpeg-devel] [PATCH 3/4] fftools/ffmpeg_enc: move decoded_side_data setup out of video-only block Anton Khirnov
2024-03-23 13:04 ` [FFmpeg-devel] [PATCH 4/4] fftools/ffmpeg_enc: simplify error handling for decoded_side_data setup Anton Khirnov
2024-03-26 21:38 ` [FFmpeg-devel] [PATCH 1/4] lavu/frame: add side data descriptors James Almer

Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
		ffmpegdev@gitmailbox.com
	public-inbox-index ffmpegdev

Example config snippet for mirrors.


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git