[FFmpeg-devel] [PATCH] avutil/frame: add new functions to get frame side data

[FFmpeg-devel] [PATCH] avutil/frame: add new functions to get frame side data

[Leandro Santiago]

The functions are:

av_frame_side_data_get_nth_c()
av_frame_side_data_get_nth()
av_frame_get_nth_side_data()

They mimic the counterparts without the `_nth` suffix/infix, with an extra
argument, which specifies the relative position of the entry of a given
type.

Signed-off-by: Leandro Santiago <leandrosansilva@gmail.com>
---
 libavutil/frame.c     | 10 ++++++++++
 libavutil/frame.h     | 43 ++++++++++++++++++++++++++++++++++++++++---
 libavutil/side_data.c | 14 +++++++++++---
 3 files changed, 61 insertions(+), 6 deletions(-)

diff --git a/libavutil/frame.c b/libavutil/frame.c
index 71bde48f8b..dd0fd06378 100644
--- a/libavutil/frame.c
+++ b/libavutil/frame.c
@@ -699,6 +699,16 @@ AVFrameSideData *av_frame_get_side_data(const AVFrame *frame,
     );
 }
 
+AVFrameSideData *av_frame_get_nth_side_data(const AVFrame *frame,
+                                            enum AVFrameSideDataType type,
+                                            int n)
+{
+    return (AVFrameSideData *)av_frame_side_data_get_nth(
+        frame->side_data, frame->nb_side_data,
+        type, n
+    );
+}
+
 static int frame_copy_video(AVFrame *dst, const AVFrame *src)
 {
     int planes;
diff --git a/libavutil/frame.h b/libavutil/frame.h
index 49260ae2dd..56375280bb 100644
--- a/libavutil/frame.h
+++ b/libavutil/frame.h
@@ -1017,12 +1017,22 @@ AVFrameSideData *av_frame_new_side_data_from_buf(AVFrame *frame,
                                                  AVBufferRef *buf);
 
 /**
- * @return a pointer to the side data of a given type on success, NULL if there
+ * @return a pointer to the first side data of a given type on success, NULL if there
  * is no side data with such type in this frame.
  */
 AVFrameSideData *av_frame_get_side_data(const AVFrame *frame,
                                         enum AVFrameSideDataType type);
 
+/**
+ * @return a pointer to the side data of a given type on success,
+ * after skipping the first matches. Otherwise return NULL if there
+ * is no side data with such type in this frame or if all the matches were skipped.
+ * @param n the relative position of the side data entry
+ */
+AVFrameSideData *av_frame_get_nth_side_data(const AVFrame *frame,
+                                            enum AVFrameSideDataType type,
+                                            int n);
+
 /**
  * Remove and free all side data instances of the given type.
  */
@@ -1170,7 +1180,7 @@ int av_frame_side_data_clone(AVFrameSideData ***sd, int *nb_sd,
                              const AVFrameSideData *src, unsigned int flags);
 
 /**
- * Get a side data entry of a specific type from an array.
+ * Get the first side data entry of a specific type from an array.
  *
  * @param sd    array of side data.
  * @param nb_sd integer containing the number of entries in the array.
@@ -1184,7 +1194,24 @@ const AVFrameSideData *av_frame_side_data_get_c(const AVFrameSideData * const *s
                                                 enum AVFrameSideDataType type);
 
 /**
- * Wrapper around av_frame_side_data_get_c() to workaround the limitation
+ * Get a side data entry of a specific type from an array.
+ *
+ * @param sd    array of side data.
+ * @param nb_sd integer containing the number of entries in the array.
+ * @param type  type of side data to be queried
+ * @param n     relative position of the side data entry.
+ *
+ * @return a pointer to the side data of a given type on success, NULL if there
+ *         is no side data with such type in this set or all the entries were skipped.
+ */
+const AVFrameSideData *av_frame_side_data_get_nth_c(const AVFrameSideData * const *sd,
+                                                    const int nb_sd,
+                                                    enum AVFrameSideDataType type,
+                                                    int n
+);
+
+/**
+ * Wrappers around av_frame_side_data_get{_nth}_c() and to workaround the limitation
  * that for any type T the conversion from T * const * to const T * const *
  * is not performed automatically in C.
  * @see av_frame_side_data_get_c()
@@ -1198,6 +1225,16 @@ const AVFrameSideData *av_frame_side_data_get(AVFrameSideData * const *sd,
                                     nb_sd, type);
 }
 
+static inline
+const AVFrameSideData *av_frame_side_data_get_nth(AVFrameSideData * const *sd,
+                                                  const int nb_sd,
+                                                  enum AVFrameSideDataType type,
+                                                  int n)
+{
+    return av_frame_side_data_get_nth_c((const AVFrameSideData * const *)sd,
+                                        nb_sd, type, n);
+}
+
 /**
  * Remove and free all side data instances of the given type from an array.
  */
diff --git a/libavutil/side_data.c b/libavutil/side_data.c
index 17965f2d3c..bfe408e611 100644
--- a/libavutil/side_data.c
+++ b/libavutil/side_data.c
@@ -301,13 +301,21 @@ int av_frame_side_data_clone(AVFrameSideData ***sd, int *nb_sd,
     return 0;
 }
 
-const AVFrameSideData *av_frame_side_data_get_c(const AVFrameSideData * const *sd,
+const AVFrameSideData *av_frame_side_data_get_nth_c(const AVFrameSideData * const *sd,
                                                 const int nb_sd,
-                                                enum AVFrameSideDataType type)
+                                                enum AVFrameSideDataType type,
+                                                int n)
 {
     for (int i = 0; i < nb_sd; i++) {
-        if (sd[i]->type == type)
+        if (sd[i]->type == type && --n < 0)
             return sd[i];
     }
     return NULL;
 }
+
+const AVFrameSideData *av_frame_side_data_get_c(const AVFrameSideData * const *sd,
+                                                const int nb_sd,
+                                                enum AVFrameSideDataType type)
+{
+    return av_frame_side_data_get_nth_c(sd, nb_sd, type, 0);
+}
-- 
2.48.1

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

Re: [FFmpeg-devel] [PATCH] avutil/frame: add new functions to get frame side data

[Leandro Santiago]

Just pinging to see if anyone has had time to look at this patch :-)

I also noticed that this patch did not trigger an entry on patchwork. Is it normal or should the patches go to patchwork automatically?

Also, what's the correct approach within ffmpeg codebase to implement unit tests? Or would you recommend using the FATE tests?

On 3/4/25 12:55, Leandro Santiago wrote:
> The functions are:
>
> av_frame_side_data_get_nth_c()
> av_frame_side_data_get_nth()
> av_frame_get_nth_side_data()
>
> They mimic the counterparts without the `_nth` suffix/infix, with an extra
> argument, which specifies the relative position of the entry of a given
> type.
>
> Signed-off-by: Leandro Santiago <leandrosansilva@gmail.com>
> ---
>  libavutil/frame.c     | 10 ++++++++++
>  libavutil/frame.h     | 43 ++++++++++++++++++++++++++++++++++++++++---
>  libavutil/side_data.c | 14 +++++++++++---
>  3 files changed, 61 insertions(+), 6 deletions(-)
>
> diff --git a/libavutil/frame.c b/libavutil/frame.c
> index 71bde48f8b..dd0fd06378 100644
> --- a/libavutil/frame.c
> +++ b/libavutil/frame.c
> @@ -699,6 +699,16 @@ AVFrameSideData *av_frame_get_side_data(const AVFrame *frame,
>      );
>  }
>  
> +AVFrameSideData *av_frame_get_nth_side_data(const AVFrame *frame,
> +                                            enum AVFrameSideDataType type,
> +                                            int n)
> +{
> +    return (AVFrameSideData *)av_frame_side_data_get_nth(
> +        frame->side_data, frame->nb_side_data,
> +        type, n
> +    );
> +}
> +
>  static int frame_copy_video(AVFrame *dst, const AVFrame *src)
>  {
>      int planes;
> diff --git a/libavutil/frame.h b/libavutil/frame.h
> index 49260ae2dd..56375280bb 100644
> --- a/libavutil/frame.h
> +++ b/libavutil/frame.h
> @@ -1017,12 +1017,22 @@ AVFrameSideData *av_frame_new_side_data_from_buf(AVFrame *frame,
>                                                   AVBufferRef *buf);
>  
>  /**
> - * @return a pointer to the side data of a given type on success, NULL if there
> + * @return a pointer to the first side data of a given type on success, NULL if there
>   * is no side data with such type in this frame.
>   */
>  AVFrameSideData *av_frame_get_side_data(const AVFrame *frame,
>                                          enum AVFrameSideDataType type);
>  
> +/**
> + * @return a pointer to the side data of a given type on success,
> + * after skipping the first matches. Otherwise return NULL if there
> + * is no side data with such type in this frame or if all the matches were skipped.
> + * @param n the relative position of the side data entry
> + */
> +AVFrameSideData *av_frame_get_nth_side_data(const AVFrame *frame,
> +                                            enum AVFrameSideDataType type,
> +                                            int n);
> +
>  /**
>   * Remove and free all side data instances of the given type.
>   */
> @@ -1170,7 +1180,7 @@ int av_frame_side_data_clone(AVFrameSideData ***sd, int *nb_sd,
>                               const AVFrameSideData *src, unsigned int flags);
>  
>  /**
> - * Get a side data entry of a specific type from an array.
> + * Get the first side data entry of a specific type from an array.
>   *
>   * @param sd    array of side data.
>   * @param nb_sd integer containing the number of entries in the array.
> @@ -1184,7 +1194,24 @@ const AVFrameSideData *av_frame_side_data_get_c(const AVFrameSideData * const *s
>                                                  enum AVFrameSideDataType type);
>  
>  /**
> - * Wrapper around av_frame_side_data_get_c() to workaround the limitation
> + * Get a side data entry of a specific type from an array.
> + *
> + * @param sd    array of side data.
> + * @param nb_sd integer containing the number of entries in the array.
> + * @param type  type of side data to be queried
> + * @param n     relative position of the side data entry.
> + *
> + * @return a pointer to the side data of a given type on success, NULL if there
> + *         is no side data with such type in this set or all the entries were skipped.
> + */
> +const AVFrameSideData *av_frame_side_data_get_nth_c(const AVFrameSideData * const *sd,
> +                                                    const int nb_sd,
> +                                                    enum AVFrameSideDataType type,
> +                                                    int n
> +);
> +
> +/**
> + * Wrappers around av_frame_side_data_get{_nth}_c() and to workaround the limitation
>   * that for any type T the conversion from T * const * to const T * const *
>   * is not performed automatically in C.
>   * @see av_frame_side_data_get_c()
> @@ -1198,6 +1225,16 @@ const AVFrameSideData *av_frame_side_data_get(AVFrameSideData * const *sd,
>                                      nb_sd, type);
>  }
>  
> +static inline
> +const AVFrameSideData *av_frame_side_data_get_nth(AVFrameSideData * const *sd,
> +                                                  const int nb_sd,
> +                                                  enum AVFrameSideDataType type,
> +                                                  int n)
> +{
> +    return av_frame_side_data_get_nth_c((const AVFrameSideData * const *)sd,
> +                                        nb_sd, type, n);
> +}
> +
>  /**
>   * Remove and free all side data instances of the given type from an array.
>   */
> diff --git a/libavutil/side_data.c b/libavutil/side_data.c
> index 17965f2d3c..bfe408e611 100644
> --- a/libavutil/side_data.c
> +++ b/libavutil/side_data.c
> @@ -301,13 +301,21 @@ int av_frame_side_data_clone(AVFrameSideData ***sd, int *nb_sd,
>      return 0;
>  }
>  
> -const AVFrameSideData *av_frame_side_data_get_c(const AVFrameSideData * const *sd,
> +const AVFrameSideData *av_frame_side_data_get_nth_c(const AVFrameSideData * const *sd,
>                                                  const int nb_sd,
> -                                                enum AVFrameSideDataType type)
> +                                                enum AVFrameSideDataType type,
> +                                                int n)
>  {
>      for (int i = 0; i < nb_sd; i++) {
> -        if (sd[i]->type == type)
> +        if (sd[i]->type == type && --n < 0)
>              return sd[i];
>      }
>      return NULL;
>  }
> +
> +const AVFrameSideData *av_frame_side_data_get_c(const AVFrameSideData * const *sd,
> +                                                const int nb_sd,
> +                                                enum AVFrameSideDataType type)
> +{
> +    return av_frame_side_data_get_nth_c(sd, nb_sd, type, 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".

Re: [FFmpeg-devel] [PATCH] avutil/frame: add new functions to get frame side data

[Andreas Rheinhardt]

Leandro Santiago:
> The functions are:
> 
> av_frame_side_data_get_nth_c()
> av_frame_side_data_get_nth()
> av_frame_get_nth_side_data()
> 
> They mimic the counterparts without the `_nth` suffix/infix, with an extra
> argument, which specifies the relative position of the entry of a given
> type.
> 
> Signed-off-by: Leandro Santiago <leandrosansilva@gmail.com>
> ---
>  libavutil/frame.c     | 10 ++++++++++
>  libavutil/frame.h     | 43 ++++++++++++++++++++++++++++++++++++++++---
>  libavutil/side_data.c | 14 +++++++++++---
>  3 files changed, 61 insertions(+), 6 deletions(-)
> 
Is this really such an important usecase that we need to add new public
functions for this? Why can't users simply iterate over the array on
their own?

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

Re: [FFmpeg-devel] [PATCH] avutil/frame: add new functions to get frame side data

[James Almer]

[-- Attachment #1.1.1: Type: text/plain, Size: 897 bytes --]

On 3/10/2025 11:57 AM, Andreas Rheinhardt wrote:
> Leandro Santiago:
>> The functions are:
>>
>> av_frame_side_data_get_nth_c()
>> av_frame_side_data_get_nth()
>> av_frame_get_nth_side_data()
>>
>> They mimic the counterparts without the `_nth` suffix/infix, with an extra
>> argument, which specifies the relative position of the entry of a given
>> type.
>>
>> Signed-off-by: Leandro Santiago <leandrosansilva@gmail.com>
>> ---
>>   libavutil/frame.c     | 10 ++++++++++
>>   libavutil/frame.h     | 43 ++++++++++++++++++++++++++++++++++++++++---
>>   libavutil/side_data.c | 14 +++++++++++---
>>   3 files changed, 61 insertions(+), 6 deletions(-)
>>
> Is this really such an important usecase that we need to add new public
> functions for this?

IMO, no. There's only one side data type with the multi prop, SEI 
Unregistered, so this function is somewhat overkill.


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

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