* [FFmpeg-devel] [PATCH 01/10] avcodec/packet: add side data set struct and helpers
2023-09-06 17:44 [FFmpeg-devel] [PATCH 00/10 v4][RFC] AVCodecContext and AVCodecParameters side data James Almer
@ 2023-09-06 17:44 ` James Almer
2023-09-11 17:41 ` Andreas Rheinhardt
2023-09-06 17:44 ` [FFmpeg-devel] [PATCH 02/10] avcodec/codec_par: add side data to AVCodecParameters James Almer
` (10 subsequent siblings)
11 siblings, 1 reply; 28+ messages in thread
From: James Almer @ 2023-09-06 17:44 UTC (permalink / raw)
To: ffmpeg-devel
This will be useful in the following commits.
Signed-off-by: James Almer <jamrial@gmail.com>
---
libavcodec/avpacket.c | 99 +++++++++++++++++++++++++++++++++++++++++++
libavcodec/packet.h | 74 ++++++++++++++++++++++++++++++++
2 files changed, 173 insertions(+)
diff --git a/libavcodec/avpacket.c b/libavcodec/avpacket.c
index 5fef65e97a..5b133c5d8a 100644
--- a/libavcodec/avpacket.c
+++ b/libavcodec/avpacket.c
@@ -645,3 +645,102 @@ int ff_side_data_set_prft(AVPacket *pkt, int64_t timestamp)
return 0;
}
+
+AVPacketSideData *av_packet_side_data_set_get(const AVPacketSideDataSet *set,
+ enum AVPacketSideDataType type)
+{
+ for (int i = 0; i < set->nb_sd; i++)
+ if (set->sd[i]->type == type)
+ return set->sd[i];
+
+ return NULL;
+}
+
+static AVPacketSideData *add_side_data_to_set(AVPacketSideDataSet *set,
+ enum AVPacketSideDataType type,
+ uint8_t *data, size_t size)
+{
+ AVPacketSideData *sd, **tmp;
+
+ for (int i = 0; i < set->nb_sd; i++) {
+ sd = set->sd[i];
+ if (sd->type != type)
+ continue;
+
+ av_freep(&sd->data);
+ sd->data = data;
+ sd->size = size;
+ return sd;
+ }
+
+ if (set->nb_sd + 1U > INT_MAX)
+ return NULL;
+
+ tmp = av_realloc_array(set->sd, set->nb_sd + 1, sizeof(*tmp));
+ if (!tmp)
+ return NULL;
+
+ set->sd = tmp;
+
+ sd = av_mallocz(sizeof(*sd));
+ if (!sd)
+ return NULL;
+
+ sd->type = type;
+ sd->data = data;
+ sd->size = size;
+
+ set->sd[set->nb_sd++] = sd;
+
+ return sd;
+}
+
+AVPacketSideData *av_packet_side_data_set_add(AVPacketSideDataSet *set,
+ enum AVPacketSideDataType type,
+ uint8_t *data, size_t size,
+ int flags)
+{
+ return add_side_data_to_set(set, type, data, size);
+}
+
+AVPacketSideData *av_packet_side_data_set_new(AVPacketSideDataSet *set,
+ enum AVPacketSideDataType type,
+ size_t size, int flags)
+{
+ AVPacketSideData *sd = NULL;
+ uint8_t *data = av_malloc(size);
+
+ if (!data)
+ return NULL;
+
+ sd = add_side_data_to_set(set, type, data, size);
+ if (!sd)
+ av_freep(&data);
+
+ return sd;
+}
+
+void av_packet_side_data_set_remove(AVPacketSideDataSet *set,
+ enum AVPacketSideDataType type)
+{
+ for (int i = set->nb_sd - 1; i >= 0; i--) {
+ AVPacketSideData *sd = set->sd[i];
+ if (sd->type != type)
+ continue;
+ av_free(set->sd[i]->data);
+ av_free(set->sd[i]);
+ set->sd[i] = set->sd[--set->nb_sd];
+ break;
+ }
+}
+
+void av_packet_side_data_set_free(AVPacketSideDataSet *set)
+{
+ for (int i = 0; i < set->nb_sd; i++) {
+ av_free(set->sd[i]->data);
+ av_free(set->sd[i]);
+ }
+ set->nb_sd = 0;
+
+ av_freep(&set->sd);
+}
diff --git a/libavcodec/packet.h b/libavcodec/packet.h
index f28e7e7011..87720ab909 100644
--- a/libavcodec/packet.h
+++ b/libavcodec/packet.h
@@ -318,6 +318,20 @@ typedef struct AVPacketSideData {
enum AVPacketSideDataType type;
} AVPacketSideData;
+/**
+ * Structure to hold a set of AVPacketSideData
+ *
+ * @see av_packet_side_data_set_new
+ * @see av_packet_side_data_set_add
+ * @see av_packet_side_data_set_get
+ * @see av_packet_side_data_set_remove
+ * @see av_packet_side_data_set_free
+ */
+typedef struct AVPacketSideDataSet {
+ AVPacketSideData **sd;
+ int nb_sd;
+} AVPacketSideDataSet;
+
/**
* This structure stores compressed data. It is typically exported by demuxers
* and then passed as input to decoders, or received as output from encoders and
@@ -724,6 +738,66 @@ int av_packet_make_writable(AVPacket *pkt);
*/
void av_packet_rescale_ts(AVPacket *pkt, AVRational tb_src, AVRational tb_dst);
+/**
+ * Allocate a new side data entry into to a set.
+ *
+ * @param set a set to which the side data should be added
+ * @param type side data type
+ * @param size side data size
+ * @param flags currently unused
+ * @return pointer to freshly allocated side data entry on success, or NULL
+ * otherwise.
+ */
+AVPacketSideData *av_packet_side_data_set_new(AVPacketSideDataSet *set,
+ enum AVPacketSideDataType type,
+ size_t size, int flags);
+
+/**
+ * Wrap an existing array as a packet side data into a set.
+ *
+ * @param set a set to which the side data should be added
+ * @param type side data type
+ * @param data a data array. It must be allocated with the av_malloc() family
+ * of functions. The ownership of the data is transferred to the
+ * set on success
+ * @param size size of the data array
+ * @param flags currently unused
+ * @return pointer to freshly allocated side data entry on success, or NULL
+ * otherwise. On failure, the set is unchanged and the data remains
+ * owned by the caller.
+ */
+AVPacketSideData *av_packet_side_data_set_add(AVPacketSideDataSet *set,
+ enum AVPacketSideDataType type,
+ uint8_t *data, size_t size,
+ int flags);
+
+/**
+ * Remove side data of the given type from a set.
+ *
+ * @param set a set from which the side data should be removed
+ * @param type side information type
+ */
+void av_packet_side_data_set_remove(AVPacketSideDataSet *set,
+ enum AVPacketSideDataType type);
+
+/**
+ * Get side information from set.
+ *
+ * @param set a set from which the side data should be fetched
+ * @param type desired side information type
+ *
+ * @return pointer to side data if present or NULL otherwise
+ */
+AVPacketSideData *av_packet_side_data_set_get(const AVPacketSideDataSet *set,
+ enum AVPacketSideDataType type);
+
+/**
+ * Convenience function to free all the side data stored in a set.
+ *
+ * @param set the set to free
+ */
+void av_packet_side_data_set_free(AVPacketSideDataSet *set);
+
/**
* @}
*/
--
2.42.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] 28+ messages in thread* Re: [FFmpeg-devel] [PATCH 01/10] avcodec/packet: add side data set struct and helpers
2023-09-06 17:44 ` [FFmpeg-devel] [PATCH 01/10] avcodec/packet: add side data set struct and helpers James Almer
@ 2023-09-11 17:41 ` Andreas Rheinhardt
2023-09-11 18:00 ` James Almer
0 siblings, 1 reply; 28+ messages in thread
From: Andreas Rheinhardt @ 2023-09-11 17:41 UTC (permalink / raw)
To: ffmpeg-devel
James Almer:
> This will be useful in the following commits.
>
> Signed-off-by: James Almer <jamrial@gmail.com>
> ---
> libavcodec/avpacket.c | 99 +++++++++++++++++++++++++++++++++++++++++++
> libavcodec/packet.h | 74 ++++++++++++++++++++++++++++++++
> 2 files changed, 173 insertions(+)
>
> diff --git a/libavcodec/avpacket.c b/libavcodec/avpacket.c
> index 5fef65e97a..5b133c5d8a 100644
> --- a/libavcodec/avpacket.c
> +++ b/libavcodec/avpacket.c
> @@ -645,3 +645,102 @@ int ff_side_data_set_prft(AVPacket *pkt, int64_t timestamp)
>
> return 0;
> }
> +
> +AVPacketSideData *av_packet_side_data_set_get(const AVPacketSideDataSet *set,
> + enum AVPacketSideDataType type)
> +{
> + for (int i = 0; i < set->nb_sd; i++)
> + if (set->sd[i]->type == type)
> + return set->sd[i];
> +
> + return NULL;
> +}
> +
> +static AVPacketSideData *add_side_data_to_set(AVPacketSideDataSet *set,
> + enum AVPacketSideDataType type,
> + uint8_t *data, size_t size)
> +{
> + AVPacketSideData *sd, **tmp;
> +
> + for (int i = 0; i < set->nb_sd; i++) {
> + sd = set->sd[i];
> + if (sd->type != type)
> + continue;
> +
> + av_freep(&sd->data);
> + sd->data = data;
> + sd->size = size;
> + return sd;
> + }
> +
> + if (set->nb_sd + 1U > INT_MAX)
> + return NULL;
> +
> + tmp = av_realloc_array(set->sd, set->nb_sd + 1, sizeof(*tmp));
> + if (!tmp)
> + return NULL;
> +
> + set->sd = tmp;
> +
> + sd = av_mallocz(sizeof(*sd));
> + if (!sd)
> + return NULL;
> +
> + sd->type = type;
> + sd->data = data;
> + sd->size = size;
> +
> + set->sd[set->nb_sd++] = sd;
> +
> + return sd;
> +}
> +
> +AVPacketSideData *av_packet_side_data_set_add(AVPacketSideDataSet *set,
> + enum AVPacketSideDataType type,
> + uint8_t *data, size_t size,
> + int flags)
> +{
> + return add_side_data_to_set(set, type, data, size);
> +}
> +
> +AVPacketSideData *av_packet_side_data_set_new(AVPacketSideDataSet *set,
> + enum AVPacketSideDataType type,
> + size_t size, int flags)
> +{
> + AVPacketSideData *sd = NULL;
> + uint8_t *data = av_malloc(size);
How about padding in case we have new extradata?
> +
> + if (!data)
> + return NULL;
> +
> + sd = add_side_data_to_set(set, type, data, size);
> + if (!sd)
> + av_freep(&data);
> +
> + return sd;
> +}
> +
> +void av_packet_side_data_set_remove(AVPacketSideDataSet *set,
> + enum AVPacketSideDataType type)
> +{
> + for (int i = set->nb_sd - 1; i >= 0; i--) {
> + AVPacketSideData *sd = set->sd[i];
> + if (sd->type != type)
> + continue;
> + av_free(set->sd[i]->data);
> + av_free(set->sd[i]);
> + set->sd[i] = set->sd[--set->nb_sd];
> + break;
> + }
> +}
> +
> +void av_packet_side_data_set_free(AVPacketSideDataSet *set)
> +{
> + for (int i = 0; i < set->nb_sd; i++) {
> + av_free(set->sd[i]->data);
> + av_free(set->sd[i]);
> + }
> + set->nb_sd = 0;
> +
> + av_freep(&set->sd);
> +}
> diff --git a/libavcodec/packet.h b/libavcodec/packet.h
> index f28e7e7011..87720ab909 100644
> --- a/libavcodec/packet.h
> +++ b/libavcodec/packet.h
> @@ -318,6 +318,20 @@ typedef struct AVPacketSideData {
> enum AVPacketSideDataType type;
> } AVPacketSideData;
>
> +/**
> + * Structure to hold a set of AVPacketSideData
This should mention whether it is legal or not to have multiple side
datas of the same type in the same set.
> + *
> + * @see av_packet_side_data_set_new
> + * @see av_packet_side_data_set_add
> + * @see av_packet_side_data_set_get
> + * @see av_packet_side_data_set_remove
> + * @see av_packet_side_data_set_free
> + */
This should mention that its size is part of the ABI.
> +typedef struct AVPacketSideDataSet {
> + AVPacketSideData **sd;
Allocating the AVPacketSideDatas independently is a big break from how
it is done with AVPackets. Is this really intended? IMO not worth it.
> + int nb_sd;
int? Why not something unsigned given that it will never be negative?
> +} AVPacketSideDataSet;
> +
> /**
> * This structure stores compressed data. It is typically exported by demuxers
> * and then passed as input to decoders, or received as output from encoders and
> @@ -724,6 +738,66 @@ int av_packet_make_writable(AVPacket *pkt);
> */
> void av_packet_rescale_ts(AVPacket *pkt, AVRational tb_src, AVRational tb_dst);
>
> +/**
> + * Allocate a new side data entry into to a set.
> + *
> + * @param set a set to which the side data should be added
> + * @param type side data type
> + * @param size side data size
> + * @param flags currently unused
must be zero
> + * @return pointer to freshly allocated side data entry on success, or NULL
> + * otherwise.
> + */
> +AVPacketSideData *av_packet_side_data_set_new(AVPacketSideDataSet *set,
> + enum AVPacketSideDataType type,
> + size_t size, int flags);
> +
> +/**
> + * Wrap an existing array as a packet side data into a set.
> + *
> + * @param set a set to which the side data should be added
> + * @param type side data type
> + * @param data a data array. It must be allocated with the av_malloc() family
> + * of functions. The ownership of the data is transferred to the
> + * set on success
> + * @param size size of the data array
> + * @param flags currently unused
> + * @return pointer to freshly allocated side data entry on success, or NULL
> + * otherwise. On failure, the set is unchanged and the data remains
> + * owned by the caller.
> + */
> +AVPacketSideData *av_packet_side_data_set_add(AVPacketSideDataSet *set,
> + enum AVPacketSideDataType type,
> + uint8_t *data, size_t size,
data should be void*; the days in which side data was just a buffer and
not a structure are long gone.
Changing AVPacketSideData is a different issue.
> + int flags);
> +
> +/**
> + * Remove side data of the given type from a set.
> + *
> + * @param set a set from which the side data should be removed
> + * @param type side information type
> + */
> +void av_packet_side_data_set_remove(AVPacketSideDataSet *set,
> + enum AVPacketSideDataType type);
> +
> +/**
> + * Get side information from set.
> + *
> + * @param set a set from which the side data should be fetched
> + * @param type desired side information type
> + *
> + * @return pointer to side data if present or NULL otherwise
> + */
> +AVPacketSideData *av_packet_side_data_set_get(const AVPacketSideDataSet *set,
Is it really good to give the user the right to modify the entries of a
set that they don't own?
> + enum AVPacketSideDataType type);
> +
> +/**
> + * Convenience function to free all the side data stored in a set.
> + *
> + * @param set the set to free
> + */
> +void av_packet_side_data_set_free(AVPacketSideDataSet *set);
> +
> /**
> * @}
> */
_______________________________________________
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] 28+ messages in thread* Re: [FFmpeg-devel] [PATCH 01/10] avcodec/packet: add side data set struct and helpers
2023-09-11 17:41 ` Andreas Rheinhardt
@ 2023-09-11 18:00 ` James Almer
0 siblings, 0 replies; 28+ messages in thread
From: James Almer @ 2023-09-11 18:00 UTC (permalink / raw)
To: ffmpeg-devel
On 9/11/2023 2:41 PM, Andreas Rheinhardt wrote:
> James Almer:
>> This will be useful in the following commits.
>>
>> Signed-off-by: James Almer <jamrial@gmail.com>
>> ---
>> libavcodec/avpacket.c | 99 +++++++++++++++++++++++++++++++++++++++++++
>> libavcodec/packet.h | 74 ++++++++++++++++++++++++++++++++
>> 2 files changed, 173 insertions(+)
>>
>> diff --git a/libavcodec/avpacket.c b/libavcodec/avpacket.c
>> index 5fef65e97a..5b133c5d8a 100644
>> --- a/libavcodec/avpacket.c
>> +++ b/libavcodec/avpacket.c
>> @@ -645,3 +645,102 @@ int ff_side_data_set_prft(AVPacket *pkt, int64_t timestamp)
>>
>> return 0;
>> }
>> +
>> +AVPacketSideData *av_packet_side_data_set_get(const AVPacketSideDataSet *set,
>> + enum AVPacketSideDataType type)
>> +{
>> + for (int i = 0; i < set->nb_sd; i++)
>> + if (set->sd[i]->type == type)
>> + return set->sd[i];
>> +
>> + return NULL;
>> +}
>> +
>> +static AVPacketSideData *add_side_data_to_set(AVPacketSideDataSet *set,
>> + enum AVPacketSideDataType type,
>> + uint8_t *data, size_t size)
>> +{
>> + AVPacketSideData *sd, **tmp;
>> +
>> + for (int i = 0; i < set->nb_sd; i++) {
>> + sd = set->sd[i];
>> + if (sd->type != type)
>> + continue;
>> +
>> + av_freep(&sd->data);
>> + sd->data = data;
>> + sd->size = size;
>> + return sd;
>> + }
>> +
>> + if (set->nb_sd + 1U > INT_MAX)
>> + return NULL;
>> +
>> + tmp = av_realloc_array(set->sd, set->nb_sd + 1, sizeof(*tmp));
>> + if (!tmp)
>> + return NULL;
>> +
>> + set->sd = tmp;
>> +
>> + sd = av_mallocz(sizeof(*sd));
>> + if (!sd)
>> + return NULL;
>> +
>> + sd->type = type;
>> + sd->data = data;
>> + sd->size = size;
>> +
>> + set->sd[set->nb_sd++] = sd;
>> +
>> + return sd;
>> +}
>> +
>> +AVPacketSideData *av_packet_side_data_set_add(AVPacketSideDataSet *set,
>> + enum AVPacketSideDataType type,
>> + uint8_t *data, size_t size,
>> + int flags)
>> +{
>> + return add_side_data_to_set(set, type, data, size);
>> +}
>> +
>> +AVPacketSideData *av_packet_side_data_set_new(AVPacketSideDataSet *set,
>> + enum AVPacketSideDataType type,
>> + size_t size, int flags)
>> +{
>> + AVPacketSideData *sd = NULL;
>> + uint8_t *data = av_malloc(size);
>
> How about padding in case we have new extradata?
Ok.
Despite side data of new extradata type not being something we'll see in
global side data, in the future AVPacket could also start using this
struct and helpers, so it's worth including padding now.
>
>> +
>> + if (!data)
>> + return NULL;
>> +
>> + sd = add_side_data_to_set(set, type, data, size);
>> + if (!sd)
>> + av_freep(&data);
>> +
>> + return sd;
>> +}
>> +
>> +void av_packet_side_data_set_remove(AVPacketSideDataSet *set,
>> + enum AVPacketSideDataType type)
>> +{
>> + for (int i = set->nb_sd - 1; i >= 0; i--) {
>> + AVPacketSideData *sd = set->sd[i];
>> + if (sd->type != type)
>> + continue;
>> + av_free(set->sd[i]->data);
>> + av_free(set->sd[i]);
>> + set->sd[i] = set->sd[--set->nb_sd];
>> + break;
>> + }
>> +}
>> +
>> +void av_packet_side_data_set_free(AVPacketSideDataSet *set)
>> +{
>> + for (int i = 0; i < set->nb_sd; i++) {
>> + av_free(set->sd[i]->data);
>> + av_free(set->sd[i]);
>> + }
>> + set->nb_sd = 0;
>> +
>> + av_freep(&set->sd);
>> +}
>> diff --git a/libavcodec/packet.h b/libavcodec/packet.h
>> index f28e7e7011..87720ab909 100644
>> --- a/libavcodec/packet.h
>> +++ b/libavcodec/packet.h
>> @@ -318,6 +318,20 @@ typedef struct AVPacketSideData {
>> enum AVPacketSideDataType type;
>> } AVPacketSideData;
>>
>> +/**
>> + * Structure to hold a set of AVPacketSideData
>
> This should mention whether it is legal or not to have multiple side
> datas of the same type in the same set.
Ok.
>
>> + *
>> + * @see av_packet_side_data_set_new
>> + * @see av_packet_side_data_set_add
>> + * @see av_packet_side_data_set_get
>> + * @see av_packet_side_data_set_remove
>> + * @see av_packet_side_data_set_free
>> + */
>
> This should mention that its size is part of the ABI.
Ok.
>
>> +typedef struct AVPacketSideDataSet {
>> + AVPacketSideData **sd;
>
> Allocating the AVPacketSideDatas independently is a big break from how
> it is done with AVPackets. Is this really intended? IMO not worth it.
I did it to have this struct in line with Jan's frame one (and frame
side data in general). This way helpers can also share a common signature.
>
>> + int nb_sd;
>
> int? Why not something unsigned given that it will never be negative?
Entry count is int pretty much everywhere. Is it worth making it
unsigned only here?
>
>> +} AVPacketSideDataSet;
>> +
>> /**
>> * This structure stores compressed data. It is typically exported by demuxers
>> * and then passed as input to decoders, or received as output from encoders and
>> @@ -724,6 +738,66 @@ int av_packet_make_writable(AVPacket *pkt);
>> */
>> void av_packet_rescale_ts(AVPacket *pkt, AVRational tb_src, AVRational tb_dst);
>>
>> +/**
>> + * Allocate a new side data entry into to a set.
>> + *
>> + * @param set a set to which the side data should be added
>> + * @param type side data type
>> + * @param size side data size
>> + * @param flags currently unused
>
> must be zero
Will add.
>
>> + * @return pointer to freshly allocated side data entry on success, or NULL
>> + * otherwise.
>> + */
>> +AVPacketSideData *av_packet_side_data_set_new(AVPacketSideDataSet *set,
>> + enum AVPacketSideDataType type,
>> + size_t size, int flags);
>> +
>> +/**
>> + * Wrap an existing array as a packet side data into a set.
>> + *
>> + * @param set a set to which the side data should be added
>> + * @param type side data type
>> + * @param data a data array. It must be allocated with the av_malloc() family
>> + * of functions. The ownership of the data is transferred to the
>> + * set on success
>> + * @param size size of the data array
>> + * @param flags currently unused
>> + * @return pointer to freshly allocated side data entry on success, or NULL
>> + * otherwise. On failure, the set is unchanged and the data remains
>> + * owned by the caller.
>> + */
>> +AVPacketSideData *av_packet_side_data_set_add(AVPacketSideDataSet *set,
>> + enum AVPacketSideDataType type,
>> + uint8_t *data, size_t size,
>
> data should be void*; the days in which side data was just a buffer and
> not a structure are long gone.
Ok.
> Changing AVPacketSideData is a different issue.
>
>> + int flags);
>> +
>> +/**
>> + * Remove side data of the given type from a set.
>> + *
>> + * @param set a set from which the side data should be removed
>> + * @param type side information type
>> + */
>> +void av_packet_side_data_set_remove(AVPacketSideDataSet *set,
>> + enum AVPacketSideDataType type);
>> +
>> +/**
>> + * Get side information from set.
>> + *
>> + * @param set a set from which the side data should be fetched
>> + * @param type desired side information type
>> + *
>> + * @return pointer to side data if present or NULL otherwise
>> + */
>> +AVPacketSideData *av_packet_side_data_set_get(const AVPacketSideDataSet *set,
>
> Is it really good to give the user the right to modify the entries of a
> set that they don't own?
Will make it const.
>
>> + enum AVPacketSideDataType type);
>> +
>> +/**
>> + * Convenience function to free all the side data stored in a set.
>> + *
>> + * @param set the set to free
>> + */
>> +void av_packet_side_data_set_free(AVPacketSideDataSet *set);
>> +
>> /**
>> * @}
>> */
This aside, do you have an opinion regarding the inclusion of a new
field of AVPacketSideDataSet to AVCodecContext, like in the previous
iteration of this set, instead of using coded_side_data? I really prefer
the former, as currently there's no way for users to allocate or fill
coded_side_data other than doing it manually, and that's pretty ugly and
unintuitive.
Since coded_side_data is barely used right now, deprecating it will be
completely painless.
Anton suggested dropping the set struct altogether and just use the
underlying fields directly in AVCodecParameters, then make the helpers
take IN|OUT pointer to pointer parameters to change the array and the
element count fields, which IMO is also pretty ugly.
_______________________________________________
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] 28+ messages in thread
* [FFmpeg-devel] [PATCH 02/10] avcodec/codec_par: add side data to AVCodecParameters
2023-09-06 17:44 [FFmpeg-devel] [PATCH 00/10 v4][RFC] AVCodecContext and AVCodecParameters side data James Almer
2023-09-06 17:44 ` [FFmpeg-devel] [PATCH 01/10] avcodec/packet: add side data set struct and helpers James Almer
@ 2023-09-06 17:44 ` James Almer
2023-09-11 17:45 ` Andreas Rheinhardt
2023-09-06 17:44 ` [FFmpeg-devel] [PATCH 03/10] avformat/avformat: use the side data from AVStream.codecpar James Almer
` (9 subsequent siblings)
11 siblings, 1 reply; 28+ messages in thread
From: James Almer @ 2023-09-06 17:44 UTC (permalink / raw)
To: ffmpeg-devel
This will simplify the propagation of side data to decoders and from encoders.
Global side data will now reside in the AVCodecContext, thus be available
during init(), removing the need to propagate it inside packets.
Global and frame specific side data will therefore be distinct.
Signed-off-by: James Almer <jamrial@gmail.com>
---
libavcodec/codec_par.c | 81 ++++++++++++++++++++++++++++++++++++++++++
libavcodec/codec_par.h | 6 ++++
2 files changed, 87 insertions(+)
diff --git a/libavcodec/codec_par.c b/libavcodec/codec_par.c
index a38a475dc7..c0c941c2b6 100644
--- a/libavcodec/codec_par.c
+++ b/libavcodec/codec_par.c
@@ -27,11 +27,13 @@
#include "libavutil/mem.h"
#include "avcodec.h"
#include "codec_par.h"
+#include "packet.h"
static void codec_parameters_reset(AVCodecParameters *par)
{
av_freep(&par->extradata);
av_channel_layout_uninit(&par->ch_layout);
+ av_packet_side_data_set_free(&par->side_data);
memset(par, 0, sizeof(*par));
@@ -82,6 +84,8 @@ int avcodec_parameters_copy(AVCodecParameters *dst, const AVCodecParameters *src
dst->ch_layout = (AVChannelLayout){0};
dst->extradata = NULL;
dst->extradata_size = 0;
+ dst->side_data.sd = NULL;
+ dst->side_data.nb_sd = 0;
if (src->extradata) {
dst->extradata = av_mallocz(src->extradata_size + AV_INPUT_BUFFER_PADDING_SIZE);
if (!dst->extradata)
@@ -89,6 +93,32 @@ int avcodec_parameters_copy(AVCodecParameters *dst, const AVCodecParameters *src
memcpy(dst->extradata, src->extradata, src->extradata_size);
dst->extradata_size = src->extradata_size;
}
+ if (src->side_data.nb_sd) {
+ const AVPacketSideDataSet *src_set = &src->side_data;
+ AVPacketSideDataSet *dst_set = &dst->side_data;
+
+ dst_set->sd = av_calloc(src_set->nb_sd, sizeof(*dst_set->sd));
+ if (!dst_set->sd)
+ return AVERROR(ENOMEM);
+
+ for (int i = 0; i < src_set->nb_sd; i++) {
+ const AVPacketSideData *src_sd = src_set->sd[i];
+ AVPacketSideData *dst_sd = av_mallocz(sizeof(*dst_sd));
+
+ if (!dst_sd)
+ return AVERROR(ENOMEM);
+
+ dst_sd->data = av_memdup(src_sd->data, src_sd->size);
+ if (!dst_sd->data) {
+ return AVERROR(ENOMEM);
+ av_free(dst_sd);
+ }
+
+ dst_sd->type = src_sd->type;
+ dst_sd->size = src_sd->size;
+ dst_set->sd[dst_set->nb_sd++] = dst_sd;
+ }
+ }
ret = av_channel_layout_copy(&dst->ch_layout, &src->ch_layout);
if (ret < 0)
@@ -178,6 +208,32 @@ FF_ENABLE_DEPRECATION_WARNINGS
par->extradata_size = codec->extradata_size;
}
+ if (codec->nb_coded_side_data) {
+ AVPacketSideDataSet *dst_set = &par->side_data;
+
+ dst_set->sd = av_calloc(codec->nb_coded_side_data, sizeof(*dst_set->sd));
+ if (!dst_set->sd)
+ return AVERROR(ENOMEM);
+
+ for (int i = 0; i < codec->nb_coded_side_data; i++) {
+ const AVPacketSideData *src_sd = &codec->coded_side_data[i];
+ AVPacketSideData *dst_sd = av_mallocz(sizeof(*dst_sd));
+
+ if (!dst_sd)
+ return AVERROR(ENOMEM);
+
+ dst_sd->data = av_memdup(src_sd->data, src_sd->size);
+ if (!dst_sd->data) {
+ return AVERROR(ENOMEM);
+ av_free(dst_sd);
+ }
+
+ dst_sd->type = src_sd->type;
+ dst_sd->size = src_sd->size;
+ dst_set->sd[dst_set->nb_sd++] = dst_sd;
+ }
+ }
+
return 0;
}
@@ -262,5 +318,30 @@ FF_ENABLE_DEPRECATION_WARNINGS
codec->extradata_size = par->extradata_size;
}
+ for (int i = 0; i < codec->nb_coded_side_data; i++)
+ av_free(codec->coded_side_data[i].data);
+ av_freep(&codec->coded_side_data);
+ codec->nb_coded_side_data = 0;
+ if (par->side_data.nb_sd) {
+ const AVPacketSideDataSet *src_set = &par->side_data;
+
+ codec->coded_side_data = av_calloc(src_set->nb_sd, sizeof(*codec->coded_side_data));
+ if (!codec->coded_side_data)
+ return AVERROR(ENOMEM);
+
+ for (int i = 0; i < src_set->nb_sd; i++) {
+ const AVPacketSideData *src_sd = src_set->sd[i];
+ AVPacketSideData *dst_sd = &codec->coded_side_data[i];
+
+ dst_sd->data = av_memdup(src_sd->data, src_sd->size);
+ if (!dst_sd->data)
+ return AVERROR(ENOMEM);
+
+ dst_sd->type = src_sd->type;
+ dst_sd->size = src_sd->size;
+ codec->nb_coded_side_data++;
+ }
+ }
+
return 0;
}
diff --git a/libavcodec/codec_par.h b/libavcodec/codec_par.h
index add90fdb1e..169e595b7c 100644
--- a/libavcodec/codec_par.h
+++ b/libavcodec/codec_par.h
@@ -29,6 +29,7 @@
#include "libavutil/pixfmt.h"
#include "codec_id.h"
+#include "packet.h"
/**
* @addtogroup lavc_core
@@ -223,6 +224,11 @@ typedef struct AVCodecParameters {
* when no higher-level timing information is available.
*/
AVRational framerate;
+
+ /**
+ * Additional data associated with the entire stream.
+ */
+ AVPacketSideDataSet side_data;
} AVCodecParameters;
/**
--
2.42.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] 28+ messages in thread* Re: [FFmpeg-devel] [PATCH 02/10] avcodec/codec_par: add side data to AVCodecParameters
2023-09-06 17:44 ` [FFmpeg-devel] [PATCH 02/10] avcodec/codec_par: add side data to AVCodecParameters James Almer
@ 2023-09-11 17:45 ` Andreas Rheinhardt
2023-09-12 16:34 ` James Almer
0 siblings, 1 reply; 28+ messages in thread
From: Andreas Rheinhardt @ 2023-09-11 17:45 UTC (permalink / raw)
To: ffmpeg-devel
James Almer:
> This will simplify the propagation of side data to decoders and from encoders.
> Global side data will now reside in the AVCodecContext, thus be available
> during init(), removing the need to propagate it inside packets.
>
> Global and frame specific side data will therefore be distinct.
>
> Signed-off-by: James Almer <jamrial@gmail.com>
> ---
> libavcodec/codec_par.c | 81 ++++++++++++++++++++++++++++++++++++++++++
> libavcodec/codec_par.h | 6 ++++
> 2 files changed, 87 insertions(+)
>
> diff --git a/libavcodec/codec_par.c b/libavcodec/codec_par.c
> index a38a475dc7..c0c941c2b6 100644
> --- a/libavcodec/codec_par.c
> +++ b/libavcodec/codec_par.c
> @@ -27,11 +27,13 @@
> #include "libavutil/mem.h"
> #include "avcodec.h"
> #include "codec_par.h"
> +#include "packet.h"
>
> static void codec_parameters_reset(AVCodecParameters *par)
> {
> av_freep(&par->extradata);
> av_channel_layout_uninit(&par->ch_layout);
> + av_packet_side_data_set_free(&par->side_data);
>
> memset(par, 0, sizeof(*par));
>
> @@ -82,6 +84,8 @@ int avcodec_parameters_copy(AVCodecParameters *dst, const AVCodecParameters *src
> dst->ch_layout = (AVChannelLayout){0};
> dst->extradata = NULL;
> dst->extradata_size = 0;
> + dst->side_data.sd = NULL;
> + dst->side_data.nb_sd = 0;
> if (src->extradata) {
> dst->extradata = av_mallocz(src->extradata_size + AV_INPUT_BUFFER_PADDING_SIZE);
> if (!dst->extradata)
> @@ -89,6 +93,32 @@ int avcodec_parameters_copy(AVCodecParameters *dst, const AVCodecParameters *src
> memcpy(dst->extradata, src->extradata, src->extradata_size);
> dst->extradata_size = src->extradata_size;
> }
> + if (src->side_data.nb_sd) {
> + const AVPacketSideDataSet *src_set = &src->side_data;
> + AVPacketSideDataSet *dst_set = &dst->side_data;
> +
> + dst_set->sd = av_calloc(src_set->nb_sd, sizeof(*dst_set->sd));
> + if (!dst_set->sd)
> + return AVERROR(ENOMEM);
> +
> + for (int i = 0; i < src_set->nb_sd; i++) {
> + const AVPacketSideData *src_sd = src_set->sd[i];
> + AVPacketSideData *dst_sd = av_mallocz(sizeof(*dst_sd));
> +
> + if (!dst_sd)
> + return AVERROR(ENOMEM);
> +
> + dst_sd->data = av_memdup(src_sd->data, src_sd->size);
> + if (!dst_sd->data) {
> + return AVERROR(ENOMEM);
> + av_free(dst_sd);
> + }
> +
> + dst_sd->type = src_sd->type;
> + dst_sd->size = src_sd->size;
> + dst_set->sd[dst_set->nb_sd++] = dst_sd;
> + }
> + }
>
> ret = av_channel_layout_copy(&dst->ch_layout, &src->ch_layout);
> if (ret < 0)
> @@ -178,6 +208,32 @@ FF_ENABLE_DEPRECATION_WARNINGS
> par->extradata_size = codec->extradata_size;
> }
>
> + if (codec->nb_coded_side_data) {
> + AVPacketSideDataSet *dst_set = &par->side_data;
> +
> + dst_set->sd = av_calloc(codec->nb_coded_side_data, sizeof(*dst_set->sd));
> + if (!dst_set->sd)
> + return AVERROR(ENOMEM);
> +
> + for (int i = 0; i < codec->nb_coded_side_data; i++) {
> + const AVPacketSideData *src_sd = &codec->coded_side_data[i];
> + AVPacketSideData *dst_sd = av_mallocz(sizeof(*dst_sd));
> +
> + if (!dst_sd)
> + return AVERROR(ENOMEM);
> +
> + dst_sd->data = av_memdup(src_sd->data, src_sd->size);
> + if (!dst_sd->data) {
> + return AVERROR(ENOMEM);
> + av_free(dst_sd);
No compiler warning for this?
> + }
> +
> + dst_sd->type = src_sd->type;
> + dst_sd->size = src_sd->size;
> + dst_set->sd[dst_set->nb_sd++] = dst_sd;
> + }
> + }
> +
> return 0;
> }
>
> @@ -262,5 +318,30 @@ FF_ENABLE_DEPRECATION_WARNINGS
> codec->extradata_size = par->extradata_size;
> }
>
> + for (int i = 0; i < codec->nb_coded_side_data; i++)
> + av_free(codec->coded_side_data[i].data);
> + av_freep(&codec->coded_side_data);
> + codec->nb_coded_side_data = 0;
> + if (par->side_data.nb_sd) {
> + const AVPacketSideDataSet *src_set = &par->side_data;
> +
> + codec->coded_side_data = av_calloc(src_set->nb_sd, sizeof(*codec->coded_side_data));
> + if (!codec->coded_side_data)
> + return AVERROR(ENOMEM);
> +
> + for (int i = 0; i < src_set->nb_sd; i++) {
> + const AVPacketSideData *src_sd = src_set->sd[i];
> + AVPacketSideData *dst_sd = &codec->coded_side_data[i];
> +
> + dst_sd->data = av_memdup(src_sd->data, src_sd->size);
> + if (!dst_sd->data)
> + return AVERROR(ENOMEM);
> +
> + dst_sd->type = src_sd->type;
> + dst_sd->size = src_sd->size;
> + codec->nb_coded_side_data++;
> + }
> + }
> +
> return 0;
> }
> diff --git a/libavcodec/codec_par.h b/libavcodec/codec_par.h
> index add90fdb1e..169e595b7c 100644
> --- a/libavcodec/codec_par.h
> +++ b/libavcodec/codec_par.h
> @@ -29,6 +29,7 @@
> #include "libavutil/pixfmt.h"
>
> #include "codec_id.h"
> +#include "packet.h"
>
> /**
> * @addtogroup lavc_core
> @@ -223,6 +224,11 @@ typedef struct AVCodecParameters {
> * when no higher-level timing information is available.
> */
> AVRational framerate;
> +
> + /**
> + * Additional data associated with the entire stream.
> + */
> + AVPacketSideDataSet side_data;
> } AVCodecParameters;
>
> /**
_______________________________________________
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] 28+ messages in thread* Re: [FFmpeg-devel] [PATCH 02/10] avcodec/codec_par: add side data to AVCodecParameters
2023-09-11 17:45 ` Andreas Rheinhardt
@ 2023-09-12 16:34 ` James Almer
0 siblings, 0 replies; 28+ messages in thread
From: James Almer @ 2023-09-12 16:34 UTC (permalink / raw)
To: ffmpeg-devel
On 9/11/2023 2:45 PM, Andreas Rheinhardt wrote:
> James Almer:
>> This will simplify the propagation of side data to decoders and from encoders.
>> Global side data will now reside in the AVCodecContext, thus be available
>> during init(), removing the need to propagate it inside packets.
>>
>> Global and frame specific side data will therefore be distinct.
>>
>> Signed-off-by: James Almer <jamrial@gmail.com>
>> ---
>> libavcodec/codec_par.c | 81 ++++++++++++++++++++++++++++++++++++++++++
>> libavcodec/codec_par.h | 6 ++++
>> 2 files changed, 87 insertions(+)
>>
>> diff --git a/libavcodec/codec_par.c b/libavcodec/codec_par.c
>> index a38a475dc7..c0c941c2b6 100644
>> --- a/libavcodec/codec_par.c
>> +++ b/libavcodec/codec_par.c
>> @@ -27,11 +27,13 @@
>> #include "libavutil/mem.h"
>> #include "avcodec.h"
>> #include "codec_par.h"
>> +#include "packet.h"
>>
>> static void codec_parameters_reset(AVCodecParameters *par)
>> {
>> av_freep(&par->extradata);
>> av_channel_layout_uninit(&par->ch_layout);
>> + av_packet_side_data_set_free(&par->side_data);
>>
>> memset(par, 0, sizeof(*par));
>>
>> @@ -82,6 +84,8 @@ int avcodec_parameters_copy(AVCodecParameters *dst, const AVCodecParameters *src
>> dst->ch_layout = (AVChannelLayout){0};
>> dst->extradata = NULL;
>> dst->extradata_size = 0;
>> + dst->side_data.sd = NULL;
>> + dst->side_data.nb_sd = 0;
>> if (src->extradata) {
>> dst->extradata = av_mallocz(src->extradata_size + AV_INPUT_BUFFER_PADDING_SIZE);
>> if (!dst->extradata)
>> @@ -89,6 +93,32 @@ int avcodec_parameters_copy(AVCodecParameters *dst, const AVCodecParameters *src
>> memcpy(dst->extradata, src->extradata, src->extradata_size);
>> dst->extradata_size = src->extradata_size;
>> }
>> + if (src->side_data.nb_sd) {
>> + const AVPacketSideDataSet *src_set = &src->side_data;
>> + AVPacketSideDataSet *dst_set = &dst->side_data;
>> +
>> + dst_set->sd = av_calloc(src_set->nb_sd, sizeof(*dst_set->sd));
>> + if (!dst_set->sd)
>> + return AVERROR(ENOMEM);
>> +
>> + for (int i = 0; i < src_set->nb_sd; i++) {
>> + const AVPacketSideData *src_sd = src_set->sd[i];
>> + AVPacketSideData *dst_sd = av_mallocz(sizeof(*dst_sd));
>> +
>> + if (!dst_sd)
>> + return AVERROR(ENOMEM);
>> +
>> + dst_sd->data = av_memdup(src_sd->data, src_sd->size);
>> + if (!dst_sd->data) {
>> + return AVERROR(ENOMEM);
>> + av_free(dst_sd);
>> + }
>> +
>> + dst_sd->type = src_sd->type;
>> + dst_sd->size = src_sd->size;
>> + dst_set->sd[dst_set->nb_sd++] = dst_sd;
>> + }
>> + }
>>
>> ret = av_channel_layout_copy(&dst->ch_layout, &src->ch_layout);
>> if (ret < 0)
>> @@ -178,6 +208,32 @@ FF_ENABLE_DEPRECATION_WARNINGS
>> par->extradata_size = codec->extradata_size;
>> }
>>
>> + if (codec->nb_coded_side_data) {
>> + AVPacketSideDataSet *dst_set = &par->side_data;
>> +
>> + dst_set->sd = av_calloc(codec->nb_coded_side_data, sizeof(*dst_set->sd));
>> + if (!dst_set->sd)
>> + return AVERROR(ENOMEM);
>> +
>> + for (int i = 0; i < codec->nb_coded_side_data; i++) {
>> + const AVPacketSideData *src_sd = &codec->coded_side_data[i];
>> + AVPacketSideData *dst_sd = av_mallocz(sizeof(*dst_sd));
>> +
>> + if (!dst_sd)
>> + return AVERROR(ENOMEM);
>> +
>> + dst_sd->data = av_memdup(src_sd->data, src_sd->size);
>> + if (!dst_sd->data) {
>> + return AVERROR(ENOMEM);
>> + av_free(dst_sd);
>
> No compiler warning for this?
Good catch. And no, i don't get one with GCC 13.
>
>> + }
>> +
>> + dst_sd->type = src_sd->type;
>> + dst_sd->size = src_sd->size;
>> + dst_set->sd[dst_set->nb_sd++] = dst_sd;
>> + }
>> + }
>> +
>> return 0;
>> }
>>
>> @@ -262,5 +318,30 @@ FF_ENABLE_DEPRECATION_WARNINGS
>> codec->extradata_size = par->extradata_size;
>> }
>>
>> + for (int i = 0; i < codec->nb_coded_side_data; i++)
>> + av_free(codec->coded_side_data[i].data);
>> + av_freep(&codec->coded_side_data);
>> + codec->nb_coded_side_data = 0;
>> + if (par->side_data.nb_sd) {
>> + const AVPacketSideDataSet *src_set = &par->side_data;
>> +
>> + codec->coded_side_data = av_calloc(src_set->nb_sd, sizeof(*codec->coded_side_data));
>> + if (!codec->coded_side_data)
>> + return AVERROR(ENOMEM);
>> +
>> + for (int i = 0; i < src_set->nb_sd; i++) {
>> + const AVPacketSideData *src_sd = src_set->sd[i];
>> + AVPacketSideData *dst_sd = &codec->coded_side_data[i];
>> +
>> + dst_sd->data = av_memdup(src_sd->data, src_sd->size);
>> + if (!dst_sd->data)
>> + return AVERROR(ENOMEM);
>> +
>> + dst_sd->type = src_sd->type;
>> + dst_sd->size = src_sd->size;
>> + codec->nb_coded_side_data++;
>> + }
>> + }
>> +
>> return 0;
>> }
>> diff --git a/libavcodec/codec_par.h b/libavcodec/codec_par.h
>> index add90fdb1e..169e595b7c 100644
>> --- a/libavcodec/codec_par.h
>> +++ b/libavcodec/codec_par.h
>> @@ -29,6 +29,7 @@
>> #include "libavutil/pixfmt.h"
>>
>> #include "codec_id.h"
>> +#include "packet.h"
>>
>> /**
>> * @addtogroup lavc_core
>> @@ -223,6 +224,11 @@ typedef struct AVCodecParameters {
>> * when no higher-level timing information is available.
>> */
>> AVRational framerate;
>> +
>> + /**
>> + * Additional data associated with the entire stream.
>> + */
>> + AVPacketSideDataSet side_data;
>> } AVCodecParameters;
>>
>> /**
>
> _______________________________________________
> 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".
_______________________________________________
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] 28+ messages in thread
* [FFmpeg-devel] [PATCH 03/10] avformat/avformat: use the side data from AVStream.codecpar
2023-09-06 17:44 [FFmpeg-devel] [PATCH 00/10 v4][RFC] AVCodecContext and AVCodecParameters side data James Almer
2023-09-06 17:44 ` [FFmpeg-devel] [PATCH 01/10] avcodec/packet: add side data set struct and helpers James Almer
2023-09-06 17:44 ` [FFmpeg-devel] [PATCH 02/10] avcodec/codec_par: add side data to AVCodecParameters James Almer
@ 2023-09-06 17:44 ` James Almer
2023-09-11 19:19 ` Andreas Rheinhardt
2023-09-06 17:44 ` [FFmpeg-devel] [PATCH 04/10] fftools/ffmpeg: stop using AVStream.side_data James Almer
` (8 subsequent siblings)
11 siblings, 1 reply; 28+ messages in thread
From: James Almer @ 2023-09-06 17:44 UTC (permalink / raw)
To: ffmpeg-devel
Deprecate AVStream.side_data and its helpers in favor of the AVStream's
codecpar.side_data.
This will considerably simplify the propagation of global side data to decoders
and from encoders. Instead of having to do it inside packets, it will be
available during init().
Global and frame specific side data will therefore be distinct.
Signed-off-by: James Almer <jamrial@gmail.com>
---
libavdevice/android_camera.c | 9 ++--
libavformat/avformat.c | 42 ++++---------------
libavformat/avformat.h | 40 +++++++++++++++++-
libavformat/concatdec.c | 1 -
libavformat/dashdec.c | 11 -----
libavformat/demux.c | 54 +++++++++++++++---------
libavformat/demux_utils.c | 4 ++
libavformat/dovi_isom.c | 8 ++--
libavformat/dump.c | 6 +--
libavformat/hls.c | 11 -----
libavformat/hlsenc.c | 11 ++---
libavformat/internal.h | 4 ++
libavformat/matroskadec.c | 45 ++++++++++----------
libavformat/matroskaenc.c | 48 ++++++++++++---------
libavformat/mov.c | 81 ++++++++++++++++++------------------
libavformat/movenc.c | 73 +++++++++++++++-----------------
libavformat/mp3enc.c | 8 ++--
libavformat/mpegenc.c | 18 +++++---
libavformat/mpegts.c | 8 ++--
libavformat/mux.c | 19 +++++++++
libavformat/mxfdec.c | 22 +++++-----
libavformat/mxfenc.c | 8 ++--
libavformat/options.c | 2 +
libavformat/replaygain.c | 9 ++--
libavformat/seek.c | 2 +
libavformat/version_major.h | 1 +
26 files changed, 294 insertions(+), 251 deletions(-)
diff --git a/libavdevice/android_camera.c b/libavdevice/android_camera.c
index 1934999c18..012b40aa37 100644
--- a/libavdevice/android_camera.c
+++ b/libavdevice/android_camera.c
@@ -638,7 +638,7 @@ static int wait_for_image_format(AVFormatContext *avctx)
static int add_display_matrix(AVFormatContext *avctx, AVStream *st)
{
AndroidCameraCtx *ctx = avctx->priv_data;
- uint8_t *side_data;
+ AVPacketSideData *side_data;
int32_t display_matrix[9];
av_display_rotation_set(display_matrix, ctx->sensor_orientation);
@@ -647,14 +647,15 @@ static int add_display_matrix(AVFormatContext *avctx, AVStream *st)
av_display_matrix_flip(display_matrix, 1, 0);
}
- side_data = av_stream_new_side_data(st,
- AV_PKT_DATA_DISPLAYMATRIX, sizeof(display_matrix));
+ side_data = av_packet_side_data_set_new(&st->codecpar->side_data,
+ AV_PKT_DATA_DISPLAYMATRIX,
+ sizeof(display_matrix), 0);
if (!side_data) {
return AVERROR(ENOMEM);
}
- memcpy(side_data, display_matrix, sizeof(display_matrix));
+ memcpy(side_data->data, display_matrix, sizeof(display_matrix));
return 0;
}
diff --git a/libavformat/avformat.c b/libavformat/avformat.c
index 356b4de931..3afc5afd7f 100644
--- a/libavformat/avformat.c
+++ b/libavformat/avformat.c
@@ -46,9 +46,13 @@ void ff_free_stream(AVStream **pst)
if (!st)
return;
+#if FF_API_AVSTREAM_SIDE_DATA
+FF_DISABLE_DEPRECATION_WARNINGS
for (int i = 0; i < st->nb_side_data; i++)
av_freep(&st->side_data[i].data);
av_freep(&st->side_data);
+FF_ENABLE_DEPRECATION_WARNINGS
+#endif
if (st->attached_pic.data)
av_packet_unref(&st->attached_pic);
@@ -138,6 +142,8 @@ void avformat_free_context(AVFormatContext *s)
av_free(s);
}
+#if FF_API_AVSTREAM_SIDE_DATA
+FF_DISABLE_DEPRECATION_WARNINGS
uint8_t *av_stream_get_side_data(const AVStream *st,
enum AVPacketSideDataType type, size_t *size)
{
@@ -205,36 +211,8 @@ uint8_t *av_stream_new_side_data(AVStream *st, enum AVPacketSideDataType type,
return data;
}
-
-int ff_stream_side_data_copy(AVStream *dst, const AVStream *src)
-{
- /* Free existing side data*/
- for (int i = 0; i < dst->nb_side_data; i++)
- av_free(dst->side_data[i].data);
- av_freep(&dst->side_data);
- dst->nb_side_data = 0;
-
- /* Copy side data if present */
- if (src->nb_side_data) {
- dst->side_data = av_calloc(src->nb_side_data,
- sizeof(*dst->side_data));
- if (!dst->side_data)
- return AVERROR(ENOMEM);
- dst->nb_side_data = src->nb_side_data;
-
- for (int i = 0; i < src->nb_side_data; i++) {
- uint8_t *data = av_memdup(src->side_data[i].data,
- src->side_data[i].size);
- if (!data)
- return AVERROR(ENOMEM);
- dst->side_data[i].type = src->side_data[i].type;
- dst->side_data[i].size = src->side_data[i].size;
- dst->side_data[i].data = data;
- }
- }
-
- return 0;
-}
+FF_ENABLE_DEPRECATION_WARNINGS
+#endif
/**
* Copy all stream parameters from source to destination stream, with the
@@ -270,10 +248,6 @@ static int stream_params_copy(AVStream *dst, const AVStream *src)
if (ret < 0)
return ret;
- ret = ff_stream_side_data_copy(dst, src);
- if (ret < 0)
- return ret;
-
av_packet_unref(&dst->attached_pic);
if (src->attached_pic.data) {
ret = av_packet_ref(&dst->attached_pic, &src->attached_pic);
diff --git a/libavformat/avformat.h b/libavformat/avformat.h
index 1916aa2dc5..f78a027f64 100644
--- a/libavformat/avformat.h
+++ b/libavformat/avformat.h
@@ -164,6 +164,13 @@
* decoding functions avcodec_send_packet() or avcodec_decode_subtitle2() if the
* caller wishes to decode the data.
*
+ * There may be no overlap between the stream's @ref AVCodecParameters.side_data
+ * "side data" and @ref AVPacket.side_data "side data" in packets. I.e. a given
+ * side data is either exported by the demuxer in AVCodecParameters, then it never
+ * appears in the packets, or the side data is exported through the packets (always
+ * in the first packet where the value becomes known or changes), then it does not
+ * appear in AVCodecParameters.
+ *
* AVPacket.pts, AVPacket.dts and AVPacket.duration timing information will be
* set if known. They may also be unset (i.e. AV_NOPTS_VALUE for
* pts/dts, 0 for duration) if the stream does not provide them. The timing
@@ -209,6 +216,11 @@
* AVCodecParameters, rather than using @ref avcodec_parameters_copy() during
* remuxing: there is no guarantee that the codec context values remain valid
* for both input and output format contexts.
+ * - There may be no overlap between AVCodecParameters.side_data and side data in
+ * packets. I.e. a given side data is either set by the caller in
+ * AVCodecParameters, then it never appears in the packets, or the side data is
+ * sent through the packets (always in the first packet where the value becomes
+ * known or changes), then it does not appear in AVCodecParameters.
* - The caller may fill in additional information, such as @ref
* AVFormatContext.metadata "global" or @ref AVStream.metadata "per-stream"
* metadata, @ref AVFormatContext.chapters "chapters", @ref
@@ -937,6 +949,7 @@ typedef struct AVStream {
*/
AVPacket attached_pic;
+#if FF_API_AVSTREAM_SIDE_DATA
/**
* An array of side data that applies to the whole stream (i.e. the
* container does not allow it to change between packets).
@@ -953,13 +966,20 @@ typedef struct AVStream {
*
* Freed by libavformat in avformat_free_context().
*
- * @see av_format_inject_global_side_data()
+ * @deprecated use AVStream's @ref AVCodecParameters.side_data
+ * "codecpar side data".
*/
+ attribute_deprecated
AVPacketSideData *side_data;
/**
* The number of elements in the AVStream.side_data array.
+ *
+ * @deprecated use AVStream's @ref AVCodecParameters.side_data
+ * "codecpar side data".
*/
+ attribute_deprecated
int nb_side_data;
+#endif
/**
* Flags indicating events happening on the stream, a combination of
@@ -1715,11 +1735,18 @@ typedef struct AVFormatContext {
int (*io_close2)(struct AVFormatContext *s, AVIOContext *pb);
} AVFormatContext;
+#if FF_API_AVSTREAM_SIDE_DATA
/**
* This function will cause global side data to be injected in the next packet
* of each stream as well as after any subsequent seek.
+ *
+ * @deprecated global side data is always available in every AVStream's
+ * @ref AVCodecParameters.side_data "codecpar side data" array.
+ * @see av_packet_side_data_set_get()
*/
+attribute_deprecated
void av_format_inject_global_side_data(AVFormatContext *s);
+#endif
/**
* Returns the method used to set ctx->duration.
@@ -1844,6 +1871,7 @@ const AVClass *av_stream_get_class(void);
*/
AVStream *avformat_new_stream(AVFormatContext *s, const AVCodec *c);
+#if FF_API_AVSTREAM_SIDE_DATA
/**
* Wrap an existing array as stream side data.
*
@@ -1856,7 +1884,10 @@ AVStream *avformat_new_stream(AVFormatContext *s, const AVCodec *c);
*
* @return zero on success, a negative AVERROR code on failure. On failure,
* the stream is unchanged and the data remains owned by the caller.
+ * @deprecated use av_packet_side_data_set_add() with the stream's
+ * @ref AVCodecParameters.side_data "codecpar side data"
*/
+attribute_deprecated
int av_stream_add_side_data(AVStream *st, enum AVPacketSideDataType type,
uint8_t *data, size_t size);
@@ -1868,7 +1899,10 @@ int av_stream_add_side_data(AVStream *st, enum AVPacketSideDataType type,
* @param size side information size
*
* @return pointer to fresh allocated data or NULL otherwise
+ * @deprecated use av_packet_side_data_set_new() with the stream's
+ * @ref AVCodecParameters.side_data "codecpar side data"
*/
+attribute_deprecated
uint8_t *av_stream_new_side_data(AVStream *stream,
enum AVPacketSideDataType type, size_t size);
/**
@@ -1880,9 +1914,13 @@ uint8_t *av_stream_new_side_data(AVStream *stream,
* or to zero if the desired side data is not present.
*
* @return pointer to data if present or NULL otherwise
+ * @deprecated use av_packet_side_data_set_get() with the stream's
+ * @ref AVCodecParameters.side_data "codecpar side data"
*/
+attribute_deprecated
uint8_t *av_stream_get_side_data(const AVStream *stream,
enum AVPacketSideDataType type, size_t *size);
+#endif
AVProgram *av_new_program(AVFormatContext *s, int id);
diff --git a/libavformat/concatdec.c b/libavformat/concatdec.c
index 5d4f67d0ac..2aeb4f1dfd 100644
--- a/libavformat/concatdec.c
+++ b/libavformat/concatdec.c
@@ -194,7 +194,6 @@ static int copy_stream_props(AVStream *st, AVStream *source_st)
avpriv_set_pts_info(st, 64, source_st->time_base.num, source_st->time_base.den);
av_dict_copy(&st->metadata, source_st->metadata, 0);
- ff_stream_side_data_copy(st, source_st);
return 0;
}
diff --git a/libavformat/dashdec.c b/libavformat/dashdec.c
index 29d4680c68..2441087606 100644
--- a/libavformat/dashdec.c
+++ b/libavformat/dashdec.c
@@ -1952,17 +1952,6 @@ static int open_demux_for_component(AVFormatContext *s, struct representation *p
// copy disposition
st->disposition = ist->disposition;
-
- // copy side data
- for (int i = 0; i < ist->nb_side_data; i++) {
- const AVPacketSideData *sd_src = &ist->side_data[i];
- uint8_t *dst_data;
-
- dst_data = av_stream_new_side_data(st, sd_src->type, sd_src->size);
- if (!dst_data)
- return AVERROR(ENOMEM);
- memcpy(dst_data, sd_src->data, sd_src->size);
- }
}
return 0;
diff --git a/libavformat/demux.c b/libavformat/demux.c
index fcd5daf699..76d4558ad2 100644
--- a/libavformat/demux.c
+++ b/libavformat/demux.c
@@ -1409,9 +1409,10 @@ FF_ENABLE_DEPRECATION_WARNINGS
sti->skip_samples = 0;
}
+#if FF_API_AVSTREAM_SIDE_DATA
if (sti->inject_global_side_data) {
- for (int i = 0; i < st->nb_side_data; i++) {
- const AVPacketSideData *const src_sd = &st->side_data[i];
+ for (int i = 0; i < st->codecpar->side_data.nb_sd; i++) {
+ const AVPacketSideData *const src_sd = st->codecpar->side_data.sd[i];
uint8_t *dst_data;
if (av_packet_get_side_data(pkt, src_sd->type, NULL))
@@ -1427,6 +1428,7 @@ FF_ENABLE_DEPRECATION_WARNINGS
}
sti->inject_global_side_data = 0;
}
+#endif
}
if (!si->metafree) {
@@ -2431,19 +2433,6 @@ static int extract_extradata(FFFormatContext *si, AVStream *st, const AVPacket *
return 0;
}
-static int add_coded_side_data(AVStream *st, AVCodecContext *avctx)
-{
- for (int i = 0; i < avctx->nb_coded_side_data; i++) {
- const AVPacketSideData *const sd_src = &avctx->coded_side_data[i];
- uint8_t *dst_data;
- dst_data = av_stream_new_side_data(st, sd_src->type, sd_src->size);
- if (!dst_data)
- return AVERROR(ENOMEM);
- memcpy(dst_data, sd_src->data, sd_src->size);
- }
- return 0;
-}
-
int avformat_find_stream_info(AVFormatContext *ic, AVDictionary **options)
{
FFFormatContext *const si = ffformatcontext(ic);
@@ -2969,9 +2958,6 @@ int avformat_find_stream_info(AVFormatContext *ic, AVDictionary **options)
if (sti->avctx_inited) {
ret = avcodec_parameters_from_context(st->codecpar, sti->avctx);
- if (ret < 0)
- goto find_stream_info_err;
- ret = add_coded_side_data(st, sti->avctx);
if (ret < 0)
goto find_stream_info_err;
@@ -2986,14 +2972,42 @@ int avformat_find_stream_info(AVFormatContext *ic, AVDictionary **options)
props->min_bitrate = sti->avctx->rc_min_rate;
if (sti->avctx->rc_max_rate > 0)
props->max_bitrate = sti->avctx->rc_max_rate;
- if (av_stream_add_side_data(st, AV_PKT_DATA_CPB_PROPERTIES,
- (uint8_t *)props, cpb_size))
+ if (!av_packet_side_data_set_add(&st->codecpar->side_data,
+ AV_PKT_DATA_CPB_PROPERTIES,
+ (uint8_t *)props, cpb_size, 0))
av_free(props);
}
}
}
sti->avctx_inited = 0;
+#if FF_API_AVSTREAM_SIDE_DATA
+FF_DISABLE_DEPRECATION_WARNINGS
+ if (st->codecpar->side_data.nb_sd > 0) {
+ const AVPacketSideDataSet *set = &st->codecpar->side_data;
+
+ av_assert0(!st->side_data && !st->nb_side_data);
+ st->side_data = av_calloc(set->nb_sd, sizeof(*st->side_data));
+ if (!st->side_data) {
+ ret = AVERROR(ENOMEM);
+ goto find_stream_info_err;
+ }
+
+ for (int j = 0; j < set->nb_sd; j++) {
+ uint8_t *data = av_memdup(set->sd[j]->data,
+ set->sd[j]->size);
+ if (!data) {
+ ret = AVERROR(ENOMEM);
+ goto find_stream_info_err;
+ }
+ st->side_data[j].type = set->sd[j]->type;
+ st->side_data[j].size = set->sd[j]->size;
+ st->side_data[j].data = data;
+ st->nb_side_data++;
+ }
+ }
+FF_ENABLE_DEPRECATION_WARNINGS
+#endif
}
find_stream_info_err:
diff --git a/libavformat/demux_utils.c b/libavformat/demux_utils.c
index 56cc6e15d8..2946e82295 100644
--- a/libavformat/demux_utils.c
+++ b/libavformat/demux_utils.c
@@ -80,6 +80,8 @@ AVChapter *avpriv_new_chapter(AVFormatContext *s, int64_t id, AVRational time_ba
return chapter;
}
+#if FF_API_AVSTREAM_SIDE_DATA
+FF_DISABLE_DEPRECATION_WARNINGS
void av_format_inject_global_side_data(AVFormatContext *s)
{
FFFormatContext *const si = ffformatcontext(s);
@@ -89,6 +91,8 @@ void av_format_inject_global_side_data(AVFormatContext *s)
ffstream(st)->inject_global_side_data = 1;
}
}
+FF_ENABLE_DEPRECATION_WARNINGS
+#endif
int avformat_queue_attached_pictures(AVFormatContext *s)
{
diff --git a/libavformat/dovi_isom.c b/libavformat/dovi_isom.c
index c8fdf566e4..8d03d0e632 100644
--- a/libavformat/dovi_isom.c
+++ b/libavformat/dovi_isom.c
@@ -34,7 +34,6 @@ int ff_isom_parse_dvcc_dvvc(void *logctx, AVStream *st,
uint32_t buf;
AVDOVIDecoderConfigurationRecord *dovi;
size_t dovi_size;
- int ret;
if (size > (1 << 30) || size < 4)
return AVERROR_INVALIDDATA;
@@ -64,11 +63,10 @@ int ff_isom_parse_dvcc_dvvc(void *logctx, AVStream *st,
dovi->dv_bl_signal_compatibility_id = 0;
}
- ret = av_stream_add_side_data(st, AV_PKT_DATA_DOVI_CONF,
- (uint8_t *)dovi, dovi_size);
- if (ret < 0) {
+ if (!av_packet_side_data_set_add(&st->codecpar->side_data, AV_PKT_DATA_DOVI_CONF,
+ (uint8_t *)dovi, dovi_size, 0)) {
av_free(dovi);
- return ret;
+ return AVERROR(ENOMEM);
}
av_log(logctx, AV_LOG_TRACE, "DOVI in dvcC/dvvC/dvwC box, version: %d.%d, profile: %d, level: %d, "
diff --git a/libavformat/dump.c b/libavformat/dump.c
index d31e4c2ec6..d6f32f0c68 100644
--- a/libavformat/dump.c
+++ b/libavformat/dump.c
@@ -431,11 +431,11 @@ static void dump_sidedata(void *ctx, const AVStream *st, const char *indent)
{
int i;
- if (st->nb_side_data)
+ if (st->codecpar->side_data.nb_sd)
av_log(ctx, AV_LOG_INFO, "%sSide data:\n", indent);
- for (i = 0; i < st->nb_side_data; i++) {
- const AVPacketSideData *sd = &st->side_data[i];
+ for (i = 0; i < st->codecpar->side_data.nb_sd; i++) {
+ const AVPacketSideData *sd = st->codecpar->side_data.sd[i];
av_log(ctx, AV_LOG_INFO, "%s ", indent);
switch (sd->type) {
diff --git a/libavformat/hls.c b/libavformat/hls.c
index c625e30291..8f80cf64f4 100644
--- a/libavformat/hls.c
+++ b/libavformat/hls.c
@@ -1851,17 +1851,6 @@ static int set_stream_info_from_input_stream(AVStream *st, struct playlist *pls,
av_dict_copy(&st->metadata, ist->metadata, 0);
- // copy side data
- for (int i = 0; i < ist->nb_side_data; i++) {
- const AVPacketSideData *sd_src = &ist->side_data[i];
- uint8_t *dst_data;
-
- dst_data = av_stream_new_side_data(st, sd_src->type, sd_src->size);
- if (!dst_data)
- return AVERROR(ENOMEM);
- memcpy(dst_data, sd_src->data, sd_src->size);
- }
-
ffstream(st)->need_context_update = 1;
return 0;
diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
index 27d97f5f72..fcc875e236 100644
--- a/libavformat/hlsenc.c
+++ b/libavformat/hlsenc.c
@@ -1351,16 +1351,17 @@ static const char* get_relative_url(const char *master_url, const char *media_ur
static int64_t get_stream_bit_rate(AVStream *stream)
{
- AVCPBProperties *props = (AVCPBProperties*)av_stream_get_side_data(
- stream,
- AV_PKT_DATA_CPB_PROPERTIES,
- NULL
+ AVPacketSideData *sd = av_packet_side_data_set_get(
+ &stream->codecpar->side_data,
+ AV_PKT_DATA_CPB_PROPERTIES
);
if (stream->codecpar->bit_rate)
return stream->codecpar->bit_rate;
- else if (props)
+ else if (sd) {
+ AVCPBProperties *props = (AVCPBProperties*)sd->data;
return props->max_bitrate;
+ }
return 0;
}
diff --git a/libavformat/internal.h b/libavformat/internal.h
index 53e70ccb53..399ec16ded 100644
--- a/libavformat/internal.h
+++ b/libavformat/internal.h
@@ -147,7 +147,9 @@ typedef struct FFFormatContext {
int missing_ts_warning;
#endif
+#if FF_API_AVSTREAM_SIDE_DATA
int inject_global_side_data;
+#endif
int avoid_negative_ts_use_pts;
@@ -354,10 +356,12 @@ typedef struct FFStream {
uint8_t dts_ordered;
uint8_t dts_misordered;
+#if FF_API_AVSTREAM_SIDE_DATA
/**
* Internal data to inject global side data
*/
int inject_global_side_data;
+#endif
/**
* display aspect ratio (0 if unknown)
diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
index fda77b0b89..20eb75bb6d 100644
--- a/libavformat/matroskadec.c
+++ b/libavformat/matroskadec.c
@@ -2175,7 +2175,6 @@ static int mkv_stereo3d_conv(AVStream *st, MatroskaVideoStereoModeType stereo_mo
STEREOMODE_STEREO3D_MAPPING(STEREO_MODE_CONV, NOTHING)
};
AVStereo3D *stereo;
- int ret;
stereo = av_stereo3d_alloc();
if (!stereo)
@@ -2184,11 +2183,10 @@ static int mkv_stereo3d_conv(AVStream *st, MatroskaVideoStereoModeType stereo_mo
stereo->type = stereo_mode_conv[stereo_mode].type;
stereo->flags = stereo_mode_conv[stereo_mode].flags;
- ret = av_stream_add_side_data(st, AV_PKT_DATA_STEREO3D, (uint8_t *)stereo,
- sizeof(*stereo));
- if (ret < 0) {
+ if (!av_packet_side_data_set_add(&st->codecpar->side_data, AV_PKT_DATA_STEREO3D,
+ (uint8_t *)stereo, sizeof(*stereo), 0)) {
av_freep(&stereo);
- return ret;
+ return AVERROR(ENOMEM);
}
return 0;
@@ -2235,28 +2233,26 @@ static int mkv_parse_video_color(AVStream *st, const MatroskaTrack *track) {
}
if (color->max_cll && color->max_fall) {
size_t size = 0;
- int ret;
AVContentLightMetadata *metadata = av_content_light_metadata_alloc(&size);
if (!metadata)
return AVERROR(ENOMEM);
- ret = av_stream_add_side_data(st, AV_PKT_DATA_CONTENT_LIGHT_LEVEL,
- (uint8_t *)metadata, size);
- if (ret < 0) {
+ if (!av_packet_side_data_set_add(&st->codecpar->side_data, AV_PKT_DATA_CONTENT_LIGHT_LEVEL,
+ (uint8_t *)metadata, size, 0)) {
av_freep(&metadata);
- return ret;
+ return AVERROR(ENOMEM);
}
metadata->MaxCLL = color->max_cll;
metadata->MaxFALL = color->max_fall;
}
if (has_mastering_primaries || has_mastering_luminance) {
- AVMasteringDisplayMetadata *metadata =
- (AVMasteringDisplayMetadata*) av_stream_new_side_data(
- st, AV_PKT_DATA_MASTERING_DISPLAY_METADATA,
- sizeof(AVMasteringDisplayMetadata));
- if (!metadata) {
+ AVMasteringDisplayMetadata *metadata;
+ AVPacketSideData *sd = av_packet_side_data_set_new(&st->codecpar->side_data,
+ AV_PKT_DATA_MASTERING_DISPLAY_METADATA,
+ sizeof(AVMasteringDisplayMetadata), 0);
+ if (!sd)
return AVERROR(ENOMEM);
- }
+ metadata = (AVMasteringDisplayMetadata*)sd->data;
memset(metadata, 0, sizeof(AVMasteringDisplayMetadata));
if (has_mastering_primaries) {
metadata->display_primaries[0][0] = av_d2q(mastering_meta->r_x, INT_MAX);
@@ -2282,6 +2278,7 @@ static int mkv_create_display_matrix(AVStream *st,
const MatroskaTrackVideoProjection *proj,
void *logctx)
{
+ AVPacketSideData *sd;
double pitch = proj->pitch, yaw = proj->yaw, roll = proj->roll;
int32_t *matrix;
int hflip;
@@ -2298,10 +2295,12 @@ static int mkv_create_display_matrix(AVStream *st,
st->index, yaw, pitch, roll);
return 0;
}
- matrix = (int32_t*)av_stream_new_side_data(st, AV_PKT_DATA_DISPLAYMATRIX,
- 9 * sizeof(*matrix));
- if (!matrix)
+ sd = av_packet_side_data_set_new(&st->codecpar->side_data,
+ AV_PKT_DATA_DISPLAYMATRIX,
+ 9 * sizeof(*matrix), 0);
+ if (!sd)
return AVERROR(ENOMEM);
+ matrix = (int32_t*)sd->data;
hflip = yaw != 0.0;
/* ProjectionPoseRoll is in the counter-clockwise direction
@@ -2326,7 +2325,6 @@ static int mkv_parse_video_projection(AVStream *st, const MatroskaTrack *track,
size_t spherical_size;
uint32_t l = 0, t = 0, r = 0, b = 0;
uint32_t padding = 0;
- int ret;
if (mkv_projection->private.size && priv_data[0] != 0) {
av_log(logctx, AV_LOG_WARNING, "Unknown spherical metadata\n");
@@ -2402,11 +2400,10 @@ static int mkv_parse_video_projection(AVStream *st, const MatroskaTrack *track,
spherical->bound_right = r;
spherical->bound_bottom = b;
- ret = av_stream_add_side_data(st, AV_PKT_DATA_SPHERICAL, (uint8_t *)spherical,
- spherical_size);
- if (ret < 0) {
+ if (!av_packet_side_data_set_add(&st->codecpar->side_data, AV_PKT_DATA_SPHERICAL,
+ (uint8_t *)spherical, spherical_size, 0)) {
av_freep(&spherical);
- return ret;
+ return AVERROR(ENOMEM);
}
return 0;
diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
index bf2ca7106b..b188b74e82 100644
--- a/libavformat/matroskaenc.c
+++ b/libavformat/matroskaenc.c
@@ -1327,7 +1327,7 @@ fail:
static void mkv_write_video_color(EbmlWriter *writer, const AVStream *st,
const AVCodecParameters *par)
{
- const void *side_data;
+ const AVPacketSideData *side_data;
ebml_writer_open_master(writer, MATROSKA_ID_VIDEOCOLOR);
@@ -1361,20 +1361,18 @@ static void mkv_write_video_color(EbmlWriter *writer, const AVStream *st,
(ypos >> 7) + 1);
}
- side_data = av_stream_get_side_data(st, AV_PKT_DATA_CONTENT_LIGHT_LEVEL,
- NULL);
+ side_data = av_packet_side_data_set_get(&st->codecpar->side_data, AV_PKT_DATA_CONTENT_LIGHT_LEVEL);
if (side_data) {
- const AVContentLightMetadata *metadata = side_data;
+ const AVContentLightMetadata *metadata = (AVContentLightMetadata *)side_data->data;
ebml_writer_add_uint(writer, MATROSKA_ID_VIDEOCOLORMAXCLL,
metadata->MaxCLL);
ebml_writer_add_uint(writer, MATROSKA_ID_VIDEOCOLORMAXFALL,
metadata->MaxFALL);
}
- side_data = av_stream_get_side_data(st, AV_PKT_DATA_MASTERING_DISPLAY_METADATA,
- NULL);
+ side_data = av_packet_side_data_set_get(&st->codecpar->side_data, AV_PKT_DATA_MASTERING_DISPLAY_METADATA);
if (side_data) {
- const AVMasteringDisplayMetadata *metadata = side_data;
+ const AVMasteringDisplayMetadata *metadata = (AVMasteringDisplayMetadata *)side_data->data;
ebml_writer_open_master(writer, MATROSKA_ID_VIDEOCOLORMASTERINGMETA);
if (metadata->has_primaries) {
ebml_writer_add_float(writer, MATROSKA_ID_VIDEOCOLOR_RX,
@@ -1410,12 +1408,15 @@ static void mkv_write_video_color(EbmlWriter *writer, const AVStream *st,
static void mkv_handle_rotation(void *logctx, const AVStream *st,
double *yaw, double *roll)
{
- const int32_t *matrix =
- (const int32_t*)av_stream_get_side_data(st, AV_PKT_DATA_DISPLAYMATRIX, NULL);
+ const int32_t *matrix;
+ const AVPacketSideData *side_data =
+ av_packet_side_data_set_get(&st->codecpar->side_data, AV_PKT_DATA_DISPLAYMATRIX);
- if (!matrix)
+ if (!side_data)
return;
+ matrix = (int32_t *)side_data->data;
+
/* Check whether this is an affine transformation */
if (matrix[2] || matrix[5])
goto ignore;
@@ -1462,13 +1463,13 @@ static int mkv_handle_spherical(void *logctx, EbmlWriter *writer,
const AVStream *st, uint8_t private[],
double *yaw, double *pitch, double *roll)
{
- const AVSphericalMapping *spherical =
- (const AVSphericalMapping *)av_stream_get_side_data(st, AV_PKT_DATA_SPHERICAL,
- NULL);
+ AVPacketSideData *sd = av_packet_side_data_set_get(&st->codecpar->side_data, AV_PKT_DATA_SPHERICAL);
+ const AVSphericalMapping *spherical;
- if (!spherical)
+ if (!sd)
return 0;
+ spherical = (const AVSphericalMapping *)sd->data;
if (spherical->projection != AV_SPHERICAL_EQUIRECTANGULAR &&
spherical->projection != AV_SPHERICAL_EQUIRECTANGULAR_TILE &&
spherical->projection != AV_SPHERICAL_CUBEMAP) {
@@ -1628,6 +1629,7 @@ static int mkv_write_stereo_mode(AVFormatContext *s, EbmlWriter *writer,
format = stereo_mode;
}
} else {
+ AVPacketSideData *sd;
const AVStereo3D *stereo;
/* The following macro presumes all MATROSKA_VIDEO_STEREOMODE_TYPE_*
* values to be in the range 0..254. */
@@ -1639,11 +1641,12 @@ static int mkv_write_stereo_mode(AVFormatContext *s, EbmlWriter *writer,
};
int fmt;
- stereo = (const AVStereo3D*)av_stream_get_side_data(st, AV_PKT_DATA_STEREO3D,
- NULL);
- if (!stereo)
+ sd = av_packet_side_data_set_get(&st->codecpar->side_data, AV_PKT_DATA_STEREO3D);
+ if (!sd)
return 0;
+ stereo = (const AVStereo3D*)sd->data;
+
/* A garbage AVStereo3D or something with no Matroska analogon. */
if ((unsigned)stereo->type >= FF_ARRAY_ELEMS(conversion_table))
return 0;
@@ -1681,6 +1684,7 @@ static void mkv_write_blockadditionmapping(AVFormatContext *s, const MatroskaMux
{
#if CONFIG_MATROSKA_MUXER
const AVDOVIDecoderConfigurationRecord *dovi;
+ const AVPacketSideData *sd;
if (IS_SEEKABLE(s->pb, mkv)) {
track->blockadditionmapping_offset = avio_tell(pb);
@@ -1697,9 +1701,13 @@ static void mkv_write_blockadditionmapping(AVFormatContext *s, const MatroskaMux
}
}
- dovi = (const AVDOVIDecoderConfigurationRecord *)
- av_stream_get_side_data(st, AV_PKT_DATA_DOVI_CONF, NULL);
- if (dovi && dovi->dv_profile <= 10) {
+ sd = av_packet_side_data_set_get(&st->codecpar->side_data, AV_PKT_DATA_DOVI_CONF);
+
+ if (!sd)
+ return;
+
+ dovi = (const AVDOVIDecoderConfigurationRecord *)sd->data;
+ if (dovi->dv_profile <= 10) {
ebml_master mapping;
uint8_t buf[ISOM_DVCC_DVVC_SIZE];
uint32_t type;
diff --git a/libavformat/mov.c b/libavformat/mov.c
index be9975f297..a08b6c77ee 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -799,6 +799,7 @@ static int mov_read_esds(MOVContext *c, AVIOContext *pb, MOVAtom atom)
static int mov_read_dac3(MOVContext *c, AVIOContext *pb, MOVAtom atom)
{
AVStream *st;
+ AVPacketSideData *sd;
enum AVAudioServiceType *ast;
int ac3info, acmod, lfeon, bsmod;
uint64_t mask;
@@ -807,11 +808,13 @@ static int mov_read_dac3(MOVContext *c, AVIOContext *pb, MOVAtom atom)
return 0;
st = c->fc->streams[c->fc->nb_streams-1];
- ast = (enum AVAudioServiceType*)av_stream_new_side_data(st, AV_PKT_DATA_AUDIO_SERVICE_TYPE,
- sizeof(*ast));
- if (!ast)
+ sd = av_packet_side_data_set_new(&st->codecpar->side_data,
+ AV_PKT_DATA_AUDIO_SERVICE_TYPE,
+ sizeof(*ast), 0);
+ if (!sd)
return AVERROR(ENOMEM);
+ ast = (enum AVAudioServiceType*)sd->data;
ac3info = avio_rb24(pb);
bsmod = (ac3info >> 14) & 0x7;
acmod = (ac3info >> 11) & 0x7;
@@ -833,6 +836,7 @@ static int mov_read_dac3(MOVContext *c, AVIOContext *pb, MOVAtom atom)
static int mov_read_dec3(MOVContext *c, AVIOContext *pb, MOVAtom atom)
{
AVStream *st;
+ AVPacketSideData *sd;
enum AVAudioServiceType *ast;
int eac3info, acmod, lfeon, bsmod;
uint64_t mask;
@@ -841,11 +845,14 @@ static int mov_read_dec3(MOVContext *c, AVIOContext *pb, MOVAtom atom)
return 0;
st = c->fc->streams[c->fc->nb_streams-1];
- ast = (enum AVAudioServiceType*)av_stream_new_side_data(st, AV_PKT_DATA_AUDIO_SERVICE_TYPE,
- sizeof(*ast));
- if (!ast)
+ sd = av_packet_side_data_set_new(&st->codecpar->side_data,
+ AV_PKT_DATA_AUDIO_SERVICE_TYPE,
+ sizeof(*ast), 0);
+ if (!sd)
return AVERROR(ENOMEM);
+ ast = (enum AVAudioServiceType*)sd->data;
+
/* No need to parse fields for additional independent substreams and its
* associated dependent substreams since libavcodec's E-AC-3 decoder
* does not support them yet. */
@@ -1747,7 +1754,6 @@ static int mov_read_pcmc(MOVContext *c, AVIOContext *pb, MOVAtom atom)
static int mov_read_colr(MOVContext *c, AVIOContext *pb, MOVAtom atom)
{
AVStream *st;
- uint8_t *icc_profile;
char color_parameter_type[5] = { 0 };
uint16_t color_primaries, color_trc, color_matrix;
int ret;
@@ -1768,10 +1774,12 @@ static int mov_read_colr(MOVContext *c, AVIOContext *pb, MOVAtom atom)
}
if (!strncmp(color_parameter_type, "prof", 4)) {
- icc_profile = av_stream_new_side_data(st, AV_PKT_DATA_ICC_PROFILE, atom.size - 4);
- if (!icc_profile)
+ AVPacketSideData *sd = av_packet_side_data_set_new(&st->codecpar->side_data,
+ AV_PKT_DATA_ICC_PROFILE,
+ atom.size - 4, 0);
+ if (!sd)
return AVERROR(ENOMEM);
- ret = ffio_read_size(pb, icc_profile, atom.size - 4);
+ ret = ffio_read_size(pb, sd->data, atom.size - 4);
if (ret < 0)
return ret;
} else {
@@ -6838,8 +6846,9 @@ static int mov_read_pssh(MOVContext *c, AVIOContext *pb, MOVAtom atom)
AVEncryptionInitInfo *info, *old_init_info;
uint8_t **key_ids;
AVStream *st;
- uint8_t *side_data, *extra_data, *old_side_data;
- size_t side_data_size, old_side_data_size;
+ AVPacketSideData *old_side_data;
+ uint8_t *side_data, *extra_data;
+ size_t side_data_size;
int ret = 0;
unsigned int version, kid_count, extra_data_size, alloc_size = 0;
@@ -6917,9 +6926,9 @@ static int mov_read_pssh(MOVContext *c, AVIOContext *pb, MOVAtom atom)
info->data_size = extra_data_size;
// If there is existing initialization data, append to the list.
- old_side_data = av_stream_get_side_data(st, AV_PKT_DATA_ENCRYPTION_INIT_INFO, &old_side_data_size);
+ old_side_data = av_packet_side_data_set_get(&st->codecpar->side_data, AV_PKT_DATA_ENCRYPTION_INIT_INFO);
if (old_side_data) {
- old_init_info = av_encryption_init_info_get_side_data(old_side_data, old_side_data_size);
+ old_init_info = av_encryption_init_info_get_side_data(old_side_data->data, old_side_data->size);
if (old_init_info) {
// Append to the end of the list.
for (AVEncryptionInitInfo *cur = old_init_info;; cur = cur->next) {
@@ -6941,9 +6950,8 @@ static int mov_read_pssh(MOVContext *c, AVIOContext *pb, MOVAtom atom)
ret = AVERROR(ENOMEM);
goto finish;
}
- ret = av_stream_add_side_data(st, AV_PKT_DATA_ENCRYPTION_INIT_INFO,
- side_data, side_data_size);
- if (ret < 0)
+ if (!av_packet_side_data_set_add(&st->codecpar->side_data, AV_PKT_DATA_ENCRYPTION_INIT_INFO,
+ side_data, side_data_size, 0))
av_free(side_data);
finish:
@@ -8720,46 +8728,37 @@ static int mov_read_header(AVFormatContext *s)
break;
case AVMEDIA_TYPE_VIDEO:
if (sc->display_matrix) {
- err = av_stream_add_side_data(st, AV_PKT_DATA_DISPLAYMATRIX, (uint8_t*)sc->display_matrix,
- sizeof(int32_t) * 9);
- if (err < 0)
- return err;
+ if (!av_packet_side_data_set_add(&st->codecpar->side_data, AV_PKT_DATA_DISPLAYMATRIX,
+ (uint8_t*)sc->display_matrix, sizeof(int32_t) * 9, 0))
+ return AVERROR(ENOMEM);
sc->display_matrix = NULL;
}
if (sc->stereo3d) {
- err = av_stream_add_side_data(st, AV_PKT_DATA_STEREO3D,
- (uint8_t *)sc->stereo3d,
- sizeof(*sc->stereo3d));
- if (err < 0)
- return err;
+ if (!av_packet_side_data_set_add(&st->codecpar->side_data, AV_PKT_DATA_STEREO3D,
+ (uint8_t *)sc->stereo3d, sizeof(*sc->stereo3d), 0))
+ return AVERROR(ENOMEM);
sc->stereo3d = NULL;
}
if (sc->spherical) {
- err = av_stream_add_side_data(st, AV_PKT_DATA_SPHERICAL,
- (uint8_t *)sc->spherical,
- sc->spherical_size);
- if (err < 0)
- return err;
+ if (!av_packet_side_data_set_add(&st->codecpar->side_data, AV_PKT_DATA_SPHERICAL,
+ (uint8_t *)sc->spherical, sc->spherical_size, 0))
+ return AVERROR(ENOMEM);
sc->spherical = NULL;
}
if (sc->mastering) {
- err = av_stream_add_side_data(st, AV_PKT_DATA_MASTERING_DISPLAY_METADATA,
- (uint8_t *)sc->mastering,
- sizeof(*sc->mastering));
- if (err < 0)
- return err;
+ if (!av_packet_side_data_set_add(&st->codecpar->side_data, AV_PKT_DATA_MASTERING_DISPLAY_METADATA,
+ (uint8_t *)sc->mastering, sizeof(*sc->mastering), 0))
+ return AVERROR(ENOMEM);
sc->mastering = NULL;
}
if (sc->coll) {
- err = av_stream_add_side_data(st, AV_PKT_DATA_CONTENT_LIGHT_LEVEL,
- (uint8_t *)sc->coll,
- sc->coll_size);
- if (err < 0)
- return err;
+ if (!av_packet_side_data_set_add(&st->codecpar->side_data, AV_PKT_DATA_CONTENT_LIGHT_LEVEL,
+ (uint8_t *)sc->coll, sc->coll_size, 0))
+ return AVERROR(ENOMEM);
sc->coll = NULL;
}
diff --git a/libavformat/movenc.c b/libavformat/movenc.c
index 696ae5a6c9..66efed0363 100644
--- a/libavformat/movenc.c
+++ b/libavformat/movenc.c
@@ -672,11 +672,11 @@ struct mpeg4_bit_rate_values {
static struct mpeg4_bit_rate_values calculate_mpeg4_bit_rates(MOVTrack *track)
{
- AVCPBProperties *props = track->st ?
- (AVCPBProperties*)av_stream_get_side_data(track->st,
- AV_PKT_DATA_CPB_PROPERTIES,
- NULL) :
+ AVPacketSideData *sd = track->st ?
+ av_packet_side_data_set_get(&track->st->codecpar->side_data,
+ AV_PKT_DATA_CPB_PROPERTIES) :
NULL;
+ AVCPBProperties *props = sd ? (AVCPBProperties *)sd->data : NULL;
struct mpeg4_bit_rate_values bit_rates = { 0 };
bit_rates.avg_bit_rate = compute_avg_bitrate(track);
@@ -2129,18 +2129,16 @@ static int mov_write_colr_tag(AVIOContext *pb, MOVTrack *track, int prefer_icc)
// Ref (MOV): https://developer.apple.com/library/mac/technotes/tn2162/_index.html#//apple_ref/doc/uid/DTS40013070-CH1-TNTAG9
// Ref (MP4): ISO/IEC 14496-12:2012
- const uint8_t *icc_profile;
- size_t icc_profile_size;
-
if (prefer_icc) {
- icc_profile = av_stream_get_side_data(track->st, AV_PKT_DATA_ICC_PROFILE, &icc_profile_size);
+ AVPacketSideData *sd = av_packet_side_data_set_get(&track->st->codecpar->side_data,
+ AV_PKT_DATA_ICC_PROFILE);
- if (icc_profile) {
- avio_wb32(pb, 12 + icc_profile_size);
+ if (sd) {
+ avio_wb32(pb, 12 + sd->size);
ffio_wfourcc(pb, "colr");
ffio_wfourcc(pb, "prof");
- avio_write(pb, icc_profile, icc_profile_size);
- return 12 + icc_profile_size;
+ avio_write(pb, sd->data, sd->size);
+ return 12 + sd->size;
}
else {
av_log(NULL, AV_LOG_INFO, "no ICC profile found, will write nclx/nclc colour info instead\n");
@@ -2173,14 +2171,14 @@ static int mov_write_colr_tag(AVIOContext *pb, MOVTrack *track, int prefer_icc)
static int mov_write_clli_tag(AVIOContext *pb, MOVTrack *track)
{
- const uint8_t *side_data;
+ const AVPacketSideData *side_data;
const AVContentLightMetadata *content_light_metadata;
- side_data = av_stream_get_side_data(track->st, AV_PKT_DATA_CONTENT_LIGHT_LEVEL, NULL);
+ side_data = av_packet_side_data_set_get(&track->st->codecpar->side_data, AV_PKT_DATA_CONTENT_LIGHT_LEVEL);
if (!side_data) {
return 0;
}
- content_light_metadata = (const AVContentLightMetadata*)side_data;
+ content_light_metadata = (const AVContentLightMetadata*)side_data->data;
avio_wb32(pb, 12); // size
ffio_wfourcc(pb, "clli");
@@ -2198,11 +2196,12 @@ static int mov_write_mdcv_tag(AVIOContext *pb, MOVTrack *track)
{
const int chroma_den = 50000;
const int luma_den = 10000;
- const uint8_t *side_data;
- const AVMasteringDisplayMetadata *metadata;
+ const AVPacketSideData *side_data;
+ const AVMasteringDisplayMetadata *metadata = NULL;
- side_data = av_stream_get_side_data(track->st, AV_PKT_DATA_MASTERING_DISPLAY_METADATA, NULL);
- metadata = (const AVMasteringDisplayMetadata*)side_data;
+ side_data = av_packet_side_data_set_get(&track->st->codecpar->side_data, AV_PKT_DATA_MASTERING_DISPLAY_METADATA);
+ if (side_data)
+ metadata = (const AVMasteringDisplayMetadata*)side_data->data;
if (!metadata || !metadata->has_primaries || !metadata->has_luminance) {
return 0;
}
@@ -2421,7 +2420,7 @@ static int mov_write_video_tag(AVFormatContext *s, AVIOContext *pb, MOVMuxContex
track->par->color_trc != AVCOL_TRC_UNSPECIFIED &&
track->par->color_space != AVCOL_SPC_UNSPECIFIED;
if (has_color_info || mov->flags & FF_MOV_FLAG_WRITE_COLR ||
- av_stream_get_side_data(track->st, AV_PKT_DATA_ICC_PROFILE, NULL)) {
+ av_packet_side_data_set_get(&track->st->codecpar->side_data, AV_PKT_DATA_ICC_PROFILE)) {
int prefer_icc = mov->flags & FF_MOV_FLAG_PREFER_ICC || !has_color_info;
mov_write_colr_tag(pb, track, prefer_icc);
}
@@ -2435,17 +2434,16 @@ static int mov_write_video_tag(AVFormatContext *s, AVIOContext *pb, MOVMuxContex
}
if (track->mode == MODE_MP4 && mov->fc->strict_std_compliance <= FF_COMPLIANCE_UNOFFICIAL) {
- AVStereo3D* stereo_3d = (AVStereo3D*) av_stream_get_side_data(track->st, AV_PKT_DATA_STEREO3D, NULL);
- AVSphericalMapping* spherical_mapping = (AVSphericalMapping*)av_stream_get_side_data(track->st, AV_PKT_DATA_SPHERICAL, NULL);
- AVDOVIDecoderConfigurationRecord *dovi = (AVDOVIDecoderConfigurationRecord *)
- av_stream_get_side_data(track->st, AV_PKT_DATA_DOVI_CONF, NULL);
+ AVPacketSideData *stereo_3d = av_packet_side_data_set_get(&track->st->codecpar->side_data, AV_PKT_DATA_STEREO3D);
+ AVPacketSideData *spherical_mapping = av_packet_side_data_set_get(&track->st->codecpar->side_data, AV_PKT_DATA_SPHERICAL);
+ AVPacketSideData *dovi = av_packet_side_data_set_get(&track->st->codecpar->side_data, AV_PKT_DATA_DOVI_CONF);
if (stereo_3d)
- mov_write_st3d_tag(s, pb, stereo_3d);
+ mov_write_st3d_tag(s, pb, (AVStereo3D*)stereo_3d->data);
if (spherical_mapping)
- mov_write_sv3d_tag(mov->fc, pb, spherical_mapping);
+ mov_write_sv3d_tag(mov->fc, pb, (AVSphericalMapping*)spherical_mapping->data);
if (dovi)
- mov_write_dvcc_dvvc_tag(s, pb, dovi);
+ mov_write_dvcc_dvvc_tag(s, pb, (AVDOVIDecoderConfigurationRecord *)dovi->data);
}
if (track->par->sample_aspect_ratio.den && track->par->sample_aspect_ratio.num) {
@@ -3392,7 +3390,6 @@ static int mov_write_tkhd_tag(AVIOContext *pb, MOVMuxContext *mov,
int group = 0;
uint32_t *display_matrix = NULL;
- size_t display_matrix_size;
int i;
if (mov->mode == MODE_AVIF)
@@ -3402,15 +3399,15 @@ static int mov_write_tkhd_tag(AVIOContext *pb, MOVMuxContext *mov,
duration *= mov->avif_loop_count;
if (st) {
+ AVPacketSideData *sd;
if (mov->per_stream_grouping)
group = st->index;
else
group = st->codecpar->codec_type;
- display_matrix = (uint32_t*)av_stream_get_side_data(st, AV_PKT_DATA_DISPLAYMATRIX,
- &display_matrix_size);
- if (display_matrix && display_matrix_size < 9 * sizeof(*display_matrix))
- display_matrix = NULL;
+ sd = av_packet_side_data_set_get(&st->codecpar->side_data, AV_PKT_DATA_DISPLAYMATRIX);
+ if (sd && sd->size == 9 * sizeof(*display_matrix))
+ display_matrix = (uint32_t *)sd->data;
}
if (track->flags & MOV_TRACK_ENABLED)
@@ -4608,12 +4605,10 @@ static int mov_write_moov_tag(AVIOContext *pb, MOVMuxContext *mov,
track->tref_tag = MKTAG('h','i','n','t');
track->tref_id = mov->tracks[track->src_track].track_id;
} else if (track->par->codec_type == AVMEDIA_TYPE_AUDIO) {
- size_t size;
- int *fallback;
- fallback = (int*)av_stream_get_side_data(track->st,
- AV_PKT_DATA_FALLBACK_TRACK,
- &size);
- if (fallback != NULL && size == sizeof(int)) {
+ AVPacketSideData *sd = av_packet_side_data_set_get(&track->st->codecpar->side_data,
+ AV_PKT_DATA_FALLBACK_TRACK );
+ if (sd && sd->size == sizeof(int)) {
+ int *fallback = (int *)sd->data;
if (*fallback >= 0 && *fallback < mov->nb_streams) {
track->tref_tag = MKTAG('f','a','l','l');
track->tref_id = mov->tracks[*fallback].track_id;
@@ -5446,7 +5441,7 @@ static int mov_write_ftyp_tag(AVIOContext *pb, AVFormatContext *s)
if (st->codecpar->codec_id == AV_CODEC_ID_AC3 ||
st->codecpar->codec_id == AV_CODEC_ID_EAC3 ||
st->codecpar->codec_id == AV_CODEC_ID_TRUEHD ||
- av_stream_get_side_data(st, AV_PKT_DATA_DOVI_CONF, NULL))
+ av_packet_side_data_set_get(&st->codecpar->side_data, AV_PKT_DATA_DOVI_CONF))
has_dolby = 1;
}
diff --git a/libavformat/mp3enc.c b/libavformat/mp3enc.c
index 5e81f72a59..9fc1c4fea4 100644
--- a/libavformat/mp3enc.c
+++ b/libavformat/mp3enc.c
@@ -400,10 +400,10 @@ static int mp3_queue_flush(AVFormatContext *s)
static void mp3_update_xing(AVFormatContext *s)
{
MP3Context *mp3 = s->priv_data;
+ AVPacketSideData *sd;
AVReplayGain *rg;
uint16_t tag_crc;
uint8_t *toc;
- size_t rg_size;
int i;
int64_t old_pos = avio_tell(s->pb);
@@ -423,11 +423,11 @@ static void mp3_update_xing(AVFormatContext *s)
}
/* write replaygain */
- rg = (AVReplayGain*)av_stream_get_side_data(s->streams[0], AV_PKT_DATA_REPLAYGAIN,
- &rg_size);
- if (rg && rg_size >= sizeof(*rg)) {
+ sd = av_packet_side_data_set_get(&s->streams[0]->codecpar->side_data, AV_PKT_DATA_REPLAYGAIN);
+ if (sd && sd->size >= sizeof(*rg)) {
uint16_t val;
+ rg = (AVReplayGain *)sd->data;
AV_WB32(mp3->xing_frame + mp3->xing_offset + 131,
av_rescale(rg->track_peak, 1 << 23, 100000));
diff --git a/libavformat/mpegenc.c b/libavformat/mpegenc.c
index c06e308296..3f3b94112d 100644
--- a/libavformat/mpegenc.c
+++ b/libavformat/mpegenc.c
@@ -342,8 +342,6 @@ static av_cold int mpeg_mux_init(AVFormatContext *ctx)
lpcm_id = LPCM_ID;
for (i = 0; i < ctx->nb_streams; i++) {
- AVCPBProperties *props;
-
st = ctx->streams[i];
stream = av_mallocz(sizeof(StreamInfo));
if (!stream)
@@ -430,13 +428,17 @@ static av_cold int mpeg_mux_init(AVFormatContext *ctx)
stream->max_buffer_size = 4 * 1024;
s->audio_bound++;
break;
- case AVMEDIA_TYPE_VIDEO:
+ case AVMEDIA_TYPE_VIDEO: {
+ AVPacketSideData *sd;
+ AVCPBProperties *props = NULL;
if (st->codecpar->codec_id == AV_CODEC_ID_H264)
stream->id = h264_id++;
else
stream->id = mpv_id++;
- props = (AVCPBProperties*)av_stream_get_side_data(st, AV_PKT_DATA_CPB_PROPERTIES, NULL);
+ sd = av_packet_side_data_set_get(&st->codecpar->side_data, AV_PKT_DATA_CPB_PROPERTIES);
+ if (sd)
+ props = (AVCPBProperties*)sd->data;
if (props && props->buffer_size)
stream->max_buffer_size = 6 * 1024 + props->buffer_size / 8;
else {
@@ -453,6 +455,7 @@ static av_cold int mpeg_mux_init(AVFormatContext *ctx)
}
s->video_bound++;
break;
+ }
case AVMEDIA_TYPE_SUBTITLE:
stream->id = mps_id++;
stream->max_buffer_size = 16 * 1024;
@@ -470,12 +473,15 @@ static av_cold int mpeg_mux_init(AVFormatContext *ctx)
audio_bitrate = 0;
video_bitrate = 0;
for (i = 0; i < ctx->nb_streams; i++) {
- AVCPBProperties *props;
+ AVPacketSideData *sd;
+ AVCPBProperties *props = NULL;
int codec_rate;
st = ctx->streams[i];
stream = (StreamInfo *)st->priv_data;
- props = (AVCPBProperties*)av_stream_get_side_data(st, AV_PKT_DATA_CPB_PROPERTIES, NULL);
+ sd = av_packet_side_data_set_get(&st->codecpar->side_data, AV_PKT_DATA_CPB_PROPERTIES);
+ if (sd)
+ props = (AVCPBProperties*)sd->data;
if (props)
codec_rate = props->max_bitrate;
else
diff --git a/libavformat/mpegts.c b/libavformat/mpegts.c
index 0b3edda817..3ba723d060 100644
--- a/libavformat/mpegts.c
+++ b/libavformat/mpegts.c
@@ -2190,7 +2190,6 @@ int ff_parse_mpeg2_descriptor(AVFormatContext *fc, AVStream *st, int stream_type
uint32_t buf;
AVDOVIDecoderConfigurationRecord *dovi;
size_t dovi_size;
- int ret;
int dependency_pid;
if (desc_end - *pp < 4) // (8 + 8 + 7 + 6 + 1 + 1 + 1) / 8
@@ -2221,11 +2220,10 @@ int ff_parse_mpeg2_descriptor(AVFormatContext *fc, AVStream *st, int stream_type
dovi->dv_bl_signal_compatibility_id = 0;
}
- ret = av_stream_add_side_data(st, AV_PKT_DATA_DOVI_CONF,
- (uint8_t *)dovi, dovi_size);
- if (ret < 0) {
+ if (!av_packet_side_data_set_add(&st->codecpar->side_data, AV_PKT_DATA_DOVI_CONF,
+ (uint8_t *)dovi, dovi_size, 0)) {
av_free(dovi);
- return ret;
+ return AVERROR(ENOMEM);
}
av_log(fc, AV_LOG_TRACE, "DOVI, version: %d.%d, profile: %d, level: %d, "
diff --git a/libavformat/mux.c b/libavformat/mux.c
index 415bd3948f..ae07c8839e 100644
--- a/libavformat/mux.c
+++ b/libavformat/mux.c
@@ -277,6 +277,25 @@ FF_ENABLE_DEPRECATION_WARNINGS
break;
}
+#if FF_API_AVSTREAM_SIDE_DATA
+FF_DISABLE_DEPRECATION_WARNINGS
+ /* if the caller is using the deprecated AVStream side_data API,
+ * copy its contents to AVStream.codecpar, giving it priority
+ over existing side data in the latter */
+ for (int i = 0; i < st->nb_side_data; i++) {
+ const AVPacketSideData *sd_src = &st->side_data[i];
+ AVPacketSideData *sd_dst;
+
+ sd_dst = av_packet_side_data_set_new(&st->codecpar->side_data, sd_src->type, sd_src->size, 0);
+ if (!sd_dst) {
+ ret = AVERROR(ENOMEM);
+ goto fail;
+ }
+ memcpy(sd_dst->data, sd_src->data, sd_src->size);
+ }
+FF_ENABLE_DEPRECATION_WARNINGS
+#endif
+
desc = avcodec_descriptor_get(par->codec_id);
if (desc && desc->props & AV_CODEC_PROP_REORDER)
sti->reorder = 1;
diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c
index 34230ece98..2438b7c7bf 100644
--- a/libavformat/mxfdec.c
+++ b/libavformat/mxfdec.c
@@ -2577,10 +2577,12 @@ static int parse_mca_labels(MXFContext *mxf, MXFTrack *source_track, MXFDescript
if (service_type != AV_AUDIO_SERVICE_TYPE_NB && service_type != AV_AUDIO_SERVICE_TYPE_MAIN && !ambigous_service_type) {
enum AVAudioServiceType *ast;
- uint8_t* side_data = av_stream_new_side_data(st, AV_PKT_DATA_AUDIO_SERVICE_TYPE, sizeof(*ast));
+ AVPacketSideData *side_data = av_packet_side_data_set_new(&st->codecpar->side_data,
+ AV_PKT_DATA_AUDIO_SERVICE_TYPE,
+ sizeof(*ast), 0);
if (!side_data)
return AVERROR(ENOMEM);
- ast = (enum AVAudioServiceType*)side_data;
+ ast = (enum AVAudioServiceType*)side_data->data;
*ast = service_type;
}
@@ -2980,19 +2982,19 @@ static int mxf_parse_structural_metadata(MXFContext *mxf)
st->codecpar->color_trc = mxf_get_codec_ul(ff_mxf_color_trc_uls, &descriptor->color_trc_ul)->id;
st->codecpar->color_space = mxf_get_codec_ul(ff_mxf_color_space_uls, &descriptor->color_space_ul)->id;
if (descriptor->mastering) {
- ret = av_stream_add_side_data(st, AV_PKT_DATA_MASTERING_DISPLAY_METADATA,
- (uint8_t *)descriptor->mastering,
- sizeof(*descriptor->mastering));
- if (ret < 0)
+ if (!av_packet_side_data_set_add(&st->codecpar->side_data, AV_PKT_DATA_MASTERING_DISPLAY_METADATA,
+ (uint8_t *)descriptor->mastering, sizeof(*descriptor->mastering), 0)) {
+ ret = AVERROR(ENOMEM);
goto fail_and_free;
+ }
descriptor->mastering = NULL;
}
if (descriptor->coll) {
- ret = av_stream_add_side_data(st, AV_PKT_DATA_CONTENT_LIGHT_LEVEL,
- (uint8_t *)descriptor->coll,
- descriptor->coll_size);
- if (ret < 0)
+ if (!av_packet_side_data_set_add(&st->codecpar->side_data, AV_PKT_DATA_CONTENT_LIGHT_LEVEL,
+ (uint8_t *)descriptor->coll, descriptor->coll_size, 0)) {
+ ret = AVERROR(ENOMEM);
goto fail_and_free;
+ }
descriptor->coll = NULL;
}
} else if (st->codecpar->codec_type == AVMEDIA_TYPE_AUDIO) {
diff --git a/libavformat/mxfenc.c b/libavformat/mxfenc.c
index d8252ed68f..33922400e6 100644
--- a/libavformat/mxfenc.c
+++ b/libavformat/mxfenc.c
@@ -556,7 +556,7 @@ static void mxf_write_primer_pack(AVFormatContext *s)
if (s->streams[i]->codecpar->codec_id == AV_CODEC_ID_H264 && !sc->avc_intra) {
will_have_avc_tags = 1;
}
- if (av_stream_get_side_data(s->streams[i], AV_PKT_DATA_MASTERING_DISPLAY_METADATA, NULL)) {
+ if (av_packet_side_data_set_get(&s->streams[i]->codecpar->side_data, AV_PKT_DATA_MASTERING_DISPLAY_METADATA)) {
will_have_mastering_tags = 1;
}
if (s->streams[i]->codecpar->codec_id == AV_CODEC_ID_FFV1) {
@@ -1158,7 +1158,7 @@ static int64_t mxf_write_cdci_common(AVFormatContext *s, AVStream *st, const UID
const MXFCodecUL *color_trc_ul;
const MXFCodecUL *color_space_ul;
int64_t pos = mxf_write_generic_desc(s, st, key);
- uint8_t *side_data;
+ AVPacketSideData *side_data;
color_primaries_ul = mxf_get_codec_ul_by_id(ff_mxf_color_primaries_uls, st->codecpar->color_primaries);
color_trc_ul = mxf_get_codec_ul_by_id(ff_mxf_color_trc_uls, st->codecpar->color_trc);
@@ -1344,9 +1344,9 @@ static int64_t mxf_write_cdci_common(AVFormatContext *s, AVStream *st, const UID
avio_write(pb, *sc->codec_ul, 16);
// Mastering Display metadata
- side_data = av_stream_get_side_data(st, AV_PKT_DATA_MASTERING_DISPLAY_METADATA, NULL);
+ side_data = av_packet_side_data_set_get(&st->codecpar->side_data, AV_PKT_DATA_MASTERING_DISPLAY_METADATA);
if (side_data) {
- const AVMasteringDisplayMetadata *metadata = (const AVMasteringDisplayMetadata*)side_data;
+ const AVMasteringDisplayMetadata *metadata = (const AVMasteringDisplayMetadata*)side_data->data;
if (metadata->has_primaries) {
mxf_write_local_tag(s, 12, 0x8301);
avio_wb16(pb, rescale_mastering_chroma(metadata->display_primaries[0][0]));
diff --git a/libavformat/options.c b/libavformat/options.c
index e4a3aceed0..ef0b593d36 100644
--- a/libavformat/options.c
+++ b/libavformat/options.c
@@ -309,7 +309,9 @@ AVStream *avformat_new_stream(AVFormatContext *s, const AVCodec *c)
st->sample_aspect_ratio = (AVRational) { 0, 1 };
+#if FF_API_AVSTREAM_SIDE_DATA
sti->inject_global_side_data = si->inject_global_side_data;
+#endif
sti->need_context_update = 1;
diff --git a/libavformat/replaygain.c b/libavformat/replaygain.c
index 915bcb2382..ab3e39318b 100644
--- a/libavformat/replaygain.c
+++ b/libavformat/replaygain.c
@@ -69,16 +69,19 @@ static int32_t parse_value(const char *value, int32_t min)
int ff_replaygain_export_raw(AVStream *st, int32_t tg, uint32_t tp,
int32_t ag, uint32_t ap)
{
+ AVPacketSideData *sd;
AVReplayGain *replaygain;
if (tg == INT32_MIN && ag == INT32_MIN)
return 0;
- replaygain = (AVReplayGain*)av_stream_new_side_data(st, AV_PKT_DATA_REPLAYGAIN,
- sizeof(*replaygain));
- if (!replaygain)
+ sd = av_packet_side_data_set_new(&st->codecpar->side_data,
+ AV_PKT_DATA_REPLAYGAIN,
+ sizeof(*replaygain), 0);
+ if (!sd)
return AVERROR(ENOMEM);
+ replaygain = (AVReplayGain*)sd->data;
replaygain->track_gain = tg;
replaygain->track_peak = tp;
replaygain->album_gain = ag;
diff --git a/libavformat/seek.c b/libavformat/seek.c
index 386312cd3a..0180188595 100644
--- a/libavformat/seek.c
+++ b/libavformat/seek.c
@@ -745,8 +745,10 @@ void ff_read_frame_flush(AVFormatContext *s)
for (int j = 0; j < MAX_REORDER_DELAY + 1; j++)
sti->pts_buffer[j] = AV_NOPTS_VALUE;
+#if FF_API_AVSTREAM_SIDE_DATA
if (si->inject_global_side_data)
sti->inject_global_side_data = 1;
+#endif
sti->skip_samples = 0;
}
diff --git a/libavformat/version_major.h b/libavformat/version_major.h
index 293fbd3397..c348e3eb37 100644
--- a/libavformat/version_major.h
+++ b/libavformat/version_major.h
@@ -45,6 +45,7 @@
#define FF_API_GET_END_PTS (LIBAVFORMAT_VERSION_MAJOR < 61)
#define FF_API_AVIODIRCONTEXT (LIBAVFORMAT_VERSION_MAJOR < 61)
#define FF_API_AVFORMAT_IO_CLOSE (LIBAVFORMAT_VERSION_MAJOR < 61)
+#define FF_API_AVSTREAM_SIDE_DATA (LIBAVFORMAT_VERSION_MAJOR < 61)
#define FF_API_R_FRAME_RATE 1
--
2.42.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] 28+ messages in thread* Re: [FFmpeg-devel] [PATCH 03/10] avformat/avformat: use the side data from AVStream.codecpar
2023-09-06 17:44 ` [FFmpeg-devel] [PATCH 03/10] avformat/avformat: use the side data from AVStream.codecpar James Almer
@ 2023-09-11 19:19 ` Andreas Rheinhardt
2023-09-12 16:27 ` James Almer
0 siblings, 1 reply; 28+ messages in thread
From: Andreas Rheinhardt @ 2023-09-11 19:19 UTC (permalink / raw)
To: ffmpeg-devel
James Almer:
> Deprecate AVStream.side_data and its helpers in favor of the AVStream's
> codecpar.side_data.
>
> This will considerably simplify the propagation of global side data to decoders
> and from encoders. Instead of having to do it inside packets, it will be
> available during init().
> Global and frame specific side data will therefore be distinct.
>
> Signed-off-by: James Almer <jamrial@gmail.com>
> ---> diff --git a/libavformat/avformat.h b/libavformat/avformat.h
> index 1916aa2dc5..f78a027f64 100644
> --- a/libavformat/avformat.h
> +++ b/libavformat/avformat.h
> @@ -164,6 +164,13 @@
> * decoding functions avcodec_send_packet() or avcodec_decode_subtitle2() if the
> * caller wishes to decode the data.
> *
> + * There may be no overlap between the stream's @ref AVCodecParameters.side_data
> + * "side data" and @ref AVPacket.side_data "side data" in packets. I.e. a given
> + * side data is either exported by the demuxer in AVCodecParameters, then it never
> + * appears in the packets, or the side data is exported through the packets (always
> + * in the first packet where the value becomes known or changes), then it does not
> + * appear in AVCodecParameters.
> + *
Is it actually certain that our demuxers currently abide by this? E.g.
in mpegts, stream parameters can change at any time, so why does it set
it at the stream level and not the packet level?
> * AVPacket.pts, AVPacket.dts and AVPacket.duration timing information will be
> * set if known. They may also be unset (i.e. AV_NOPTS_VALUE for
> * pts/dts, 0 for duration) if the stream does not provide them. The timing
> @@ -209,6 +216,11 @@
> * AVCodecParameters, rather than using @ref avcodec_parameters_copy() during
> * remuxing: there is no guarantee that the codec context values remain valid
> * for both input and output format contexts.
> + * - There may be no overlap between AVCodecParameters.side_data and side data in
> + * packets. I.e. a given side data is either set by the caller in
> + * AVCodecParameters, then it never appears in the packets, or the side data is
> + * sent through the packets (always in the first packet where the value becomes
> + * known or changes), then it does not appear in AVCodecParameters.
I have to say, I don't really like this (and of course I am aware that
you are basically copying the doxy of AVPacketSideData here). As you
know, the Matroska muxer needs to add header fields in order to add
certain packet side data to blocks later. In case of seekable output,
one can update the header later, but in case of unseekable output that
is not true. I'd like there to be an easy way for the user to signal the
intention to send packet side data of a specific type later.
> * - The caller may fill in additional information, such as @ref
> * AVFormatContext.metadata "global" or @ref AVStream.metadata "per-stream"
> * metadata, @ref AVFormatContext.chapters "chapters", @ref
> @@ -937,6 +949,7 @@ typedef struct AVStream {
> */
> AVPacket attached_pic;
>
> +#if FF_API_AVSTREAM_SIDE_DATA
> /**
> * An array of side data that applies to the whole stream (i.e. the
> * container does not allow it to change between packets).
> @@ -953,13 +966,20 @@ typedef struct AVStream {
> *
> * Freed by libavformat in avformat_free_context().
> *
> - * @see av_format_inject_global_side_data()
> + * @deprecated use AVStream's @ref AVCodecParameters.side_data
> + * "codecpar side data".
> */
> + attribute_deprecated
> AVPacketSideData *side_data;
> /**
> * The number of elements in the AVStream.side_data array.
> + *
> + * @deprecated use AVStream's @ref AVCodecParameters.side_data
> + * "codecpar side data".
> */
> + attribute_deprecated
> int nb_side_data;
> +#endif
>
> /**
> * Flags indicating events happening on the stream, a combination of
> @@ -1715,11 +1735,18 @@ typedef struct AVFormatContext {
> int (*io_close2)(struct AVFormatContext *s, AVIOContext *pb);
> } AVFormatContext;
>
> +#if FF_API_AVSTREAM_SIDE_DATA
> /**
> * This function will cause global side data to be injected in the next packet
> * of each stream as well as after any subsequent seek.
> + *
> + * @deprecated global side data is always available in every AVStream's
> + * @ref AVCodecParameters.side_data "codecpar side data" array.
> + * @see av_packet_side_data_set_get()
> */
> +attribute_deprecated
> void av_format_inject_global_side_data(AVFormatContext *s);
> +#endif
>
> /**
> * Returns the method used to set ctx->duration.
> @@ -1844,6 +1871,7 @@ const AVClass *av_stream_get_class(void);
> */
> AVStream *avformat_new_stream(AVFormatContext *s, const AVCodec *c);
>
> +#if FF_API_AVSTREAM_SIDE_DATA
> /**
> * Wrap an existing array as stream side data.
> *
> @@ -1856,7 +1884,10 @@ AVStream *avformat_new_stream(AVFormatContext *s, const AVCodec *c);
> *
> * @return zero on success, a negative AVERROR code on failure. On failure,
> * the stream is unchanged and the data remains owned by the caller.
> + * @deprecated use av_packet_side_data_set_add() with the stream's
> + * @ref AVCodecParameters.side_data "codecpar side data"
> */
> +attribute_deprecated
> int av_stream_add_side_data(AVStream *st, enum AVPacketSideDataType type,
> uint8_t *data, size_t size);
>
> @@ -1868,7 +1899,10 @@ int av_stream_add_side_data(AVStream *st, enum AVPacketSideDataType type,
> * @param size side information size
> *
> * @return pointer to fresh allocated data or NULL otherwise
> + * @deprecated use av_packet_side_data_set_new() with the stream's
> + * @ref AVCodecParameters.side_data "codecpar side data"
> */
> +attribute_deprecated
> uint8_t *av_stream_new_side_data(AVStream *stream,
> enum AVPacketSideDataType type, size_t size);
> /**
> @@ -1880,9 +1914,13 @@ uint8_t *av_stream_new_side_data(AVStream *stream,
> * or to zero if the desired side data is not present.
> *
> * @return pointer to data if present or NULL otherwise
> + * @deprecated use av_packet_side_data_set_get() with the stream's
> + * @ref AVCodecParameters.side_data "codecpar side data"
> */
> +attribute_deprecated
> uint8_t *av_stream_get_side_data(const AVStream *stream,
> enum AVPacketSideDataType type, size_t *size);
> +#endif
>
> AVProgram *av_new_program(AVFormatContext *s, int id);
>
_______________________________________________
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] 28+ messages in thread* Re: [FFmpeg-devel] [PATCH 03/10] avformat/avformat: use the side data from AVStream.codecpar
2023-09-11 19:19 ` Andreas Rheinhardt
@ 2023-09-12 16:27 ` James Almer
2023-09-12 16:43 ` Andreas Rheinhardt
0 siblings, 1 reply; 28+ messages in thread
From: James Almer @ 2023-09-12 16:27 UTC (permalink / raw)
To: ffmpeg-devel
On 9/11/2023 4:19 PM, Andreas Rheinhardt wrote:
> James Almer:
>> Deprecate AVStream.side_data and its helpers in favor of the AVStream's
>> codecpar.side_data.
>>
>> This will considerably simplify the propagation of global side data to decoders
>> and from encoders. Instead of having to do it inside packets, it will be
>> available during init().
>> Global and frame specific side data will therefore be distinct.
>>
>> Signed-off-by: James Almer <jamrial@gmail.com>
>> ---> diff --git a/libavformat/avformat.h b/libavformat/avformat.h
>> index 1916aa2dc5..f78a027f64 100644
>> --- a/libavformat/avformat.h
>> +++ b/libavformat/avformat.h
>> @@ -164,6 +164,13 @@
>> * decoding functions avcodec_send_packet() or avcodec_decode_subtitle2() if the
>> * caller wishes to decode the data.
>> *
>> + * There may be no overlap between the stream's @ref AVCodecParameters.side_data
>> + * "side data" and @ref AVPacket.side_data "side data" in packets. I.e. a given
>> + * side data is either exported by the demuxer in AVCodecParameters, then it never
>> + * appears in the packets, or the side data is exported through the packets (always
>> + * in the first packet where the value becomes known or changes), then it does not
>> + * appear in AVCodecParameters.
>> + *
>
> Is it actually certain that our demuxers currently abide by this? E.g.
> in mpegts, stream parameters can change at any time, so why does it set
> it at the stream level and not the packet level?
Does mpegts change AVStream.codecpar mid demuxing? wouldn't that be an
API violation? I assumed that was what AV_PKT_DATA_PARAM_CHANGE and
AV_PKT_DATA_NEW_EXTRADATA packet side data were for.
>
>> * AVPacket.pts, AVPacket.dts and AVPacket.duration timing information will be
>> * set if known. They may also be unset (i.e. AV_NOPTS_VALUE for
>> * pts/dts, 0 for duration) if the stream does not provide them. The timing
>> @@ -209,6 +216,11 @@
>> * AVCodecParameters, rather than using @ref avcodec_parameters_copy() during
>> * remuxing: there is no guarantee that the codec context values remain valid
>> * for both input and output format contexts.
>> + * - There may be no overlap between AVCodecParameters.side_data and side data in
>> + * packets. I.e. a given side data is either set by the caller in
>> + * AVCodecParameters, then it never appears in the packets, or the side data is
>> + * sent through the packets (always in the first packet where the value becomes
>> + * known or changes), then it does not appear in AVCodecParameters.
>
> I have to say, I don't really like this (and of course I am aware that
> you are basically copying the doxy of AVPacketSideData here). As you
I can remove this part, to be added later if needed.
> know, the Matroska muxer needs to add header fields in order to add
> certain packet side data to blocks later. In case of seekable output,
> one can update the header later, but in case of unseekable output that
> is not true. I'd like there to be an easy way for the user to signal the
> intention to send packet side data of a specific type later.
Maybe a new AVFMT_FLAG_?
>
>> * - The caller may fill in additional information, such as @ref
>> * AVFormatContext.metadata "global" or @ref AVStream.metadata "per-stream"
>> * metadata, @ref AVFormatContext.chapters "chapters", @ref
>> @@ -937,6 +949,7 @@ typedef struct AVStream {
>> */
>> AVPacket attached_pic;
>>
>> +#if FF_API_AVSTREAM_SIDE_DATA
>> /**
>> * An array of side data that applies to the whole stream (i.e. the
>> * container does not allow it to change between packets).
>> @@ -953,13 +966,20 @@ typedef struct AVStream {
>> *
>> * Freed by libavformat in avformat_free_context().
>> *
>> - * @see av_format_inject_global_side_data()
>> + * @deprecated use AVStream's @ref AVCodecParameters.side_data
>> + * "codecpar side data".
>> */
>> + attribute_deprecated
>> AVPacketSideData *side_data;
>> /**
>> * The number of elements in the AVStream.side_data array.
>> + *
>> + * @deprecated use AVStream's @ref AVCodecParameters.side_data
>> + * "codecpar side data".
>> */
>> + attribute_deprecated
>> int nb_side_data;
>> +#endif
>>
>> /**
>> * Flags indicating events happening on the stream, a combination of
>> @@ -1715,11 +1735,18 @@ typedef struct AVFormatContext {
>> int (*io_close2)(struct AVFormatContext *s, AVIOContext *pb);
>> } AVFormatContext;
>>
>> +#if FF_API_AVSTREAM_SIDE_DATA
>> /**
>> * This function will cause global side data to be injected in the next packet
>> * of each stream as well as after any subsequent seek.
>> + *
>> + * @deprecated global side data is always available in every AVStream's
>> + * @ref AVCodecParameters.side_data "codecpar side data" array.
>> + * @see av_packet_side_data_set_get()
>> */
>> +attribute_deprecated
>> void av_format_inject_global_side_data(AVFormatContext *s);
>> +#endif
>>
>> /**
>> * Returns the method used to set ctx->duration.
>> @@ -1844,6 +1871,7 @@ const AVClass *av_stream_get_class(void);
>> */
>> AVStream *avformat_new_stream(AVFormatContext *s, const AVCodec *c);
>>
>> +#if FF_API_AVSTREAM_SIDE_DATA
>> /**
>> * Wrap an existing array as stream side data.
>> *
>> @@ -1856,7 +1884,10 @@ AVStream *avformat_new_stream(AVFormatContext *s, const AVCodec *c);
>> *
>> * @return zero on success, a negative AVERROR code on failure. On failure,
>> * the stream is unchanged and the data remains owned by the caller.
>> + * @deprecated use av_packet_side_data_set_add() with the stream's
>> + * @ref AVCodecParameters.side_data "codecpar side data"
>> */
>> +attribute_deprecated
>> int av_stream_add_side_data(AVStream *st, enum AVPacketSideDataType type,
>> uint8_t *data, size_t size);
>>
>> @@ -1868,7 +1899,10 @@ int av_stream_add_side_data(AVStream *st, enum AVPacketSideDataType type,
>> * @param size side information size
>> *
>> * @return pointer to fresh allocated data or NULL otherwise
>> + * @deprecated use av_packet_side_data_set_new() with the stream's
>> + * @ref AVCodecParameters.side_data "codecpar side data"
>> */
>> +attribute_deprecated
>> uint8_t *av_stream_new_side_data(AVStream *stream,
>> enum AVPacketSideDataType type, size_t size);
>> /**
>> @@ -1880,9 +1914,13 @@ uint8_t *av_stream_new_side_data(AVStream *stream,
>> * or to zero if the desired side data is not present.
>> *
>> * @return pointer to data if present or NULL otherwise
>> + * @deprecated use av_packet_side_data_set_get() with the stream's
>> + * @ref AVCodecParameters.side_data "codecpar side data"
>> */
>> +attribute_deprecated
>> uint8_t *av_stream_get_side_data(const AVStream *stream,
>> enum AVPacketSideDataType type, size_t *size);
>> +#endif
>>
>> AVProgram *av_new_program(AVFormatContext *s, int id);
>>
>
>
> _______________________________________________
> 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".
_______________________________________________
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] 28+ messages in thread* Re: [FFmpeg-devel] [PATCH 03/10] avformat/avformat: use the side data from AVStream.codecpar
2023-09-12 16:27 ` James Almer
@ 2023-09-12 16:43 ` Andreas Rheinhardt
2023-09-12 16:57 ` James Almer
0 siblings, 1 reply; 28+ messages in thread
From: Andreas Rheinhardt @ 2023-09-12 16:43 UTC (permalink / raw)
To: ffmpeg-devel
James Almer:
> On 9/11/2023 4:19 PM, Andreas Rheinhardt wrote:
>> James Almer:
>>> Deprecate AVStream.side_data and its helpers in favor of the AVStream's
>>> codecpar.side_data.
>>>
>>> This will considerably simplify the propagation of global side data
>>> to decoders
>>> and from encoders. Instead of having to do it inside packets, it will be
>>> available during init().
>>> Global and frame specific side data will therefore be distinct.
>>>
>>> Signed-off-by: James Almer <jamrial@gmail.com>
>>> ---> diff --git a/libavformat/avformat.h b/libavformat/avformat.h
>>> index 1916aa2dc5..f78a027f64 100644
>>> --- a/libavformat/avformat.h
>>> +++ b/libavformat/avformat.h
>>> @@ -164,6 +164,13 @@
>>> * decoding functions avcodec_send_packet() or
>>> avcodec_decode_subtitle2() if the
>>> * caller wishes to decode the data.
>>> *
>>> + * There may be no overlap between the stream's @ref
>>> AVCodecParameters.side_data
>>> + * "side data" and @ref AVPacket.side_data "side data" in packets.
>>> I.e. a given
>>> + * side data is either exported by the demuxer in AVCodecParameters,
>>> then it never
>>> + * appears in the packets, or the side data is exported through the
>>> packets (always
>>> + * in the first packet where the value becomes known or changes),
>>> then it does not
>>> + * appear in AVCodecParameters.
>>> + *
>>
>> Is it actually certain that our demuxers currently abide by this? E.g.
>> in mpegts, stream parameters can change at any time, so why does it set
>> it at the stream level and not the packet level?
>
> Does mpegts change AVStream.codecpar mid demuxing? wouldn't that be an
> API violation? I assumed that was what AV_PKT_DATA_PARAM_CHANGE and
> AV_PKT_DATA_NEW_EXTRADATA packet side data were for.
>
It seems that the dovi stream side data can be added (and potentially
even replaced) long after the stream has been created.
(The concat demuxer may even set extradata lateron and even change it
after it has been allocated; see match_streams() in
concat_read_packet(). The concat demuxer should actually add the global
side data of every input to the first packet from said input which means
that copying side data (whether in ff_stream_side_data_copy() or in
avcodec_parameters_copy() is potentially problematic.)
>>
>>> * AVPacket.pts, AVPacket.dts and AVPacket.duration timing
>>> information will be
>>> * set if known. They may also be unset (i.e. AV_NOPTS_VALUE for
>>> * pts/dts, 0 for duration) if the stream does not provide them.
>>> The timing
>>> @@ -209,6 +216,11 @@
>>> * AVCodecParameters, rather than using @ref
>>> avcodec_parameters_copy() during
>>> * remuxing: there is no guarantee that the codec context values
>>> remain valid
>>> * for both input and output format contexts.
>>> + * - There may be no overlap between AVCodecParameters.side_data and
>>> side data in
>>> + * packets. I.e. a given side data is either set by the caller in
>>> + * AVCodecParameters, then it never appears in the packets, or the
>>> side data is
>>> + * sent through the packets (always in the first packet where the
>>> value becomes
>>> + * known or changes), then it does not appear in AVCodecParameters.
>>
>> I have to say, I don't really like this (and of course I am aware that
>> you are basically copying the doxy of AVPacketSideData here). As you
>
> I can remove this part, to be added later if needed.
>
>> know, the Matroska muxer needs to add header fields in order to add
>> certain packet side data to blocks later. In case of seekable output,
>> one can update the header later, but in case of unseekable output that
>> is not true. I'd like there to be an easy way for the user to signal the
>> intention to send packet side data of a specific type later.
>
> Maybe a new AVFMT_FLAG_?
>
That would potentially be a new AVFMT_FLAG_ for every side data type;
that makes no sense.
>>
>>> * - The caller may fill in additional information, such as @ref
>>> * AVFormatContext.metadata "global" or @ref AVStream.metadata
>>> "per-stream"
>>> * metadata, @ref AVFormatContext.chapters "chapters", @ref
>>> @@ -937,6 +949,7 @@ typedef struct AVStream {
>>> */
>>> AVPacket attached_pic;
>>> +#if FF_API_AVSTREAM_SIDE_DATA
>>> /**
>>> * An array of side data that applies to the whole stream (i.e.
>>> the
>>> * container does not allow it to change between packets).
>>> @@ -953,13 +966,20 @@ typedef struct AVStream {
>>> *
>>> * Freed by libavformat in avformat_free_context().
>>> *
>>> - * @see av_format_inject_global_side_data()
>>> + * @deprecated use AVStream's @ref AVCodecParameters.side_data
>>> + * "codecpar side data".
>>> */
>>> + attribute_deprecated
>>> AVPacketSideData *side_data;
>>> /**
>>> * The number of elements in the AVStream.side_data array.
>>> + *
>>> + * @deprecated use AVStream's @ref AVCodecParameters.side_data
>>> + * "codecpar side data".
>>> */
>>> + attribute_deprecated
>>> int nb_side_data;
>>> +#endif
>>> /**
>>> * Flags indicating events happening on the stream, a
>>> combination of
>>> @@ -1715,11 +1735,18 @@ typedef struct AVFormatContext {
>>> int (*io_close2)(struct AVFormatContext *s, AVIOContext *pb);
>>> } AVFormatContext;
>>> +#if FF_API_AVSTREAM_SIDE_DATA
>>> /**
>>> * This function will cause global side data to be injected in the
>>> next packet
>>> * of each stream as well as after any subsequent seek.
>>> + *
>>> + * @deprecated global side data is always available in every AVStream's
>>> + * @ref AVCodecParameters.side_data "codecpar side data"
>>> array.
>>> + * @see av_packet_side_data_set_get()
>>> */
>>> +attribute_deprecated
>>> void av_format_inject_global_side_data(AVFormatContext *s);
>>> +#endif
>>> /**
>>> * Returns the method used to set ctx->duration.
>>> @@ -1844,6 +1871,7 @@ const AVClass *av_stream_get_class(void);
>>> */
>>> AVStream *avformat_new_stream(AVFormatContext *s, const AVCodec *c);
>>> +#if FF_API_AVSTREAM_SIDE_DATA
>>> /**
>>> * Wrap an existing array as stream side data.
>>> *
>>> @@ -1856,7 +1884,10 @@ AVStream *avformat_new_stream(AVFormatContext
>>> *s, const AVCodec *c);
>>> *
>>> * @return zero on success, a negative AVERROR code on failure. On
>>> failure,
>>> * the stream is unchanged and the data remains owned by
>>> the caller.
>>> + * @deprecated use av_packet_side_data_set_add() with the stream's
>>> + * @ref AVCodecParameters.side_data "codecpar side data"
>>> */
>>> +attribute_deprecated
>>> int av_stream_add_side_data(AVStream *st, enum AVPacketSideDataType
>>> type,
>>> uint8_t *data, size_t size);
>>> @@ -1868,7 +1899,10 @@ int av_stream_add_side_data(AVStream *st,
>>> enum AVPacketSideDataType type,
>>> * @param size side information size
>>> *
>>> * @return pointer to fresh allocated data or NULL otherwise
>>> + * @deprecated use av_packet_side_data_set_new() with the stream's
>>> + * @ref AVCodecParameters.side_data "codecpar side data"
>>> */
>>> +attribute_deprecated
>>> uint8_t *av_stream_new_side_data(AVStream *stream,
>>> enum AVPacketSideDataType type,
>>> size_t size);
>>> /**
>>> @@ -1880,9 +1914,13 @@ uint8_t *av_stream_new_side_data(AVStream
>>> *stream,
>>> * or to zero if the desired side data is not present.
>>> *
>>> * @return pointer to data if present or NULL otherwise
>>> + * @deprecated use av_packet_side_data_set_get() with the stream's
>>> + * @ref AVCodecParameters.side_data "codecpar side data"
>>> */
>>> +attribute_deprecated
>>> uint8_t *av_stream_get_side_data(const AVStream *stream,
>>> enum AVPacketSideDataType type,
>>> size_t *size);
>>> +#endif
>>> AVProgram *av_new_program(AVFormatContext *s, int id);
>>>
>>
_______________________________________________
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] 28+ messages in thread* Re: [FFmpeg-devel] [PATCH 03/10] avformat/avformat: use the side data from AVStream.codecpar
2023-09-12 16:43 ` Andreas Rheinhardt
@ 2023-09-12 16:57 ` James Almer
0 siblings, 0 replies; 28+ messages in thread
From: James Almer @ 2023-09-12 16:57 UTC (permalink / raw)
To: ffmpeg-devel
On 9/12/2023 1:43 PM, Andreas Rheinhardt wrote:
> James Almer:
>> On 9/11/2023 4:19 PM, Andreas Rheinhardt wrote:
>>> James Almer:
>>>> Deprecate AVStream.side_data and its helpers in favor of the AVStream's
>>>> codecpar.side_data.
>>>>
>>>> This will considerably simplify the propagation of global side data
>>>> to decoders
>>>> and from encoders. Instead of having to do it inside packets, it will be
>>>> available during init().
>>>> Global and frame specific side data will therefore be distinct.
>>>>
>>>> Signed-off-by: James Almer <jamrial@gmail.com>
>>>> ---> diff --git a/libavformat/avformat.h b/libavformat/avformat.h
>>>> index 1916aa2dc5..f78a027f64 100644
>>>> --- a/libavformat/avformat.h
>>>> +++ b/libavformat/avformat.h
>>>> @@ -164,6 +164,13 @@
>>>> * decoding functions avcodec_send_packet() or
>>>> avcodec_decode_subtitle2() if the
>>>> * caller wishes to decode the data.
>>>> *
>>>> + * There may be no overlap between the stream's @ref
>>>> AVCodecParameters.side_data
>>>> + * "side data" and @ref AVPacket.side_data "side data" in packets.
>>>> I.e. a given
>>>> + * side data is either exported by the demuxer in AVCodecParameters,
>>>> then it never
>>>> + * appears in the packets, or the side data is exported through the
>>>> packets (always
>>>> + * in the first packet where the value becomes known or changes),
>>>> then it does not
>>>> + * appear in AVCodecParameters.
>>>> + *
>>>
>>> Is it actually certain that our demuxers currently abide by this? E.g.
>>> in mpegts, stream parameters can change at any time, so why does it set
>>> it at the stream level and not the packet level?
>>
>> Does mpegts change AVStream.codecpar mid demuxing? wouldn't that be an
>> API violation? I assumed that was what AV_PKT_DATA_PARAM_CHANGE and
>> AV_PKT_DATA_NEW_EXTRADATA packet side data were for.
>>
>
> It seems that the dovi stream side data can be added (and potentially
> even replaced) long after the stream has been created.
Yeah, as well as other codecpar fields. How is the library user even
meant to know this happened? The doxy states codecpar is filled on
stream creation or during avformat_find_stream_info(), so they would not
expect it to change after that.
AV_PKT_DATA_PARAM_CHANGE seems to me that it's the proper way to
propagate these changes, yet it looks underused and fuzzily defined.
Maybe it should be expanded and improved for this.
> (The concat demuxer may even set extradata lateron and even change it
> after it has been allocated; see match_streams() in
> concat_read_packet(). The concat demuxer should actually add the global
> side data of every input to the first packet from said input which means
> that copying side data (whether in ff_stream_side_data_copy() or in
> avcodec_parameters_copy() is potentially problematic.)
>
>>>
>>>> * AVPacket.pts, AVPacket.dts and AVPacket.duration timing
>>>> information will be
>>>> * set if known. They may also be unset (i.e. AV_NOPTS_VALUE for
>>>> * pts/dts, 0 for duration) if the stream does not provide them.
>>>> The timing
>>>> @@ -209,6 +216,11 @@
>>>> * AVCodecParameters, rather than using @ref
>>>> avcodec_parameters_copy() during
>>>> * remuxing: there is no guarantee that the codec context values
>>>> remain valid
>>>> * for both input and output format contexts.
>>>> + * - There may be no overlap between AVCodecParameters.side_data and
>>>> side data in
>>>> + * packets. I.e. a given side data is either set by the caller in
>>>> + * AVCodecParameters, then it never appears in the packets, or the
>>>> side data is
>>>> + * sent through the packets (always in the first packet where the
>>>> value becomes
>>>> + * known or changes), then it does not appear in AVCodecParameters.
>>>
>>> I have to say, I don't really like this (and of course I am aware that
>>> you are basically copying the doxy of AVPacketSideData here). As you
>>
>> I can remove this part, to be added later if needed.
>>
>>> know, the Matroska muxer needs to add header fields in order to add
>>> certain packet side data to blocks later. In case of seekable output,
>>> one can update the header later, but in case of unseekable output that
>>> is not true. I'd like there to be an easy way for the user to signal the
>>> intention to send packet side data of a specific type later.
>>
>> Maybe a new AVFMT_FLAG_?
>>
>
> That would potentially be a new AVFMT_FLAG_ for every side data type;
> that makes no sense.
Yeah, missed the "specific type" part, and assumed you meant only a way
to signal a simple "Keep an eye for side data in packets" scenario.
>
>>>
>>>> * - The caller may fill in additional information, such as @ref
>>>> * AVFormatContext.metadata "global" or @ref AVStream.metadata
>>>> "per-stream"
>>>> * metadata, @ref AVFormatContext.chapters "chapters", @ref
>>>> @@ -937,6 +949,7 @@ typedef struct AVStream {
>>>> */
>>>> AVPacket attached_pic;
>>>> +#if FF_API_AVSTREAM_SIDE_DATA
>>>> /**
>>>> * An array of side data that applies to the whole stream (i.e.
>>>> the
>>>> * container does not allow it to change between packets).
>>>> @@ -953,13 +966,20 @@ typedef struct AVStream {
>>>> *
>>>> * Freed by libavformat in avformat_free_context().
>>>> *
>>>> - * @see av_format_inject_global_side_data()
>>>> + * @deprecated use AVStream's @ref AVCodecParameters.side_data
>>>> + * "codecpar side data".
>>>> */
>>>> + attribute_deprecated
>>>> AVPacketSideData *side_data;
>>>> /**
>>>> * The number of elements in the AVStream.side_data array.
>>>> + *
>>>> + * @deprecated use AVStream's @ref AVCodecParameters.side_data
>>>> + * "codecpar side data".
>>>> */
>>>> + attribute_deprecated
>>>> int nb_side_data;
>>>> +#endif
>>>> /**
>>>> * Flags indicating events happening on the stream, a
>>>> combination of
>>>> @@ -1715,11 +1735,18 @@ typedef struct AVFormatContext {
>>>> int (*io_close2)(struct AVFormatContext *s, AVIOContext *pb);
>>>> } AVFormatContext;
>>>> +#if FF_API_AVSTREAM_SIDE_DATA
>>>> /**
>>>> * This function will cause global side data to be injected in the
>>>> next packet
>>>> * of each stream as well as after any subsequent seek.
>>>> + *
>>>> + * @deprecated global side data is always available in every AVStream's
>>>> + * @ref AVCodecParameters.side_data "codecpar side data"
>>>> array.
>>>> + * @see av_packet_side_data_set_get()
>>>> */
>>>> +attribute_deprecated
>>>> void av_format_inject_global_side_data(AVFormatContext *s);
>>>> +#endif
>>>> /**
>>>> * Returns the method used to set ctx->duration.
>>>> @@ -1844,6 +1871,7 @@ const AVClass *av_stream_get_class(void);
>>>> */
>>>> AVStream *avformat_new_stream(AVFormatContext *s, const AVCodec *c);
>>>> +#if FF_API_AVSTREAM_SIDE_DATA
>>>> /**
>>>> * Wrap an existing array as stream side data.
>>>> *
>>>> @@ -1856,7 +1884,10 @@ AVStream *avformat_new_stream(AVFormatContext
>>>> *s, const AVCodec *c);
>>>> *
>>>> * @return zero on success, a negative AVERROR code on failure. On
>>>> failure,
>>>> * the stream is unchanged and the data remains owned by
>>>> the caller.
>>>> + * @deprecated use av_packet_side_data_set_add() with the stream's
>>>> + * @ref AVCodecParameters.side_data "codecpar side data"
>>>> */
>>>> +attribute_deprecated
>>>> int av_stream_add_side_data(AVStream *st, enum AVPacketSideDataType
>>>> type,
>>>> uint8_t *data, size_t size);
>>>> @@ -1868,7 +1899,10 @@ int av_stream_add_side_data(AVStream *st,
>>>> enum AVPacketSideDataType type,
>>>> * @param size side information size
>>>> *
>>>> * @return pointer to fresh allocated data or NULL otherwise
>>>> + * @deprecated use av_packet_side_data_set_new() with the stream's
>>>> + * @ref AVCodecParameters.side_data "codecpar side data"
>>>> */
>>>> +attribute_deprecated
>>>> uint8_t *av_stream_new_side_data(AVStream *stream,
>>>> enum AVPacketSideDataType type,
>>>> size_t size);
>>>> /**
>>>> @@ -1880,9 +1914,13 @@ uint8_t *av_stream_new_side_data(AVStream
>>>> *stream,
>>>> * or to zero if the desired side data is not present.
>>>> *
>>>> * @return pointer to data if present or NULL otherwise
>>>> + * @deprecated use av_packet_side_data_set_get() with the stream's
>>>> + * @ref AVCodecParameters.side_data "codecpar side data"
>>>> */
>>>> +attribute_deprecated
>>>> uint8_t *av_stream_get_side_data(const AVStream *stream,
>>>> enum AVPacketSideDataType type,
>>>> size_t *size);
>>>> +#endif
>>>> AVProgram *av_new_program(AVFormatContext *s, int id);
>>>>
>>>
>
> _______________________________________________
> 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".
_______________________________________________
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] 28+ messages in thread
* [FFmpeg-devel] [PATCH 04/10] fftools/ffmpeg: stop using AVStream.side_data
2023-09-06 17:44 [FFmpeg-devel] [PATCH 00/10 v4][RFC] AVCodecContext and AVCodecParameters side data James Almer
` (2 preceding siblings ...)
2023-09-06 17:44 ` [FFmpeg-devel] [PATCH 03/10] avformat/avformat: use the side data from AVStream.codecpar James Almer
@ 2023-09-06 17:44 ` James Almer
2023-09-06 17:44 ` [FFmpeg-devel] [PATCH 05/10] fftools/ffplay: " James Almer
` (7 subsequent siblings)
11 siblings, 0 replies; 28+ messages in thread
From: James Almer @ 2023-09-06 17:44 UTC (permalink / raw)
To: ffmpeg-devel
Signed-off-by: James Almer <jamrial@gmail.com>
---
fftools/ffmpeg_demux.c | 11 +++++++----
fftools/ffmpeg_enc.c | 31 +++++++++----------------------
fftools/ffmpeg_filter.c | 5 ++++-
fftools/ffmpeg_mux_init.c | 19 ++++++++++---------
4 files changed, 30 insertions(+), 36 deletions(-)
diff --git a/fftools/ffmpeg_demux.c b/fftools/ffmpeg_demux.c
index 48edbd7f6b..6cf6d2e971 100644
--- a/fftools/ffmpeg_demux.c
+++ b/fftools/ffmpeg_demux.c
@@ -482,8 +482,8 @@ static int input_packet_process(Demuxer *d, DemuxMsg *msg, AVPacket *src)
/* add the stream-global side data to the first packet */
if (ds->nb_packets == 1) {
- for (int i = 0; i < ist->st->nb_side_data; i++) {
- AVPacketSideData *src_sd = &ist->st->side_data[i];
+ for (int i = 0; i < ist->st->codecpar->side_data.nb_sd; i++) {
+ AVPacketSideData *src_sd = ist->st->codecpar->side_data.sd[i];
uint8_t *dst_data;
if (src_sd->type == AV_PKT_DATA_DISPLAYMATRIX)
@@ -979,6 +979,7 @@ static int add_display_matrix_to_stream(const OptionsContext *o,
AVFormatContext *ctx, InputStream *ist)
{
AVStream *st = ist->st;
+ AVPacketSideData *sd;
double rotation = DBL_MAX;
int hflip = -1, vflip = -1;
int hflip_set = 0, vflip_set = 0, rotation_set = 0;
@@ -995,12 +996,14 @@ static int add_display_matrix_to_stream(const OptionsContext *o,
if (!rotation_set && !hflip_set && !vflip_set)
return 0;
- buf = (int32_t *)av_stream_new_side_data(st, AV_PKT_DATA_DISPLAYMATRIX, sizeof(int32_t) * 9);
- if (!buf) {
+ sd = av_packet_side_data_set_new(&st->codecpar->side_data, AV_PKT_DATA_DISPLAYMATRIX,
+ sizeof(int32_t) * 9, 0);
+ if (!sd) {
av_log(ist, AV_LOG_FATAL, "Failed to generate a display matrix!\n");
return AVERROR(ENOMEM);
}
+ buf = (int32_t *)sd->data;
av_display_rotation_set(buf,
rotation_set ? -(rotation) : -0.0f);
diff --git a/fftools/ffmpeg_enc.c b/fftools/ffmpeg_enc.c
index f28884e50c..503a2c033b 100644
--- a/fftools/ffmpeg_enc.c
+++ b/fftools/ffmpeg_enc.c
@@ -460,20 +460,6 @@ int enc_open(OutputStream *ost, AVFrame *frame)
return ret;
}
- if (ost->enc_ctx->nb_coded_side_data) {
- int i;
-
- for (i = 0; i < ost->enc_ctx->nb_coded_side_data; i++) {
- const AVPacketSideData *sd_src = &ost->enc_ctx->coded_side_data[i];
- uint8_t *dst_data;
-
- dst_data = av_stream_new_side_data(ost->st, sd_src->type, sd_src->size);
- if (!dst_data)
- return AVERROR(ENOMEM);
- memcpy(dst_data, sd_src->data, sd_src->size);
- }
- }
-
/*
* Add global input side data. For now this is naive, and copies it
* from the input stream's global side data. All side data should
@@ -483,15 +469,16 @@ int enc_open(OutputStream *ost, AVFrame *frame)
*/
if (ist) {
int i;
- for (i = 0; i < ist->st->nb_side_data; i++) {
- AVPacketSideData *sd = &ist->st->side_data[i];
- if (sd->type != AV_PKT_DATA_CPB_PROPERTIES) {
- uint8_t *dst = av_stream_new_side_data(ost->st, sd->type, sd->size);
- if (!dst)
+ for (i = 0; i < ist->st->codecpar->side_data.nb_sd; i++) {
+ AVPacketSideData *sd_src = ist->st->codecpar->side_data.sd[i];
+ if (sd_src->type != AV_PKT_DATA_CPB_PROPERTIES) {
+ AVPacketSideData *sd_dst = av_packet_side_data_set_new(&ost->par_in->side_data,
+ sd_src->type, sd_src->size, 0);
+ if (!sd_dst)
return AVERROR(ENOMEM);
- memcpy(dst, sd->data, sd->size);
- if (ist->autorotate && sd->type == AV_PKT_DATA_DISPLAYMATRIX)
- av_display_rotation_set((int32_t *)dst, 0);
+ memcpy(sd_dst->data, sd_src->data, sd_src->size);
+ if (ist->autorotate && sd_src->type == AV_PKT_DATA_DISPLAYMATRIX)
+ av_display_rotation_set((int32_t *)sd_dst->data, 0);
}
}
}
diff --git a/fftools/ffmpeg_filter.c b/fftools/ffmpeg_filter.c
index 9bf870b615..4be415c4f8 100644
--- a/fftools/ffmpeg_filter.c
+++ b/fftools/ffmpeg_filter.c
@@ -1383,11 +1383,14 @@ static int configure_input_video_filter(FilterGraph *fg, InputFilter *ifilter,
// TODO: insert hwaccel enabled filters like transpose_vaapi into the graph
if (ist->autorotate && !(desc->flags & AV_PIX_FMT_FLAG_HWACCEL)) {
+ AVPacketSideData *sd = NULL;
int32_t *displaymatrix = ifp->displaymatrix;
double theta;
if (!ifp->displaymatrix_present)
- displaymatrix = (int32_t *)av_stream_get_side_data(ist->st, AV_PKT_DATA_DISPLAYMATRIX, NULL);
+ sd = av_packet_side_data_set_get(&ist->st->codecpar->side_data, AV_PKT_DATA_DISPLAYMATRIX);
+ if (sd)
+ displaymatrix = (int32_t *)sd->data;
theta = get_rotation(displaymatrix);
if (fabs(theta - 90) < 1.0) {
diff --git a/fftools/ffmpeg_mux_init.c b/fftools/ffmpeg_mux_init.c
index cf4cd2d5b7..46bc90b1c2 100644
--- a/fftools/ffmpeg_mux_init.c
+++ b/fftools/ffmpeg_mux_init.c
@@ -1041,24 +1041,25 @@ static int streamcopy_init(const Muxer *mux, OutputStream *ost)
}
}
- for (int i = 0; i < ist->st->nb_side_data; i++) {
- const AVPacketSideData *sd_src = &ist->st->side_data[i];
- uint8_t *dst_data;
+ for (int i = 0; i < ist->st->codecpar->side_data.nb_sd; i++) {
+ const AVPacketSideData *sd_src = ist->st->codecpar->side_data.sd[i];
+ AVPacketSideData *sd_dst;
- dst_data = av_stream_new_side_data(ost->st, sd_src->type, sd_src->size);
- if (!dst_data) {
+ sd_dst = av_packet_side_data_set_new(&ost->st->codecpar->side_data, sd_src->type, sd_src->size, 0);
+ if (!sd_dst) {
ret = AVERROR(ENOMEM);
goto fail;
}
- memcpy(dst_data, sd_src->data, sd_src->size);
+ memcpy(sd_dst->data, sd_src->data, sd_src->size);
}
#if FFMPEG_ROTATION_METADATA
if (ost->rotate_overridden) {
- uint8_t *sd = av_stream_new_side_data(ost->st, AV_PKT_DATA_DISPLAYMATRIX,
- sizeof(int32_t) * 9);
+ AVPacketSideData *sd = av_packet_side_data_set_new(&ost->st->codecpar->side_data,
+ AV_PKT_DATA_DISPLAYMATRIX,
+ sizeof(int32_t) * 9, 0);
if (sd)
- av_display_rotation_set((int32_t *)sd, -ost->rotate_override_value);
+ av_display_rotation_set((int32_t *)sd->data, -ost->rotate_override_value);
}
#endif
--
2.42.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] 28+ messages in thread* [FFmpeg-devel] [PATCH 05/10] fftools/ffplay: stop using AVStream.side_data
2023-09-06 17:44 [FFmpeg-devel] [PATCH 00/10 v4][RFC] AVCodecContext and AVCodecParameters side data James Almer
` (3 preceding siblings ...)
2023-09-06 17:44 ` [FFmpeg-devel] [PATCH 04/10] fftools/ffmpeg: stop using AVStream.side_data James Almer
@ 2023-09-06 17:44 ` James Almer
2023-09-06 17:44 ` [FFmpeg-devel] [PATCH 06/10] fftools/ffprobe: " James Almer
` (6 subsequent siblings)
11 siblings, 0 replies; 28+ messages in thread
From: James Almer @ 2023-09-06 17:44 UTC (permalink / raw)
To: ffmpeg-devel
Signed-off-by: James Almer <jamrial@gmail.com>
---
fftools/ffplay.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/fftools/ffplay.c b/fftools/ffplay.c
index 006da7ab57..a0d5811c94 100644
--- a/fftools/ffplay.c
+++ b/fftools/ffplay.c
@@ -1916,8 +1916,12 @@ static int configure_video_filters(AVFilterGraph *graph, VideoState *is, const c
AVFrameSideData *sd = av_frame_get_side_data(frame, AV_FRAME_DATA_DISPLAYMATRIX);
if (sd)
displaymatrix = (int32_t *)sd->data;
- if (!displaymatrix)
- displaymatrix = (int32_t *)av_stream_get_side_data(is->video_st, AV_PKT_DATA_DISPLAYMATRIX, NULL);
+ if (!displaymatrix) {
+ AVPacketSideData *sd = av_packet_side_data_set_get(&is->video_st->codecpar->side_data,
+ AV_PKT_DATA_DISPLAYMATRIX);
+ if (sd)
+ displaymatrix = (int32_t *)sd->data;
+ }
theta = get_rotation(displaymatrix);
if (fabs(theta - 90) < 1.0) {
--
2.42.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] 28+ messages in thread* [FFmpeg-devel] [PATCH 06/10] fftools/ffprobe: stop using AVStream.side_data
2023-09-06 17:44 [FFmpeg-devel] [PATCH 00/10 v4][RFC] AVCodecContext and AVCodecParameters side data James Almer
` (4 preceding siblings ...)
2023-09-06 17:44 ` [FFmpeg-devel] [PATCH 05/10] fftools/ffplay: " James Almer
@ 2023-09-06 17:44 ` James Almer
2023-09-06 17:44 ` [FFmpeg-devel] [PATCH 07/10] avcodec/hevcdec: check for DOVI configuration record in AVCodecContext side data James Almer
` (5 subsequent siblings)
11 siblings, 0 replies; 28+ messages in thread
From: James Almer @ 2023-09-06 17:44 UTC (permalink / raw)
To: ffmpeg-devel
Signed-off-by: James Almer <jamrial@gmail.com>
---
fftools/ffprobe.c | 30 ++++++++++++++----------------
1 file changed, 14 insertions(+), 16 deletions(-)
diff --git a/fftools/ffprobe.c b/fftools/ffprobe.c
index 4fcfe1164b..28ccb45b26 100644
--- a/fftools/ffprobe.c
+++ b/fftools/ffprobe.c
@@ -2279,16 +2279,9 @@ static void print_ambient_viewing_environment(WriterContext *w,
static void print_pkt_side_data(WriterContext *w,
AVCodecParameters *par,
- const AVPacketSideData *side_data,
- int nb_side_data,
- SectionID id_data_list,
+ const AVPacketSideData *sd,
SectionID id_data)
{
- int i;
-
- writer_print_section_header(w, id_data_list);
- for (i = 0; i < nb_side_data; i++) {
- const AVPacketSideData *sd = &side_data[i];
const char *name = av_packet_side_data_name(sd->type);
writer_print_section_header(w, id_data);
@@ -2382,9 +2375,6 @@ static void print_pkt_side_data(WriterContext *w,
} else if (sd->type == AV_PKT_DATA_AFD && sd->size > 0) {
print_int("active_format", *sd->data);
}
- writer_print_section_footer(w);
- }
- writer_print_section_footer(w);
}
static void print_private_data(WriterContext *w, void *priv_data)
@@ -2544,9 +2534,13 @@ static void show_packet(WriterContext *w, InputFile *ifile, AVPacket *pkt, int p
av_dict_free(&dict);
}
- print_pkt_side_data(w, st->codecpar, pkt->side_data, pkt->side_data_elems,
- SECTION_ID_PACKET_SIDE_DATA_LIST,
+ writer_print_section_header(w, SECTION_ID_PACKET_SIDE_DATA_LIST);
+ for (int i = 0; i < pkt->side_data_elems; i++) {
+ print_pkt_side_data(w, st->codecpar, &pkt->side_data[i],
SECTION_ID_PACKET_SIDE_DATA);
+ writer_print_section_footer(w);
+ }
+ writer_print_section_footer(w);
}
writer_print_section_footer(w);
@@ -3188,10 +3182,14 @@ static int show_stream(WriterContext *w, AVFormatContext *fmt_ctx, int stream_id
if (do_show_stream_tags)
ret = show_tags(w, stream->metadata, in_program ? SECTION_ID_PROGRAM_STREAM_TAGS : SECTION_ID_STREAM_TAGS);
- if (stream->nb_side_data) {
- print_pkt_side_data(w, stream->codecpar, stream->side_data, stream->nb_side_data,
- SECTION_ID_STREAM_SIDE_DATA_LIST,
+ if (stream->codecpar->side_data.nb_sd) {
+ writer_print_section_header(w, SECTION_ID_STREAM_SIDE_DATA_LIST);
+ for (int i = 0; i < stream->codecpar->side_data.nb_sd; i++) {
+ print_pkt_side_data(w, stream->codecpar, stream->codecpar->side_data.sd[i],
SECTION_ID_STREAM_SIDE_DATA);
+ writer_print_section_footer(w);
+ }
+ writer_print_section_footer(w);
}
writer_print_section_footer(w);
--
2.42.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] 28+ messages in thread* [FFmpeg-devel] [PATCH 07/10] avcodec/hevcdec: check for DOVI configuration record in AVCodecContext side data
2023-09-06 17:44 [FFmpeg-devel] [PATCH 00/10 v4][RFC] AVCodecContext and AVCodecParameters side data James Almer
` (5 preceding siblings ...)
2023-09-06 17:44 ` [FFmpeg-devel] [PATCH 06/10] fftools/ffprobe: " James Almer
@ 2023-09-06 17:44 ` James Almer
2023-09-11 17:58 ` Andreas Rheinhardt
2023-09-06 17:44 ` [FFmpeg-devel] [PATCH 08/10] avcodec/decode: check for global side data " James Almer
` (4 subsequent siblings)
11 siblings, 1 reply; 28+ messages in thread
From: James Almer @ 2023-09-06 17:44 UTC (permalink / raw)
To: ffmpeg-devel
Signed-off-by: James Almer <jamrial@gmail.com>
---
libavcodec/avcodec.h | 2 +-
libavcodec/hevcdec.c | 15 ++++++++++++++-
libavcodec/internal.h | 3 +++
libavcodec/utils.c | 10 ++++++++++
4 files changed, 28 insertions(+), 2 deletions(-)
diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
index 1f477209b0..b875076ba5 100644
--- a/libavcodec/avcodec.h
+++ b/libavcodec/avcodec.h
@@ -1893,7 +1893,7 @@ typedef struct AVCodecContext {
/**
* Additional data associated with the entire coded stream.
*
- * - decoding: unused
+ * - decoding: set by user
* - encoding: may be set by libavcodec after avcodec_open2().
*/
AVPacketSideData *coded_side_data;
diff --git a/libavcodec/hevcdec.c b/libavcodec/hevcdec.c
index df40c91ba6..162b832800 100644
--- a/libavcodec/hevcdec.c
+++ b/libavcodec/hevcdec.c
@@ -3337,8 +3337,15 @@ static int hevc_decode_frame(AVCodecContext *avctx, AVFrame *rframe,
}
sd = av_packet_get_side_data(avpkt, AV_PKT_DATA_DOVI_CONF, &sd_size);
- if (sd && sd_size > 0)
+ if (sd && sd_size > 0) {
+ int old = s->dovi_ctx.dv_profile;
+
ff_dovi_update_cfg(&s->dovi_ctx, (AVDOVIDecoderConfigurationRecord *) sd);
+ if (old)
+ av_log(avctx, AV_LOG_DEBUG,
+ "New DOVI configuration record from input packet (profile %d -> %u).\n",
+ old, s->dovi_ctx.dv_profile);
+ }
s->ref = NULL;
ret = decode_nal_units(s, avpkt->data, avpkt->size);
@@ -3641,12 +3648,18 @@ static av_cold int hevc_decode_init(AVCodecContext *avctx)
atomic_init(&s->wpp_err, 0);
if (!avctx->internal->is_copy) {
+ const AVPacketSideData *sd;
+
if (avctx->extradata_size > 0 && avctx->extradata) {
ret = hevc_decode_extradata(s, avctx->extradata, avctx->extradata_size, 1);
if (ret < 0) {
return ret;
}
}
+
+ sd = ff_get_coded_side_data(avctx, AV_PKT_DATA_DOVI_CONF);
+ if (sd && sd->size > 0)
+ ff_dovi_update_cfg(&s->dovi_ctx, (AVDOVIDecoderConfigurationRecord *) sd->data);
}
return 0;
diff --git a/libavcodec/internal.h b/libavcodec/internal.h
index 83e0bc3fb2..517d96fb97 100644
--- a/libavcodec/internal.h
+++ b/libavcodec/internal.h
@@ -202,4 +202,7 @@ int ff_alloc_timecode_sei(const AVFrame *frame, AVRational rate, size_t prefix_l
*/
int64_t ff_guess_coded_bitrate(AVCodecContext *avctx);
+AVPacketSideData *ff_get_coded_side_data(const AVCodecContext *avctx,
+ enum AVPacketSideDataType type);
+
#endif /* AVCODEC_INTERNAL_H */
diff --git a/libavcodec/utils.c b/libavcodec/utils.c
index d54e050848..4fbd591bf1 100644
--- a/libavcodec/utils.c
+++ b/libavcodec/utils.c
@@ -1111,3 +1111,13 @@ int64_t ff_guess_coded_bitrate(AVCodecContext *avctx)
return bitrate;
}
+
+AVPacketSideData *ff_get_coded_side_data(const AVCodecContext *avctx,
+ enum AVPacketSideDataType type)
+{
+ for (int i = 0; i < avctx->nb_coded_side_data; i++)
+ if (avctx->coded_side_data[i].type == type)
+ return &avctx->coded_side_data[i];
+
+ return NULL;
+}
--
2.42.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] 28+ messages in thread* Re: [FFmpeg-devel] [PATCH 07/10] avcodec/hevcdec: check for DOVI configuration record in AVCodecContext side data
2023-09-06 17:44 ` [FFmpeg-devel] [PATCH 07/10] avcodec/hevcdec: check for DOVI configuration record in AVCodecContext side data James Almer
@ 2023-09-11 17:58 ` Andreas Rheinhardt
0 siblings, 0 replies; 28+ messages in thread
From: Andreas Rheinhardt @ 2023-09-11 17:58 UTC (permalink / raw)
To: ffmpeg-devel
James Almer:
> Signed-off-by: James Almer <jamrial@gmail.com>
> ---
> libavcodec/avcodec.h | 2 +-
> libavcodec/hevcdec.c | 15 ++++++++++++++-
> libavcodec/internal.h | 3 +++
> libavcodec/utils.c | 10 ++++++++++
> 4 files changed, 28 insertions(+), 2 deletions(-)
>
> diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
> index 1f477209b0..b875076ba5 100644
> --- a/libavcodec/avcodec.h
> +++ b/libavcodec/avcodec.h
> @@ -1893,7 +1893,7 @@ typedef struct AVCodecContext {
> /**
> * Additional data associated with the entire coded stream.
> *
> - * - decoding: unused
> + * - decoding: set by user
> * - encoding: may be set by libavcodec after avcodec_open2().
> */
> AVPacketSideData *coded_side_data;
> diff --git a/libavcodec/hevcdec.c b/libavcodec/hevcdec.c
> index df40c91ba6..162b832800 100644
> --- a/libavcodec/hevcdec.c
> +++ b/libavcodec/hevcdec.c
> @@ -3337,8 +3337,15 @@ static int hevc_decode_frame(AVCodecContext *avctx, AVFrame *rframe,
> }
>
> sd = av_packet_get_side_data(avpkt, AV_PKT_DATA_DOVI_CONF, &sd_size);
> - if (sd && sd_size > 0)
> + if (sd && sd_size > 0) {
> + int old = s->dovi_ctx.dv_profile;
> +
> ff_dovi_update_cfg(&s->dovi_ctx, (AVDOVIDecoderConfigurationRecord *) sd);
> + if (old)
> + av_log(avctx, AV_LOG_DEBUG,
> + "New DOVI configuration record from input packet (profile %d -> %u).\n",
> + old, s->dovi_ctx.dv_profile);
> + }
>
> s->ref = NULL;
> ret = decode_nal_units(s, avpkt->data, avpkt->size);
> @@ -3641,12 +3648,18 @@ static av_cold int hevc_decode_init(AVCodecContext *avctx)
> atomic_init(&s->wpp_err, 0);
>
> if (!avctx->internal->is_copy) {
> + const AVPacketSideData *sd;
> +
> if (avctx->extradata_size > 0 && avctx->extradata) {
> ret = hevc_decode_extradata(s, avctx->extradata, avctx->extradata_size, 1);
> if (ret < 0) {
> return ret;
> }
> }
> +
> + sd = ff_get_coded_side_data(avctx, AV_PKT_DATA_DOVI_CONF);
> + if (sd && sd->size > 0)
> + ff_dovi_update_cfg(&s->dovi_ctx, (AVDOVIDecoderConfigurationRecord *) sd->data);
> }
>
> return 0;
> diff --git a/libavcodec/internal.h b/libavcodec/internal.h
> index 83e0bc3fb2..517d96fb97 100644
> --- a/libavcodec/internal.h
> +++ b/libavcodec/internal.h
> @@ -202,4 +202,7 @@ int ff_alloc_timecode_sei(const AVFrame *frame, AVRational rate, size_t prefix_l
> */
> int64_t ff_guess_coded_bitrate(AVCodecContext *avctx);
>
> +AVPacketSideData *ff_get_coded_side_data(const AVCodecContext *avctx,
> + enum AVPacketSideDataType type);
Why is this not in decode.h? An encoder has little reason to inquire
whether side data exists (given that if it exists, it has added it).
> +
> #endif /* AVCODEC_INTERNAL_H */
> diff --git a/libavcodec/utils.c b/libavcodec/utils.c
> index d54e050848..4fbd591bf1 100644
> --- a/libavcodec/utils.c
> +++ b/libavcodec/utils.c
> @@ -1111,3 +1111,13 @@ int64_t ff_guess_coded_bitrate(AVCodecContext *avctx)
>
> return bitrate;
> }
> +
> +AVPacketSideData *ff_get_coded_side_data(const AVCodecContext *avctx,
> + enum AVPacketSideDataType type)
> +{
> + for (int i = 0; i < avctx->nb_coded_side_data; i++)
> + if (avctx->coded_side_data[i].type == type)
> + return &avctx->coded_side_data[i];
> +
> + return NULL;
> +}
_______________________________________________
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] 28+ messages in thread
* [FFmpeg-devel] [PATCH 08/10] avcodec/decode: check for global side data in AVCodecContext side data
2023-09-06 17:44 [FFmpeg-devel] [PATCH 00/10 v4][RFC] AVCodecContext and AVCodecParameters side data James Almer
` (6 preceding siblings ...)
2023-09-06 17:44 ` [FFmpeg-devel] [PATCH 07/10] avcodec/hevcdec: check for DOVI configuration record in AVCodecContext side data James Almer
@ 2023-09-06 17:44 ` James Almer
2023-09-11 18:35 ` Andreas Rheinhardt
2023-09-06 17:44 ` [FFmpeg-devel] [PATCH 09/10] fftools/ffmpeg: stop injecting stream side data in packets James Almer
` (3 subsequent siblings)
11 siblings, 1 reply; 28+ messages in thread
From: James Almer @ 2023-09-06 17:44 UTC (permalink / raw)
To: ffmpeg-devel
Signed-off-by: James Almer <jamrial@gmail.com>
---
libavcodec/decode.c | 55 +++++++++++++++++++++++++++++++++++++--------
libavcodec/decode.h | 2 +-
2 files changed, 47 insertions(+), 10 deletions(-)
diff --git a/libavcodec/decode.c b/libavcodec/decode.c
index 169ee79acd..1a431ba7d7 100644
--- a/libavcodec/decode.c
+++ b/libavcodec/decode.c
@@ -1409,25 +1409,31 @@ static int add_metadata_from_side_data(const AVPacket *avpkt, AVFrame *frame)
return av_packet_unpack_dictionary(side_metadata, size, frame_md);
}
-int ff_decode_frame_props_from_pkt(const AVCodecContext *avctx,
+static const struct {
+ enum AVPacketSideDataType packet;
+ enum AVFrameSideDataType frame;
+} sd_global_map[] = {
+ { AV_PKT_DATA_REPLAYGAIN , AV_FRAME_DATA_REPLAYGAIN },
+ { AV_PKT_DATA_SPHERICAL, AV_FRAME_DATA_SPHERICAL },
+ { AV_PKT_DATA_STEREO3D, AV_FRAME_DATA_STEREO3D },
+ { AV_PKT_DATA_AUDIO_SERVICE_TYPE, AV_FRAME_DATA_AUDIO_SERVICE_TYPE },
+ { AV_PKT_DATA_MASTERING_DISPLAY_METADATA, AV_FRAME_DATA_MASTERING_DISPLAY_METADATA },
+ { AV_PKT_DATA_CONTENT_LIGHT_LEVEL, AV_FRAME_DATA_CONTENT_LIGHT_LEVEL },
+ { AV_PKT_DATA_ICC_PROFILE, AV_FRAME_DATA_ICC_PROFILE },
+ { AV_PKT_DATA_DYNAMIC_HDR10_PLUS, AV_FRAME_DATA_DYNAMIC_HDR_PLUS },
+};
+
+int ff_decode_frame_props_from_pkt(AVCodecContext *avctx,
AVFrame *frame, const AVPacket *pkt)
{
static const struct {
enum AVPacketSideDataType packet;
enum AVFrameSideDataType frame;
} sd[] = {
- { AV_PKT_DATA_REPLAYGAIN , AV_FRAME_DATA_REPLAYGAIN },
{ AV_PKT_DATA_DISPLAYMATRIX, AV_FRAME_DATA_DISPLAYMATRIX },
- { AV_PKT_DATA_SPHERICAL, AV_FRAME_DATA_SPHERICAL },
- { AV_PKT_DATA_STEREO3D, AV_FRAME_DATA_STEREO3D },
- { AV_PKT_DATA_AUDIO_SERVICE_TYPE, AV_FRAME_DATA_AUDIO_SERVICE_TYPE },
- { AV_PKT_DATA_MASTERING_DISPLAY_METADATA, AV_FRAME_DATA_MASTERING_DISPLAY_METADATA },
- { AV_PKT_DATA_CONTENT_LIGHT_LEVEL, AV_FRAME_DATA_CONTENT_LIGHT_LEVEL },
{ AV_PKT_DATA_A53_CC, AV_FRAME_DATA_A53_CC },
{ AV_PKT_DATA_AFD, AV_FRAME_DATA_AFD },
- { AV_PKT_DATA_ICC_PROFILE, AV_FRAME_DATA_ICC_PROFILE },
{ AV_PKT_DATA_S12M_TIMECODE, AV_FRAME_DATA_S12M_TIMECODE },
- { AV_PKT_DATA_DYNAMIC_HDR10_PLUS, AV_FRAME_DATA_DYNAMIC_HDR_PLUS },
{ AV_PKT_DATA_SKIP_SAMPLES, AV_FRAME_DATA_SKIP_SAMPLES },
};
@@ -1440,6 +1446,23 @@ FF_DISABLE_DEPRECATION_WARNINGS
FF_ENABLE_DEPRECATION_WARNINGS
#endif
+ for (int i = 0; i < FF_ARRAY_ELEMS(sd_global_map); i++) {
+ size_t size;
+ uint8_t *packet_sd = av_packet_get_side_data(pkt, sd_global_map[i].packet, &size);
+ if (packet_sd) {
+ AVFrameSideData *frame_sd;
+
+ av_log(avctx, AV_LOG_DEBUG,
+ "Overwriting global side data of type \"%s\" in output frame with side data "
+ "from input packet.\n", av_packet_side_data_name(sd_global_map[i].packet));
+
+ av_frame_remove_side_data(frame, sd_global_map[i].frame);
+ frame_sd = av_frame_new_side_data(frame, sd_global_map[i].frame, size);
+ if (!frame_sd)
+ return AVERROR(ENOMEM);
+ memcpy(frame_sd->data, packet_sd, size);
+ }
+ }
for (int i = 0; i < FF_ARRAY_ELEMS(sd); i++) {
size_t size;
uint8_t *packet_sd = av_packet_get_side_data(pkt, sd[i].packet, &size);
@@ -1476,6 +1499,20 @@ int ff_decode_frame_props(AVCodecContext *avctx, AVFrame *frame)
const AVPacket *pkt = avctx->internal->last_pkt_props;
int ret;
+ for (int i = 0; i < FF_ARRAY_ELEMS(sd_global_map); i++) {
+ const AVPacketSideData *packet_sd = ff_get_coded_side_data(avctx,
+ sd_global_map[i].packet);
+ if (packet_sd) {
+ AVFrameSideData *frame_sd = av_frame_new_side_data(frame,
+ sd_global_map[i].frame,
+ packet_sd->size);
+ if (!frame_sd)
+ return AVERROR(ENOMEM);
+
+ memcpy(frame_sd->data, packet_sd->data, packet_sd->size);
+ }
+ }
+
if (!(ffcodec(avctx->codec)->caps_internal & FF_CODEC_CAP_SETS_FRAME_PROPS)) {
ret = ff_decode_frame_props_from_pkt(avctx, frame, pkt);
if (ret < 0)
diff --git a/libavcodec/decode.h b/libavcodec/decode.h
index a52152e4a7..52e5aafc34 100644
--- a/libavcodec/decode.h
+++ b/libavcodec/decode.h
@@ -67,7 +67,7 @@ int ff_decode_get_packet(AVCodecContext *avctx, AVPacket *pkt);
/**
* Set various frame properties from the provided packet.
*/
-int ff_decode_frame_props_from_pkt(const AVCodecContext *avctx,
+int ff_decode_frame_props_from_pkt(AVCodecContext *avctx,
AVFrame *frame, const AVPacket *pkt);
/**
--
2.42.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] 28+ messages in thread* Re: [FFmpeg-devel] [PATCH 08/10] avcodec/decode: check for global side data in AVCodecContext side data
2023-09-06 17:44 ` [FFmpeg-devel] [PATCH 08/10] avcodec/decode: check for global side data " James Almer
@ 2023-09-11 18:35 ` Andreas Rheinhardt
2023-09-11 18:41 ` James Almer
0 siblings, 1 reply; 28+ messages in thread
From: Andreas Rheinhardt @ 2023-09-11 18:35 UTC (permalink / raw)
To: ffmpeg-devel
James Almer:
> Signed-off-by: James Almer <jamrial@gmail.com>
> ---
> libavcodec/decode.c | 55 +++++++++++++++++++++++++++++++++++++--------
> libavcodec/decode.h | 2 +-
> 2 files changed, 47 insertions(+), 10 deletions(-)
>
> diff --git a/libavcodec/decode.c b/libavcodec/decode.c
> index 169ee79acd..1a431ba7d7 100644
> --- a/libavcodec/decode.c
> +++ b/libavcodec/decode.c
> @@ -1409,25 +1409,31 @@ static int add_metadata_from_side_data(const AVPacket *avpkt, AVFrame *frame)
> return av_packet_unpack_dictionary(side_metadata, size, frame_md);
> }
>
> -int ff_decode_frame_props_from_pkt(const AVCodecContext *avctx,
> +static const struct {
> + enum AVPacketSideDataType packet;
> + enum AVFrameSideDataType frame;
> +} sd_global_map[] = {
> + { AV_PKT_DATA_REPLAYGAIN , AV_FRAME_DATA_REPLAYGAIN },
> + { AV_PKT_DATA_SPHERICAL, AV_FRAME_DATA_SPHERICAL },
> + { AV_PKT_DATA_STEREO3D, AV_FRAME_DATA_STEREO3D },
> + { AV_PKT_DATA_AUDIO_SERVICE_TYPE, AV_FRAME_DATA_AUDIO_SERVICE_TYPE },
> + { AV_PKT_DATA_MASTERING_DISPLAY_METADATA, AV_FRAME_DATA_MASTERING_DISPLAY_METADATA },
> + { AV_PKT_DATA_CONTENT_LIGHT_LEVEL, AV_FRAME_DATA_CONTENT_LIGHT_LEVEL },
> + { AV_PKT_DATA_ICC_PROFILE, AV_FRAME_DATA_ICC_PROFILE },
> + { AV_PKT_DATA_DYNAMIC_HDR10_PLUS, AV_FRAME_DATA_DYNAMIC_HDR_PLUS },
> +};
> +
> +int ff_decode_frame_props_from_pkt(AVCodecContext *avctx,
> AVFrame *frame, const AVPacket *pkt)
> {
> static const struct {
> enum AVPacketSideDataType packet;
> enum AVFrameSideDataType frame;
> } sd[] = {
> - { AV_PKT_DATA_REPLAYGAIN , AV_FRAME_DATA_REPLAYGAIN },
> { AV_PKT_DATA_DISPLAYMATRIX, AV_FRAME_DATA_DISPLAYMATRIX },
> - { AV_PKT_DATA_SPHERICAL, AV_FRAME_DATA_SPHERICAL },
> - { AV_PKT_DATA_STEREO3D, AV_FRAME_DATA_STEREO3D },
> - { AV_PKT_DATA_AUDIO_SERVICE_TYPE, AV_FRAME_DATA_AUDIO_SERVICE_TYPE },
> - { AV_PKT_DATA_MASTERING_DISPLAY_METADATA, AV_FRAME_DATA_MASTERING_DISPLAY_METADATA },
> - { AV_PKT_DATA_CONTENT_LIGHT_LEVEL, AV_FRAME_DATA_CONTENT_LIGHT_LEVEL },
> { AV_PKT_DATA_A53_CC, AV_FRAME_DATA_A53_CC },
> { AV_PKT_DATA_AFD, AV_FRAME_DATA_AFD },
> - { AV_PKT_DATA_ICC_PROFILE, AV_FRAME_DATA_ICC_PROFILE },
> { AV_PKT_DATA_S12M_TIMECODE, AV_FRAME_DATA_S12M_TIMECODE },
> - { AV_PKT_DATA_DYNAMIC_HDR10_PLUS, AV_FRAME_DATA_DYNAMIC_HDR_PLUS },
> { AV_PKT_DATA_SKIP_SAMPLES, AV_FRAME_DATA_SKIP_SAMPLES },
> };
>
> @@ -1440,6 +1446,23 @@ FF_DISABLE_DEPRECATION_WARNINGS
> FF_ENABLE_DEPRECATION_WARNINGS
> #endif
>
> + for (int i = 0; i < FF_ARRAY_ELEMS(sd_global_map); i++) {
> + size_t size;
> + uint8_t *packet_sd = av_packet_get_side_data(pkt, sd_global_map[i].packet, &size);
> + if (packet_sd) {
> + AVFrameSideData *frame_sd;
> +
> + av_log(avctx, AV_LOG_DEBUG,
> + "Overwriting global side data of type \"%s\" in output frame with side data "
> + "from input packet.\n", av_packet_side_data_name(sd_global_map[i].packet));
Overwriting? Is it guaranteed that the frame has the side data at all?
And if it had, why would you overwrite it at all? After all, if the
decoder added something, isn't this to be preferred to stuff from
AVPacket side data?
Apart from that: packet_sd should be const.
> +
> + av_frame_remove_side_data(frame, sd_global_map[i].frame);
> + frame_sd = av_frame_new_side_data(frame, sd_global_map[i].frame, size);
> + if (!frame_sd)
> + return AVERROR(ENOMEM);
> + memcpy(frame_sd->data, packet_sd, size);
> + }
> + }
> for (int i = 0; i < FF_ARRAY_ELEMS(sd); i++) {
> size_t size;
> uint8_t *packet_sd = av_packet_get_side_data(pkt, sd[i].packet, &size);
> @@ -1476,6 +1499,20 @@ int ff_decode_frame_props(AVCodecContext *avctx, AVFrame *frame)
> const AVPacket *pkt = avctx->internal->last_pkt_props;
> int ret;
>
> + for (int i = 0; i < FF_ARRAY_ELEMS(sd_global_map); i++) {
> + const AVPacketSideData *packet_sd = ff_get_coded_side_data(avctx,
> + sd_global_map[i].packet);
> + if (packet_sd) {
> + AVFrameSideData *frame_sd = av_frame_new_side_data(frame,
> + sd_global_map[i].frame,
> + packet_sd->size);
> + if (!frame_sd)
> + return AVERROR(ENOMEM);
> +
> + memcpy(frame_sd->data, packet_sd->data, packet_sd->size);
> + }
> + }
> +
> if (!(ffcodec(avctx->codec)->caps_internal & FF_CODEC_CAP_SETS_FRAME_PROPS)) {
> ret = ff_decode_frame_props_from_pkt(avctx, frame, pkt);
> if (ret < 0)
> diff --git a/libavcodec/decode.h b/libavcodec/decode.h
> index a52152e4a7..52e5aafc34 100644
> --- a/libavcodec/decode.h
> +++ b/libavcodec/decode.h
> @@ -67,7 +67,7 @@ int ff_decode_get_packet(AVCodecContext *avctx, AVPacket *pkt);
> /**
> * Set various frame properties from the provided packet.
> */
> -int ff_decode_frame_props_from_pkt(const AVCodecContext *avctx,
> +int ff_decode_frame_props_from_pkt(AVCodecContext *avctx,
> AVFrame *frame, const AVPacket *pkt);
>
> /**
_______________________________________________
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] 28+ messages in thread* Re: [FFmpeg-devel] [PATCH 08/10] avcodec/decode: check for global side data in AVCodecContext side data
2023-09-11 18:35 ` Andreas Rheinhardt
@ 2023-09-11 18:41 ` James Almer
0 siblings, 0 replies; 28+ messages in thread
From: James Almer @ 2023-09-11 18:41 UTC (permalink / raw)
To: ffmpeg-devel
On 9/11/2023 3:35 PM, Andreas Rheinhardt wrote:
> James Almer:
>> Signed-off-by: James Almer <jamrial@gmail.com>
>> ---
>> libavcodec/decode.c | 55 +++++++++++++++++++++++++++++++++++++--------
>> libavcodec/decode.h | 2 +-
>> 2 files changed, 47 insertions(+), 10 deletions(-)
>>
>> diff --git a/libavcodec/decode.c b/libavcodec/decode.c
>> index 169ee79acd..1a431ba7d7 100644
>> --- a/libavcodec/decode.c
>> +++ b/libavcodec/decode.c
>> @@ -1409,25 +1409,31 @@ static int add_metadata_from_side_data(const AVPacket *avpkt, AVFrame *frame)
>> return av_packet_unpack_dictionary(side_metadata, size, frame_md);
>> }
>>
>> -int ff_decode_frame_props_from_pkt(const AVCodecContext *avctx,
>> +static const struct {
>> + enum AVPacketSideDataType packet;
>> + enum AVFrameSideDataType frame;
>> +} sd_global_map[] = {
>> + { AV_PKT_DATA_REPLAYGAIN , AV_FRAME_DATA_REPLAYGAIN },
>> + { AV_PKT_DATA_SPHERICAL, AV_FRAME_DATA_SPHERICAL },
>> + { AV_PKT_DATA_STEREO3D, AV_FRAME_DATA_STEREO3D },
>> + { AV_PKT_DATA_AUDIO_SERVICE_TYPE, AV_FRAME_DATA_AUDIO_SERVICE_TYPE },
>> + { AV_PKT_DATA_MASTERING_DISPLAY_METADATA, AV_FRAME_DATA_MASTERING_DISPLAY_METADATA },
>> + { AV_PKT_DATA_CONTENT_LIGHT_LEVEL, AV_FRAME_DATA_CONTENT_LIGHT_LEVEL },
>> + { AV_PKT_DATA_ICC_PROFILE, AV_FRAME_DATA_ICC_PROFILE },
>> + { AV_PKT_DATA_DYNAMIC_HDR10_PLUS, AV_FRAME_DATA_DYNAMIC_HDR_PLUS },
>> +};
>> +
>> +int ff_decode_frame_props_from_pkt(AVCodecContext *avctx,
>> AVFrame *frame, const AVPacket *pkt)
>> {
>> static const struct {
>> enum AVPacketSideDataType packet;
>> enum AVFrameSideDataType frame;
>> } sd[] = {
>> - { AV_PKT_DATA_REPLAYGAIN , AV_FRAME_DATA_REPLAYGAIN },
>> { AV_PKT_DATA_DISPLAYMATRIX, AV_FRAME_DATA_DISPLAYMATRIX },
>> - { AV_PKT_DATA_SPHERICAL, AV_FRAME_DATA_SPHERICAL },
>> - { AV_PKT_DATA_STEREO3D, AV_FRAME_DATA_STEREO3D },
>> - { AV_PKT_DATA_AUDIO_SERVICE_TYPE, AV_FRAME_DATA_AUDIO_SERVICE_TYPE },
>> - { AV_PKT_DATA_MASTERING_DISPLAY_METADATA, AV_FRAME_DATA_MASTERING_DISPLAY_METADATA },
>> - { AV_PKT_DATA_CONTENT_LIGHT_LEVEL, AV_FRAME_DATA_CONTENT_LIGHT_LEVEL },
>> { AV_PKT_DATA_A53_CC, AV_FRAME_DATA_A53_CC },
>> { AV_PKT_DATA_AFD, AV_FRAME_DATA_AFD },
>> - { AV_PKT_DATA_ICC_PROFILE, AV_FRAME_DATA_ICC_PROFILE },
>> { AV_PKT_DATA_S12M_TIMECODE, AV_FRAME_DATA_S12M_TIMECODE },
>> - { AV_PKT_DATA_DYNAMIC_HDR10_PLUS, AV_FRAME_DATA_DYNAMIC_HDR_PLUS },
>> { AV_PKT_DATA_SKIP_SAMPLES, AV_FRAME_DATA_SKIP_SAMPLES },
>> };
>>
>> @@ -1440,6 +1446,23 @@ FF_DISABLE_DEPRECATION_WARNINGS
>> FF_ENABLE_DEPRECATION_WARNINGS
>> #endif
>>
>> + for (int i = 0; i < FF_ARRAY_ELEMS(sd_global_map); i++) {
>> + size_t size;
>> + uint8_t *packet_sd = av_packet_get_side_data(pkt, sd_global_map[i].packet, &size);
>> + if (packet_sd) {
>> + AVFrameSideData *frame_sd;
>> +
>> + av_log(avctx, AV_LOG_DEBUG,
>> + "Overwriting global side data of type \"%s\" in output frame with side data "
>> + "from input packet.\n", av_packet_side_data_name(sd_global_map[i].packet));
>
> Overwriting? Is it guaranteed that the frame has the side data at all?
> And if it had, why would you overwrite it at all? After all, if the
> decoder added something, isn't this to be preferred to stuff from
> AVPacket side data?
I guess you're right, it probably shouldn't.
I'll remove the av_frame_remove_side_data() call bellow as well as the
debug log, so the behavior will remain the same in such scenarios as
it's right now (Packet side data will be added to the frame as a second
entry for the same type, which is normally never used as the _get()
helper wont reach it).
> Apart from that: packet_sd should be const.
Ok.
>
>> +
>> + av_frame_remove_side_data(frame, sd_global_map[i].frame);
>> + frame_sd = av_frame_new_side_data(frame, sd_global_map[i].frame, size);
>> + if (!frame_sd)
>> + return AVERROR(ENOMEM);
>> + memcpy(frame_sd->data, packet_sd, size);
>> + }
>> + }
>> for (int i = 0; i < FF_ARRAY_ELEMS(sd); i++) {
>> size_t size;
>> uint8_t *packet_sd = av_packet_get_side_data(pkt, sd[i].packet, &size);
>> @@ -1476,6 +1499,20 @@ int ff_decode_frame_props(AVCodecContext *avctx, AVFrame *frame)
>> const AVPacket *pkt = avctx->internal->last_pkt_props;
>> int ret;
>>
>> + for (int i = 0; i < FF_ARRAY_ELEMS(sd_global_map); i++) {
>> + const AVPacketSideData *packet_sd = ff_get_coded_side_data(avctx,
>> + sd_global_map[i].packet);
>> + if (packet_sd) {
>> + AVFrameSideData *frame_sd = av_frame_new_side_data(frame,
>> + sd_global_map[i].frame,
>> + packet_sd->size);
>> + if (!frame_sd)
>> + return AVERROR(ENOMEM);
>> +
>> + memcpy(frame_sd->data, packet_sd->data, packet_sd->size);
>> + }
>> + }
>> +
>> if (!(ffcodec(avctx->codec)->caps_internal & FF_CODEC_CAP_SETS_FRAME_PROPS)) {
>> ret = ff_decode_frame_props_from_pkt(avctx, frame, pkt);
>> if (ret < 0)
>> diff --git a/libavcodec/decode.h b/libavcodec/decode.h
>> index a52152e4a7..52e5aafc34 100644
>> --- a/libavcodec/decode.h
>> +++ b/libavcodec/decode.h
>> @@ -67,7 +67,7 @@ int ff_decode_get_packet(AVCodecContext *avctx, AVPacket *pkt);
>> /**
>> * Set various frame properties from the provided packet.
>> */
>> -int ff_decode_frame_props_from_pkt(const AVCodecContext *avctx,
>> +int ff_decode_frame_props_from_pkt(AVCodecContext *avctx,
>> AVFrame *frame, const AVPacket *pkt);
>>
>> /**
>
> _______________________________________________
> 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".
_______________________________________________
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] 28+ messages in thread
* [FFmpeg-devel] [PATCH 09/10] fftools/ffmpeg: stop injecting stream side data in packets
2023-09-06 17:44 [FFmpeg-devel] [PATCH 00/10 v4][RFC] AVCodecContext and AVCodecParameters side data James Almer
` (7 preceding siblings ...)
2023-09-06 17:44 ` [FFmpeg-devel] [PATCH 08/10] avcodec/decode: check for global side data " James Almer
@ 2023-09-06 17:44 ` James Almer
2023-09-06 17:44 ` [FFmpeg-devel] [PATCH 10/10] fftools/ffplay: " James Almer
` (2 subsequent siblings)
11 siblings, 0 replies; 28+ messages in thread
From: James Almer @ 2023-09-06 17:44 UTC (permalink / raw)
To: ffmpeg-devel
This is no longer needed as the side data is available for decoders in the
AVCodecContext.
The tests affected reflect the removal of useless CPB and Stereo 3D side
data in packets.
Signed-off-by: James Almer <jamrial@gmail.com>
---
fftools/ffmpeg_demux.c | 22 -------------------
tests/ref/fate/autorotate | 4 ++--
tests/ref/fate/copy-trac3074 | 2 +-
tests/ref/fate/matroska-avoid-negative-ts | 2 +-
tests/ref/fate/matroska-dovi-write-config7 | 2 +-
tests/ref/fate/matroska-dovi-write-config8 | 2 +-
tests/ref/fate/matroska-encoding-delay | 2 +-
.../fate/matroska-mastering-display-metadata | 4 ++--
tests/ref/fate/matroska-spherical-mono-remux | 4 ++--
tests/ref/fate/matroska-stereo_mode | 8 +++----
tests/ref/fate/matroska-vp8-alpha-remux | 2 +-
.../ref/fate/mov-mp4-disposition-mpegts-remux | 4 ++--
| 2 +-
tests/ref/fate/mxf-remux-applehdr10 | 2 +-
tests/ref/fate/vp9-superframe-bsf | 2 +-
15 files changed, 21 insertions(+), 43 deletions(-)
diff --git a/fftools/ffmpeg_demux.c b/fftools/ffmpeg_demux.c
index 6cf6d2e971..450438eaab 100644
--- a/fftools/ffmpeg_demux.c
+++ b/fftools/ffmpeg_demux.c
@@ -480,28 +480,6 @@ static int input_packet_process(Demuxer *d, DemuxMsg *msg, AVPacket *src)
ds->data_size += pkt->size;
ds->nb_packets++;
- /* add the stream-global side data to the first packet */
- if (ds->nb_packets == 1) {
- for (int i = 0; i < ist->st->codecpar->side_data.nb_sd; i++) {
- AVPacketSideData *src_sd = ist->st->codecpar->side_data.sd[i];
- uint8_t *dst_data;
-
- if (src_sd->type == AV_PKT_DATA_DISPLAYMATRIX)
- continue;
-
- if (av_packet_get_side_data(pkt, src_sd->type, NULL))
- continue;
-
- dst_data = av_packet_new_side_data(pkt, src_sd->type, src_sd->size);
- if (!dst_data) {
- ret = AVERROR(ENOMEM);
- goto fail;
- }
-
- memcpy(dst_data, src_sd->data, src_sd->size);
- }
- }
-
if (debug_ts) {
av_log(NULL, AV_LOG_INFO, "demuxer+ffmpeg -> ist_index:%d:%d type:%s pkt_pts:%s pkt_pts_time:%s pkt_dts:%s pkt_dts_time:%s duration:%s duration_time:%s off:%s off_time:%s\n",
f->index, pkt->stream_index,
diff --git a/tests/ref/fate/autorotate b/tests/ref/fate/autorotate
index dff628bbff..2aa29fafa7 100644
--- a/tests/ref/fate/autorotate
+++ b/tests/ref/fate/autorotate
@@ -11,8 +11,8 @@
#codec_id 1: ac3
#sample_rate 1: 44100
#channel_layout_name 1: mono
-0, -512, 0, 512, 6997, 0x55c700f6, S=1, 40
-1, -256, -256, 1536, 416, 0x92ddc529, S=2, 10, 4
+0, -512, 0, 512, 6997, 0x55c700f6
+1, -256, -256, 1536, 416, 0x92ddc529, S=1, 10
0, 0, 512, 512, 4847, 0xe74f522e, F=0x0
1, 1280, 1280, 1536, 418, 0x0a7fcd2d
0, 512, 1024, 512, 5281, 0xbd4a5dac, F=0x0
diff --git a/tests/ref/fate/copy-trac3074 b/tests/ref/fate/copy-trac3074
index b6d23f8c1c..53ba27e920 100644
--- a/tests/ref/fate/copy-trac3074
+++ b/tests/ref/fate/copy-trac3074
@@ -5,7 +5,7 @@
#codec_id 0: eac3
#sample_rate 0: 48000
#channel_layout_name 0: stereo
-0, 0, 0, 1536, 512, 0x2beaf79f, S=1, 4
+0, 0, 0, 1536, 512, 0x2beaf79f
0, 1536, 1536, 1536, 512, 0x29ddf9d6
0, 3072, 3072, 1536, 512, 0xba0afa79
0, 4608, 4608, 1536, 512, 0xe019f394
diff --git a/tests/ref/fate/matroska-avoid-negative-ts b/tests/ref/fate/matroska-avoid-negative-ts
index 05821cbf09..3ba289c6e6 100644
--- a/tests/ref/fate/matroska-avoid-negative-ts
+++ b/tests/ref/fate/matroska-avoid-negative-ts
@@ -11,7 +11,7 @@ dede1d72a28c7eb0a849acf230b08247 *tests/data/fate/matroska-avoid-negative-ts.mat
#codec_id 1: mp3
#sample_rate 1: 44100
#channel_layout_name 1: mono
-0, -37, 43, 40, 9156, 0xe5bd034a, S=1, 40
+0, -37, 43, 40, 9156, 0xe5bd034a
1, 0, 0, 26, 417, 0x7198c15e
0, 3, 3, 40, 1740, 0x29ac4480, F=0x0
1, 26, 26, 26, 417, 0x3c67c32d
diff --git a/tests/ref/fate/matroska-dovi-write-config7 b/tests/ref/fate/matroska-dovi-write-config7
index aaeeb34751..dc5b73a44e 100644
--- a/tests/ref/fate/matroska-dovi-write-config7
+++ b/tests/ref/fate/matroska-dovi-write-config7
@@ -13,7 +13,7 @@
#dimensions 1: 1920x1080
#sar 1: 0/1
0, -83, 0, 41, 699, 0x728548f1
-1, -83, 0, 41, 1085, 0xfb2dba82, S=1, 8
+1, -83, 0, 41, 1085, 0xfb2dba82
0, -42, 167, 41, 95, 0xc0312044, F=0x0
1, -42, 167, 41, 481, 0xf23f91d5, F=0x0
0, 0, 83, 41, 99, 0x5e0a2221, F=0x0
diff --git a/tests/ref/fate/matroska-dovi-write-config8 b/tests/ref/fate/matroska-dovi-write-config8
index 55fe191047..472cbed708 100644
--- a/tests/ref/fate/matroska-dovi-write-config8
+++ b/tests/ref/fate/matroska-dovi-write-config8
@@ -12,7 +12,7 @@
#codec_id 1: aac
#sample_rate 1: 44100
#channel_layout_name 1: stereo
-0, -67, 0, 33, 63375, 0xc76606ab, S=1, 8
+0, -67, 0, 33, 63375, 0xc76606ab
0, -34, 133, 33, 46706, 0x0e08a7e5, F=0x0
0, 0, 67, 33, 29766, 0x753c031a, F=0x0
1, 0, 0, 23, 6, 0x031e0108
diff --git a/tests/ref/fate/matroska-encoding-delay b/tests/ref/fate/matroska-encoding-delay
index d2ff2d07be..ee1989cbcb 100644
--- a/tests/ref/fate/matroska-encoding-delay
+++ b/tests/ref/fate/matroska-encoding-delay
@@ -12,7 +12,7 @@
#sample_rate 1: 48000
#channel_layout_name 1: stereo
1, -10, -10, 24, 1152, 0x724077b8
-0, 0, 0, 40, 237628, 0xeff25579, S=1, 40
+0, 0, 0, 40, 237628, 0xeff25579
1, 14, 14, 24, 1152, 0x80625572
1, 38, 38, 24, 1152, 0x7d7f4dce
0, 40, 40, 40, 238066, 0xb2265f41
diff --git a/tests/ref/fate/matroska-mastering-display-metadata b/tests/ref/fate/matroska-mastering-display-metadata
index 3726469213..53f84c1793 100644
--- a/tests/ref/fate/matroska-mastering-display-metadata
+++ b/tests/ref/fate/matroska-mastering-display-metadata
@@ -22,9 +22,9 @@
#codec_id 3: ffv1
#dimensions 3: 1280x720
#sar 3: 1/1
-0, 0, 0, 16, 57008, 0x43416399, S=2, 8, 88
+0, 0, 0, 16, 57008, 0x43416399
1, 0, 0, 16, 2403, 0xaa818522
-3, 0, 0, 16, 274117, 0xc439610f, S=2, 8, 88
+3, 0, 0, 16, 274117, 0xc439610f
0, 17, 17, 16, 57248, 0xa06cd7b5
1, 17, 17, 16, 2403, 0xe1a991e5
2, 17, 17, 16, 1602, 0x5d868171
diff --git a/tests/ref/fate/matroska-spherical-mono-remux b/tests/ref/fate/matroska-spherical-mono-remux
index e9904b2c92..6fcda14822 100644
--- a/tests/ref/fate/matroska-spherical-mono-remux
+++ b/tests/ref/fate/matroska-spherical-mono-remux
@@ -12,8 +12,8 @@ fddfea5f05a7a9a0d187df9a72900055 *tests/data/fate/matroska-spherical-mono-remux.
#codec_id 1: h264
#dimensions 1: 1920x1080
#sar 1: 0/1
-0, -80, 0, 40, 69118, 0x73cb52f0, S=2, 12, 36
-1, -80, 0, 40, 69118, 0x73cb52f0, S=2, 12, 36
+0, -80, 0, 40, 69118, 0x73cb52f0
+1, -80, 0, 40, 69118, 0x73cb52f0
0, -40, 160, 40, 1103, 0x082a059f, F=0x0
1, -40, 160, 40, 1103, 0x082a059f, F=0x0
[STREAM]
diff --git a/tests/ref/fate/matroska-stereo_mode b/tests/ref/fate/matroska-stereo_mode
index 5c36a6d666..739b789fea 100644
--- a/tests/ref/fate/matroska-stereo_mode
+++ b/tests/ref/fate/matroska-stereo_mode
@@ -43,10 +43,10 @@ a7a220a77001e81685ec807ce5ac3bc6 *tests/data/fate/matroska-stereo_mode.matroska
#dimensions 6: 512x512
#sar 6: 2/1
0, 0, 0, 1000, 206173, 0x95af7455
-1, 0, 0, 1000, 206173, 0x95af7455, S=1, 12
-2, 0, 0, 1000, 206173, 0x95af7455, S=1, 12
-3, 0, 0, 1000, 206173, 0x95af7455, S=1, 12
-4, 0, 0, 1000, 206173, 0x95af7455, S=1, 12
+1, 0, 0, 1000, 206173, 0x95af7455
+2, 0, 0, 1000, 206173, 0x95af7455
+3, 0, 0, 1000, 206173, 0x95af7455
+4, 0, 0, 1000, 206173, 0x95af7455
5, 0, 0, 1000, 206173, 0x95af7455
6, 0, 0, 1000, 206173, 0x95af7455
0, 1000, 1000, 1000, 36, 0x34891010, F=0x0
diff --git a/tests/ref/fate/matroska-vp8-alpha-remux b/tests/ref/fate/matroska-vp8-alpha-remux
index eba3ffb77a..8117325433 100644
--- a/tests/ref/fate/matroska-vp8-alpha-remux
+++ b/tests/ref/fate/matroska-vp8-alpha-remux
@@ -5,7 +5,7 @@
#codec_id 0: vp8
#dimensions 0: 320x213
#sar 0: 1/1
-0, 0, 0, 33, 2108, 0x59b92a34, S=2, 1900, 12
+0, 0, 0, 33, 2108, 0x59b92a34, S=1, 1900
0, 32, 32, 33, 142, 0x2f2a3fed, F=0x0, S=1, 160
0, 65, 65, 33, 157, 0x17804767, F=0x0, S=1, 209
0, 99, 99, 33, 206, 0x537262ca, F=0x0, S=1, 317
diff --git a/tests/ref/fate/mov-mp4-disposition-mpegts-remux b/tests/ref/fate/mov-mp4-disposition-mpegts-remux
index efef043074..ba419843dc 100644
--- a/tests/ref/fate/mov-mp4-disposition-mpegts-remux
+++ b/tests/ref/fate/mov-mp4-disposition-mpegts-remux
@@ -10,9 +10,9 @@ adb3b95c07a5f3e0c86641dd62f01dae *tests/data/fate/mov-mp4-disposition-mpegts-rem
#codec_id 1: ac3
#sample_rate 1: 48000
#channel_layout_name 1: stereo
-1, 0, 0, 1536, 768, 0xa63778d4, S=1, 4
+1, 0, 0, 1536, 768, 0xa63778d4
1, 1536, 1536, 1536, 768, 0x7d577f3f
-0, 3072, 3072, 1536, 768, 0xc2867884, S=1, 4
+0, 3072, 3072, 1536, 768, 0xc2867884
1, 3072, 3072, 1536, 768, 0xd86b7c8f
0, 4608, 4608, 1536, 690, 0xa2714bf3
1, 4608, 4608, 1536, 626, 0x09f4382f
--git a/tests/ref/fate/mxf-d10-user-comments b/tests/ref/fate/mxf-d10-user-comments
index 1b59beec7c..ccfdc83f11 100644
--- a/tests/ref/fate/mxf-d10-user-comments
+++ b/tests/ref/fate/mxf-d10-user-comments
@@ -6,7 +6,7 @@
#codec_id 0: mpeg2video
#dimensions 0: 1280x720
#sar 0: 3/4
-0, -1, 0, 1, 150000, 0x0547870d, S=1, 40
+0, -1, 0, 1, 150000, 0x0547870d
0, 0, 1, 1, 150000, 0xe80a1612, F=0x0
0, 1, 2, 1, 150000, 0xc5c50e2f, F=0x0
0, 2, 3, 1, 150000, 0x51e28a04, F=0x0
diff --git a/tests/ref/fate/mxf-remux-applehdr10 b/tests/ref/fate/mxf-remux-applehdr10
index 29e0e03a72..9fbf8b60c7 100644
--- a/tests/ref/fate/mxf-remux-applehdr10
+++ b/tests/ref/fate/mxf-remux-applehdr10
@@ -10,7 +10,7 @@
#codec_id 1: pcm_s24le
#sample_rate 1: 48000
#channel_layout_name 1: mono
-0, 0, 0, 1, 57008, 0x43416399, S=1, 88
+0, 0, 0, 1, 57008, 0x43416399
1, 0, 0, 801, 2403, 0x00000000
0, 1, 1, 1, 57248, 0xa06cd7b5
1, 801, 801, 801, 2403, 0x00000000
diff --git a/tests/ref/fate/vp9-superframe-bsf b/tests/ref/fate/vp9-superframe-bsf
index d7985c6973..485644dfb2 100644
--- a/tests/ref/fate/vp9-superframe-bsf
+++ b/tests/ref/fate/vp9-superframe-bsf
@@ -3,7 +3,7 @@
#codec_id 0: vp9
#dimensions 0: 352x288
#sar 0: 1/1
-0, 0, 0, 33, 6958, 0x38e58ee6, S=1, 12
+0, 0, 0, 33, 6958, 0x38e58ee6
0, 33, 33, 33, 852, 0x3edf9ed0, F=0x0
0, 66, 66, 33, 27, 0x62d007e5, F=0x0
0, 100, 100, 33, 25, 0x51980749, F=0x0
--
2.42.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] 28+ messages in thread* [FFmpeg-devel] [PATCH 10/10] fftools/ffplay: stop injecting stream side data in packets
2023-09-06 17:44 [FFmpeg-devel] [PATCH 00/10 v4][RFC] AVCodecContext and AVCodecParameters side data James Almer
` (8 preceding siblings ...)
2023-09-06 17:44 ` [FFmpeg-devel] [PATCH 09/10] fftools/ffmpeg: stop injecting stream side data in packets James Almer
@ 2023-09-06 17:44 ` James Almer
2023-09-09 12:42 ` [FFmpeg-devel] [PATCH 00/10 v4][RFC] AVCodecContext and AVCodecParameters side data James Almer
2023-09-14 15:43 ` Anton Khirnov
11 siblings, 0 replies; 28+ messages in thread
From: James Almer @ 2023-09-06 17:44 UTC (permalink / raw)
To: ffmpeg-devel
This is no longer needed as the side data is available for decoders in the
AVCodecContext.
Signed-off-by: James Almer <jamrial@gmail.com>
---
fftools/ffplay.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/fftools/ffplay.c b/fftools/ffplay.c
index a0d5811c94..b489e6264a 100644
--- a/fftools/ffplay.c
+++ b/fftools/ffplay.c
@@ -2793,8 +2793,6 @@ static int read_thread(void *arg)
if (genpts)
ic->flags |= AVFMT_FLAG_GENPTS;
- av_format_inject_global_side_data(ic);
-
if (find_stream_info) {
AVDictionary **opts;
int orig_nb_streams = ic->nb_streams;
--
2.42.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] 28+ messages in thread* Re: [FFmpeg-devel] [PATCH 00/10 v4][RFC] AVCodecContext and AVCodecParameters side data
2023-09-06 17:44 [FFmpeg-devel] [PATCH 00/10 v4][RFC] AVCodecContext and AVCodecParameters side data James Almer
` (9 preceding siblings ...)
2023-09-06 17:44 ` [FFmpeg-devel] [PATCH 10/10] fftools/ffplay: " James Almer
@ 2023-09-09 12:42 ` James Almer
2023-09-14 15:43 ` Anton Khirnov
11 siblings, 0 replies; 28+ messages in thread
From: James Almer @ 2023-09-09 12:42 UTC (permalink / raw)
To: ffmpeg-devel
On 9/6/2023 2:44 PM, James Almer wrote:
> Changes since the previous version:
> - Removed the AVPacketSideDataSet from AVCodecContext, using instead the
> existing coded_side_data field, at Anton's request.
>
> I still prefer using a new field of type AVPacketSideDataSet, given that the
> user can currently only fill coded_side_data manually (See the implementation
> in the codecpar copy functions, which non lavf users would need to replicate),
> and more so considering coded_side_data is a field pretty much nothing but lavf
> uses.
>
> Opinions are very welcome and needed.
Ping.
_______________________________________________
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] 28+ messages in thread* Re: [FFmpeg-devel] [PATCH 00/10 v4][RFC] AVCodecContext and AVCodecParameters side data
2023-09-06 17:44 [FFmpeg-devel] [PATCH 00/10 v4][RFC] AVCodecContext and AVCodecParameters side data James Almer
` (10 preceding siblings ...)
2023-09-09 12:42 ` [FFmpeg-devel] [PATCH 00/10 v4][RFC] AVCodecContext and AVCodecParameters side data James Almer
@ 2023-09-14 15:43 ` Anton Khirnov
2023-09-14 16:40 ` James Almer
11 siblings, 1 reply; 28+ messages in thread
From: Anton Khirnov @ 2023-09-14 15:43 UTC (permalink / raw)
To: FFmpeg development discussions and patches
Quoting James Almer (2023-09-06 19:44:20)
> Changes since the previous version:
> - Removed the AVPacketSideDataSet from AVCodecContext, using instead the
> existing coded_side_data field, at Anton's request.
>
> I still prefer using a new field of type AVPacketSideDataSet, given that the
> user can currently only fill coded_side_data manually (See the implementation
> in the codecpar copy functions, which non lavf users would need to replicate),
> and more so considering coded_side_data is a field pretty much nothing but lavf
> uses.
>
> Opinions are very welcome and needed.
To provide more context on the issue:
AVPackets can have side data attached to them, in the form of
AVPacket.{side_data,side_data_elems} array+count.
Some of this side data can occasionally be stored in global headers,
e.g. in containers or extradata. We have some limited support for this:
* AVStream.{side_data,nb_side_data} array+count allows demuxers to
export this to user, and muxers to accept it from the user
* AVCodecContext.{coded_side_data,nb_coded_side_data} allows decoders to
accept it from the user (in principle, in practice it is not used),
and encoders to export it to the user (this is used, but not very
much)
The broad idea for this set is adding stream-global "coded/packet" side
data to AVCodecParameters, so that it can be naturally communicated from
demuxers to bitstream filters to decoders, and from encoders to
bitstream filters to muxers. Since AVStream already contains an
AVCodecParameters instance, the abovementioned AVStream.side_data
becomes redundant and is deprecated, the muxer/demuxer stream-global
side data would be passed through AVStream.codecpar.
The original version proposed by James adds a new struct, that bundles
the side data array+count, and a set of helpers operating on this
struct. Then this new struct is added to AVCodecParameters and
AVCodecContext, which replaces the abovementioned AVStream and
AVCodecContext side data array+count. Notably AVPacket is left as-is,
because its side data is widely used and replacing array+count by this
new struct would be a very intrusive break for little gain.
My objections to this approach are
* coded side data is now stored differently in AVPacket and in
everything else
* the case for replacing AVCodecContext.coded_side_data does not seem
sufficiently strong to me
My alternative suggestion was:
* avoid adding a new struct (which merely bundles array+count)
* add a set of helpers that accept array+count as parameters and operate
on them
* use these helpers for all operations on side data across AVPacket,
AVCodecContext, and AVCodecParameters
I have a moderately strong preference for this approach, but James
disagrees. More opinions are very much welcome.
--
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] 28+ messages in thread* Re: [FFmpeg-devel] [PATCH 00/10 v4][RFC] AVCodecContext and AVCodecParameters side data
2023-09-14 15:43 ` Anton Khirnov
@ 2023-09-14 16:40 ` James Almer
2023-09-25 19:54 ` James Almer
0 siblings, 1 reply; 28+ messages in thread
From: James Almer @ 2023-09-14 16:40 UTC (permalink / raw)
To: ffmpeg-devel
On 9/14/2023 12:43 PM, Anton Khirnov wrote:
> Quoting James Almer (2023-09-06 19:44:20)
>> Changes since the previous version:
>> - Removed the AVPacketSideDataSet from AVCodecContext, using instead the
>> existing coded_side_data field, at Anton's request.
>>
>> I still prefer using a new field of type AVPacketSideDataSet, given that the
>> user can currently only fill coded_side_data manually (See the implementation
>> in the codecpar copy functions, which non lavf users would need to replicate),
>> and more so considering coded_side_data is a field pretty much nothing but lavf
>> uses.
>>
>> Opinions are very welcome and needed.
>
> To provide more context on the issue:
>
> AVPackets can have side data attached to them, in the form of
> AVPacket.{side_data,side_data_elems} array+count.
>
> Some of this side data can occasionally be stored in global headers,
> e.g. in containers or extradata. We have some limited support for this:
> * AVStream.{side_data,nb_side_data} array+count allows demuxers to
> export this to user, and muxers to accept it from the user
> * AVCodecContext.{coded_side_data,nb_coded_side_data} allows decoders to
> accept it from the user (in principle, in practice it is not used),
> and encoders to export it to the user (this is used, but not very
> much)
>
> The broad idea for this set is adding stream-global "coded/packet" side
> data to AVCodecParameters, so that it can be naturally communicated from
> demuxers to bitstream filters to decoders, and from encoders to
> bitstream filters to muxers. Since AVStream already contains an
> AVCodecParameters instance, the abovementioned AVStream.side_data
> becomes redundant and is deprecated, the muxer/demuxer stream-global
> side data would be passed through AVStream.codecpar.
>
> The original version proposed by James adds a new struct, that bundles
> the side data array+count, and a set of helpers operating on this
> struct. Then this new struct is added to AVCodecParameters and
> AVCodecContext, which replaces the abovementioned AVStream and
> AVCodecContext side data array+count. Notably AVPacket is left as-is,
> because its side data is widely used and replacing array+count by this
> new struct would be a very intrusive break for little gain.
In the future, packet side data could be made ref counted. For this,
we'd need to add a new AVBufferRef field to AVPacketSideData, which is
currently tied to the ABI.
Ideally, if this happens, sizeof(AVPacketSideData) would be made not a
part of the ABI, putting it in line with the AVFrame counterpart. Doing
this is pretty much not an option for the existing array+count fields in
AVPacket and AVCodecContext, because everyone, from external API users
to our own API users like the CLI last i checked, loop through it
manually as nothing prevents them to. But if a new field of the new set
struct is added as replacement (a struct where the AVPacketSideData
field is an array of pointers, and whose helpers can be defined as "must
use to handle this struct"), then this would not be an issue.
I probably said it several times, including in the above paragraph, but
one of my objectives is trying to sync frame and packet side data
structs, fields and helpers, since right now both are considerably
different.
Jan has a patchset also introducing a side data set for frames, so this
is the time to try and align both as much as possible (which i already
talked to him about) and laying the ground for future changes.
>
> My objections to this approach are
> * coded side data is now stored differently in AVPacket and in
> everything else
> * the case for replacing AVCodecContext.coded_side_data does not seem
> sufficiently strong to me
It's used only by a handful of encoders to export CPB side data, which
probably only lavf looks at, and by no decoder at all. It's a field that
would be completely painless to replace.
>
> My alternative suggestion was:
> * avoid adding a new struct (which merely bundles array+count)
> * add a set of helpers that accept array+count as parameters and operate
> on them
See above my comment about sizeof(AVPacketSideData) and users accessing
the array it directly.
> * use these helpers for all operations on side data across AVPacket,
> AVCodecContext, and AVCodecParameters
>
> I have a moderately strong preference for this approach, but James
> disagrees. More opinions are very much welcome.
>
_______________________________________________
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] 28+ messages in thread* Re: [FFmpeg-devel] [PATCH 00/10 v4][RFC] AVCodecContext and AVCodecParameters side data
2023-09-14 16:40 ` James Almer
@ 2023-09-25 19:54 ` James Almer
2023-09-25 20:02 ` Paul B Mahol
0 siblings, 1 reply; 28+ messages in thread
From: James Almer @ 2023-09-25 19:54 UTC (permalink / raw)
To: ffmpeg-devel
On 9/14/2023 1:40 PM, James Almer wrote:
> On 9/14/2023 12:43 PM, Anton Khirnov wrote:
>> Quoting James Almer (2023-09-06 19:44:20)
>>> Changes since the previous version:
>>> - Removed the AVPacketSideDataSet from AVCodecContext, using instead the
>>> existing coded_side_data field, at Anton's request.
>>>
>>> I still prefer using a new field of type AVPacketSideDataSet, given
>>> that the
>>> user can currently only fill coded_side_data manually (See the
>>> implementation
>>> in the codecpar copy functions, which non lavf users would need to
>>> replicate),
>>> and more so considering coded_side_data is a field pretty much
>>> nothing but lavf
>>> uses.
>>>
>>> Opinions are very welcome and needed.
>>
>> To provide more context on the issue:
>>
>> AVPackets can have side data attached to them, in the form of
>> AVPacket.{side_data,side_data_elems} array+count.
>>
>> Some of this side data can occasionally be stored in global headers,
>> e.g. in containers or extradata. We have some limited support for this:
>> * AVStream.{side_data,nb_side_data} array+count allows demuxers to
>> export this to user, and muxers to accept it from the user
>> * AVCodecContext.{coded_side_data,nb_coded_side_data} allows decoders to
>> accept it from the user (in principle, in practice it is not used),
>> and encoders to export it to the user (this is used, but not very
>> much)
>>
>> The broad idea for this set is adding stream-global "coded/packet" side
>> data to AVCodecParameters, so that it can be naturally communicated from
>> demuxers to bitstream filters to decoders, and from encoders to
>> bitstream filters to muxers. Since AVStream already contains an
>> AVCodecParameters instance, the abovementioned AVStream.side_data
>> becomes redundant and is deprecated, the muxer/demuxer stream-global
>> side data would be passed through AVStream.codecpar.
>>
>> The original version proposed by James adds a new struct, that bundles
>> the side data array+count, and a set of helpers operating on this
>> struct. Then this new struct is added to AVCodecParameters and
>> AVCodecContext, which replaces the abovementioned AVStream and
>> AVCodecContext side data array+count. Notably AVPacket is left as-is,
>> because its side data is widely used and replacing array+count by this
>> new struct would be a very intrusive break for little gain.
>
> In the future, packet side data could be made ref counted. For this,
> we'd need to add a new AVBufferRef field to AVPacketSideData, which is
> currently tied to the ABI.
> Ideally, if this happens, sizeof(AVPacketSideData) would be made not a
> part of the ABI, putting it in line with the AVFrame counterpart. Doing
> this is pretty much not an option for the existing array+count fields in
> AVPacket and AVCodecContext, because everyone, from external API users
> to our own API users like the CLI last i checked, loop through it
> manually as nothing prevents them to. But if a new field of the new set
> struct is added as replacement (a struct where the AVPacketSideData
> field is an array of pointers, and whose helpers can be defined as "must
> use to handle this struct"), then this would not be an issue.
>
> I probably said it several times, including in the above paragraph, but
> one of my objectives is trying to sync frame and packet side data
> structs, fields and helpers, since right now both are considerably
> different.
> Jan has a patchset also introducing a side data set for frames, so this
> is the time to try and align both as much as possible (which i already
> talked to him about) and laying the ground for future changes.
>
>>
>> My objections to this approach are
>> * coded side data is now stored differently in AVPacket and in
>> everything else
>> * the case for replacing AVCodecContext.coded_side_data does not seem
>> sufficiently strong to me
>
> It's used only by a handful of encoders to export CPB side data, which
> probably only lavf looks at, and by no decoder at all. It's a field that
> would be completely painless to replace.
>
>>
>> My alternative suggestion was:
>> * avoid adding a new struct (which merely bundles array+count)
>> * add a set of helpers that accept array+count as parameters and operate
>> on them
>
> See above my comment about sizeof(AVPacketSideData) and users accessing
> the array it directly.
>
>> * use these helpers for all operations on side data across AVPacket,
>> AVCodecContext, and AVCodecParameters
>>
>> I have a moderately strong preference for this approach, but James
>> disagrees. More opinions are very much welcome.
Ping. We really need opinions on what approach to use.
Here's some visual representation of what is being discussed.
Currently, things look like this:
#########
Side data definition
-----
struct AVPacketSideData {
uint8_t *data;
size_t size;
enum AVPacketSideDataType type;
};
-----
Users
-----
struct AVCodecContext {
[...]
}
struct AVCodecParameters {
[...]
}
struct AVPacket {
[...]
AVPacketSideData *side_data;
int side_data_elems;
[...]
}
uint8_t* av_packet_new_side_data(AVPacket *pkt, enum
AVPacketSideDataType type,
size_t size);
int av_packet_add_side_data(AVPacket *pkt,
enum AVPacketSideDataType type,
uint8_t *data, size_t size);
uint8_t* av_packet_get_side_data(const AVPacket *pkt,
enum AVPacketSideDataType type,
size_t *size);
struct AVStream {
[...]
AVCodecParameters *codec_par;
AVPacketSideData *side_data;
int nb_side_data;
[...]
};
uint8_t* av_stream_new_side_data(AVStream *stream,
enum AVPacketSideDataType type,
size_t size);
int av_stream_add_side_data(AVStream *st,
enum AVPacketSideDataType type,
uint8_t *data, size_t size);
uint8_t* av_stream_get_side_data(const AVStream *stream,
enum AVPacketSideDataType type,
size_t *size);
-----
#########
The only way for global side data exported by demuxers to hit decoders
or bsfs is by injecting it into the first output packet. This means it's
not available at init().
The solution is obviously being able to store the side data in the
decoder context, which the caller would copy from the demuxer context.
My suggestion is to introduce a new struct to hold a set of side data,
and helpers that work with it, whereas Anton's suggestion is to just
storing the pointer to side data array and side data array size directly
without wrapping them.
My suggestion would have things look like this:
#########
Side data definition
-----
struct AVPacketSideData {
uint8_t *data;
size_t size;
enum AVPacketSideDataType type;
};
struct AVPacketSideDataSet {
AVPacketSideData **sd;
int nb_sd;
};
AVPacketSideData *av_packet_side_data_set_new(AVPacketSideDataSet *set,
enum AVPacketSideDataType type,
size_t size);
AVPacketSideData *av_packet_side_data_set_add(AVPacketSideDataSet *set,
enum AVPacketSideDataType type,
uint8_t *data, size_t size);
AVPacketSideData *av_packet_side_data_set_get(AVPacketSideDataSet *set,
enum AVPacketSideDataType type);
void av_packet_side_data_set_remove(AVPacketSideDataSet *set,
enum AVPacketSideDataType type);
void av_packet_side_data_set_uninit(AVPacketSideDataSet *set);
-----
Users
-----
struct AVCodecContext {
[...]
AVPacketSideDataSet *side_data;
[...]
}
struct AVCodecParameters {
[...]
AVPacketSideDataSet *side_data;
[...]
}
AVPacket and its helpers untouched.
struct AVStream {
[...]
AVCodecParameters *codec_par;
[...]
};
-----
In which you'd do for example
av_packet_side_data_set_new(avctx->side_data,
AV_PKT_DATA_DISPLAYMATRIX,
sizeof(int32_t) * 9);
av_packet_side_data_set_new(st->codec_par->side_data,
AV_PKT_DATA_DISPLAYMATRIX,
sizeof(int32_t) * 9);
av_packet_side_data_set_get(avctx->side_data,
AV_PKT_DATA_DISPLAYMATRIX);
av_packet_side_data_set_get(st->codec_par->side_data,
AV_PKT_DATA_DISPLAYMATRIX);
#########
Whereas Anton's would have things look like this:
#########
Side data definition
-----
struct AVPacketSideData {
uint8_t *data;
size_t size;
enum AVPacketSideDataType type;
};
AVPacketSideData *av_packet_side_data_set_new(
AVPacketSideData **sd, // INOUT
size_t *sd_size, // INOUT
enum AVPacketSideDataType type,
size_t size);
AVPacketSideData *av_packet_side_data_set_add(
AVPacketSideData **sd, // INOUT
size_t *sd_size, // INOUT
enum AVPacketSideDataType type,
uint8_t *data, size_t size);
AVPacketSideData *av_packet_side_data_set_get(AVPacketSideData *sd,
size_t sd_size,
enum AVPacketSideDataType type);
void av_packet_side_data_set_remove(AVPacketSideData *sd,
size_t *sd_size,
enum AVPacketSideDataType type);
void av_packet_side_data_set_uninit(AVPacketSideData *sd);
-----
Users
-----
struct AVCodecContext {
[...]
AVPacketSideData *coded_side_data;
int nb_coded_side_data;
[...]
}
struct AVCodecParameters {
[...]
AVPacketSideData *side_data;
int nb_side_data;
[...]
}
AVPacket and its helpers untouched.
struct AVStream {
[...]
AVCodecParameters *codec_par;
[...]
};
-----
In which you'd do for example
av_packet_side_data_set_new(&avctx->coded_side_data,
&avctx->nb_coded_side_data,
AV_PKT_DATA_DISPLAYMATRIX,
av_packet_side_data_set_new(&st->codec_par->side_data,
&st->codec_par->nb_side_data,
AV_PKT_DATA_DISPLAYMATRIX,
sizeof(int32_t) * 9);
av_packet_side_data_set_get(avctx->coded_side_data,
avctx->nb_coded_side_data,
AV_PKT_DATA_DISPLAYMATRIX);
av_packet_side_data_set_get(st->codec_par->side_data,
st->codec_par->nb_side_data,
AV_PKT_DATA_DISPLAYMATRIX);
#########
Does anyone have a preference?
_______________________________________________
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] 28+ messages in thread* Re: [FFmpeg-devel] [PATCH 00/10 v4][RFC] AVCodecContext and AVCodecParameters side data
2023-09-25 19:54 ` James Almer
@ 2023-09-25 20:02 ` Paul B Mahol
2023-09-25 20:18 ` James Almer
0 siblings, 1 reply; 28+ messages in thread
From: Paul B Mahol @ 2023-09-25 20:02 UTC (permalink / raw)
To: FFmpeg development discussions and patches
On 9/25/23, James Almer <jamrial@gmail.com> wrote:
> On 9/14/2023 1:40 PM, James Almer wrote:
>> On 9/14/2023 12:43 PM, Anton Khirnov wrote:
>>> Quoting James Almer (2023-09-06 19:44:20)
>>>> Changes since the previous version:
>>>> - Removed the AVPacketSideDataSet from AVCodecContext, using instead the
>>>> existing coded_side_data field, at Anton's request.
>>>>
>>>> I still prefer using a new field of type AVPacketSideDataSet, given
>>>> that the
>>>> user can currently only fill coded_side_data manually (See the
>>>> implementation
>>>> in the codecpar copy functions, which non lavf users would need to
>>>> replicate),
>>>> and more so considering coded_side_data is a field pretty much
>>>> nothing but lavf
>>>> uses.
>>>>
>>>> Opinions are very welcome and needed.
>>>
>>> To provide more context on the issue:
>>>
>>> AVPackets can have side data attached to them, in the form of
>>> AVPacket.{side_data,side_data_elems} array+count.
>>>
>>> Some of this side data can occasionally be stored in global headers,
>>> e.g. in containers or extradata. We have some limited support for this:
>>> * AVStream.{side_data,nb_side_data} array+count allows demuxers to
>>> export this to user, and muxers to accept it from the user
>>> * AVCodecContext.{coded_side_data,nb_coded_side_data} allows decoders to
>>> accept it from the user (in principle, in practice it is not used),
>>> and encoders to export it to the user (this is used, but not very
>>> much)
>>>
>>> The broad idea for this set is adding stream-global "coded/packet" side
>>> data to AVCodecParameters, so that it can be naturally communicated from
>>> demuxers to bitstream filters to decoders, and from encoders to
>>> bitstream filters to muxers. Since AVStream already contains an
>>> AVCodecParameters instance, the abovementioned AVStream.side_data
>>> becomes redundant and is deprecated, the muxer/demuxer stream-global
>>> side data would be passed through AVStream.codecpar.
>>>
>>> The original version proposed by James adds a new struct, that bundles
>>> the side data array+count, and a set of helpers operating on this
>>> struct. Then this new struct is added to AVCodecParameters and
>>> AVCodecContext, which replaces the abovementioned AVStream and
>>> AVCodecContext side data array+count. Notably AVPacket is left as-is,
>>> because its side data is widely used and replacing array+count by this
>>> new struct would be a very intrusive break for little gain.
>>
>> In the future, packet side data could be made ref counted. For this,
>> we'd need to add a new AVBufferRef field to AVPacketSideData, which is
>> currently tied to the ABI.
>> Ideally, if this happens, sizeof(AVPacketSideData) would be made not a
>> part of the ABI, putting it in line with the AVFrame counterpart. Doing
>> this is pretty much not an option for the existing array+count fields in
>> AVPacket and AVCodecContext, because everyone, from external API users
>> to our own API users like the CLI last i checked, loop through it
>> manually as nothing prevents them to. But if a new field of the new set
>> struct is added as replacement (a struct where the AVPacketSideData
>> field is an array of pointers, and whose helpers can be defined as "must
>> use to handle this struct"), then this would not be an issue.
>>
>> I probably said it several times, including in the above paragraph, but
>> one of my objectives is trying to sync frame and packet side data
>> structs, fields and helpers, since right now both are considerably
>> different.
>> Jan has a patchset also introducing a side data set for frames, so this
>> is the time to try and align both as much as possible (which i already
>> talked to him about) and laying the ground for future changes.
>>
>>>
>>> My objections to this approach are
>>> * coded side data is now stored differently in AVPacket and in
>>> everything else
>>> * the case for replacing AVCodecContext.coded_side_data does not seem
>>> sufficiently strong to me
>>
>> It's used only by a handful of encoders to export CPB side data, which
>> probably only lavf looks at, and by no decoder at all. It's a field that
>> would be completely painless to replace.
>>
>>>
>>> My alternative suggestion was:
>>> * avoid adding a new struct (which merely bundles array+count)
>>> * add a set of helpers that accept array+count as parameters and operate
>>> on them
>>
>> See above my comment about sizeof(AVPacketSideData) and users accessing
>> the array it directly.
>>
>>> * use these helpers for all operations on side data across AVPacket,
>>> AVCodecContext, and AVCodecParameters
>>>
>>> I have a moderately strong preference for this approach, but James
>>> disagrees. More opinions are very much welcome.
>
> Ping. We really need opinions on what approach to use.
>
> Here's some visual representation of what is being discussed.
> Currently, things look like this:
>
> #########
> Side data definition
> -----
> struct AVPacketSideData {
> uint8_t *data;
> size_t size;
> enum AVPacketSideDataType type;
> };
> -----
>
> Users
> -----
> struct AVCodecContext {
> [...]
> }
>
> struct AVCodecParameters {
> [...]
> }
>
> struct AVPacket {
> [...]
> AVPacketSideData *side_data;
> int side_data_elems;
> [...]
> }
> uint8_t* av_packet_new_side_data(AVPacket *pkt, enum
> AVPacketSideDataType type,
> size_t size);
> int av_packet_add_side_data(AVPacket *pkt,
> enum AVPacketSideDataType type,
> uint8_t *data, size_t size);
> uint8_t* av_packet_get_side_data(const AVPacket *pkt,
> enum AVPacketSideDataType type,
> size_t *size);
>
> struct AVStream {
> [...]
> AVCodecParameters *codec_par;
> AVPacketSideData *side_data;
> int nb_side_data;
> [...]
> };
> uint8_t* av_stream_new_side_data(AVStream *stream,
> enum AVPacketSideDataType type,
> size_t size);
> int av_stream_add_side_data(AVStream *st,
> enum AVPacketSideDataType type,
> uint8_t *data, size_t size);
> uint8_t* av_stream_get_side_data(const AVStream *stream,
> enum AVPacketSideDataType type,
> size_t *size);
> -----
> #########
>
> The only way for global side data exported by demuxers to hit decoders
> or bsfs is by injecting it into the first output packet. This means it's
> not available at init().
> The solution is obviously being able to store the side data in the
> decoder context, which the caller would copy from the demuxer context.
> My suggestion is to introduce a new struct to hold a set of side data,
> and helpers that work with it, whereas Anton's suggestion is to just
> storing the pointer to side data array and side data array size directly
> without wrapping them.
>
> My suggestion would have things look like this:
>
> #########
> Side data definition
> -----
> struct AVPacketSideData {
> uint8_t *data;
> size_t size;
> enum AVPacketSideDataType type;
> };
>
> struct AVPacketSideDataSet {
> AVPacketSideData **sd;
> int nb_sd;
> };
>
> AVPacketSideData *av_packet_side_data_set_new(AVPacketSideDataSet *set,
> enum AVPacketSideDataType type,
> size_t size);
> AVPacketSideData *av_packet_side_data_set_add(AVPacketSideDataSet *set,
> enum AVPacketSideDataType type,
> uint8_t *data, size_t size);
> AVPacketSideData *av_packet_side_data_set_get(AVPacketSideDataSet *set,
> enum AVPacketSideDataType type);
> void av_packet_side_data_set_remove(AVPacketSideDataSet *set,
> enum AVPacketSideDataType type);
> void av_packet_side_data_set_uninit(AVPacketSideDataSet *set);
> -----
>
> Users
> -----
> struct AVCodecContext {
> [...]
> AVPacketSideDataSet *side_data;
> [...]
> }
>
> struct AVCodecParameters {
> [...]
> AVPacketSideDataSet *side_data;
> [...]
> }
>
> AVPacket and its helpers untouched.
>
> struct AVStream {
> [...]
> AVCodecParameters *codec_par;
> [...]
> };
> -----
>
> In which you'd do for example
>
> av_packet_side_data_set_new(avctx->side_data,
> AV_PKT_DATA_DISPLAYMATRIX,
> sizeof(int32_t) * 9);
> av_packet_side_data_set_new(st->codec_par->side_data,
> AV_PKT_DATA_DISPLAYMATRIX,
> sizeof(int32_t) * 9);
> av_packet_side_data_set_get(avctx->side_data,
> AV_PKT_DATA_DISPLAYMATRIX);
> av_packet_side_data_set_get(st->codec_par->side_data,
> AV_PKT_DATA_DISPLAYMATRIX);
> #########
>
> Whereas Anton's would have things look like this:
>
> #########
> Side data definition
> -----
> struct AVPacketSideData {
> uint8_t *data;
> size_t size;
> enum AVPacketSideDataType type;
> };
>
> AVPacketSideData *av_packet_side_data_set_new(
> AVPacketSideData **sd, // INOUT
> size_t *sd_size, // INOUT
> enum AVPacketSideDataType type,
> size_t size);
> AVPacketSideData *av_packet_side_data_set_add(
> AVPacketSideData **sd, // INOUT
> size_t *sd_size, // INOUT
> enum AVPacketSideDataType type,
> uint8_t *data, size_t size);
> AVPacketSideData *av_packet_side_data_set_get(AVPacketSideData *sd,
> size_t sd_size,
> enum AVPacketSideDataType type);
> void av_packet_side_data_set_remove(AVPacketSideData *sd,
> size_t *sd_size,
> enum AVPacketSideDataType type);
> void av_packet_side_data_set_uninit(AVPacketSideData *sd);
> -----
>
> Users
> -----
> struct AVCodecContext {
> [...]
> AVPacketSideData *coded_side_data;
> int nb_coded_side_data;
> [...]
> }
>
> struct AVCodecParameters {
> [...]
> AVPacketSideData *side_data;
> int nb_side_data;
> [...]
> }
>
> AVPacket and its helpers untouched.
>
> struct AVStream {
> [...]
> AVCodecParameters *codec_par;
> [...]
> };
> -----
>
> In which you'd do for example
>
> av_packet_side_data_set_new(&avctx->coded_side_data,
> &avctx->nb_coded_side_data,
> AV_PKT_DATA_DISPLAYMATRIX,
> av_packet_side_data_set_new(&st->codec_par->side_data,
> &st->codec_par->nb_side_data,
> AV_PKT_DATA_DISPLAYMATRIX,
> sizeof(int32_t) * 9);
> av_packet_side_data_set_get(avctx->coded_side_data,
> avctx->nb_coded_side_data,
> AV_PKT_DATA_DISPLAYMATRIX);
> av_packet_side_data_set_get(st->codec_par->side_data,
> st->codec_par->nb_side_data,
> AV_PKT_DATA_DISPLAYMATRIX);
> #########
>
> Does anyone have a preference?
Addition of newer API to just reduce number of arguments when
using it looks to me too much for little gain.
> _______________________________________________
> 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".
>
_______________________________________________
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] 28+ messages in thread* Re: [FFmpeg-devel] [PATCH 00/10 v4][RFC] AVCodecContext and AVCodecParameters side data
2023-09-25 20:02 ` Paul B Mahol
@ 2023-09-25 20:18 ` James Almer
0 siblings, 0 replies; 28+ messages in thread
From: James Almer @ 2023-09-25 20:18 UTC (permalink / raw)
To: ffmpeg-devel
On 9/25/2023 5:02 PM, Paul B Mahol wrote:
> On 9/25/23, James Almer <jamrial@gmail.com> wrote:
>> On 9/14/2023 1:40 PM, James Almer wrote:
>>> On 9/14/2023 12:43 PM, Anton Khirnov wrote:
>>>> Quoting James Almer (2023-09-06 19:44:20)
>>>>> Changes since the previous version:
>>>>> - Removed the AVPacketSideDataSet from AVCodecContext, using instead the
>>>>> existing coded_side_data field, at Anton's request.
>>>>>
>>>>> I still prefer using a new field of type AVPacketSideDataSet, given
>>>>> that the
>>>>> user can currently only fill coded_side_data manually (See the
>>>>> implementation
>>>>> in the codecpar copy functions, which non lavf users would need to
>>>>> replicate),
>>>>> and more so considering coded_side_data is a field pretty much
>>>>> nothing but lavf
>>>>> uses.
>>>>>
>>>>> Opinions are very welcome and needed.
>>>>
>>>> To provide more context on the issue:
>>>>
>>>> AVPackets can have side data attached to them, in the form of
>>>> AVPacket.{side_data,side_data_elems} array+count.
>>>>
>>>> Some of this side data can occasionally be stored in global headers,
>>>> e.g. in containers or extradata. We have some limited support for this:
>>>> * AVStream.{side_data,nb_side_data} array+count allows demuxers to
>>>> export this to user, and muxers to accept it from the user
>>>> * AVCodecContext.{coded_side_data,nb_coded_side_data} allows decoders to
>>>> accept it from the user (in principle, in practice it is not used),
>>>> and encoders to export it to the user (this is used, but not very
>>>> much)
>>>>
>>>> The broad idea for this set is adding stream-global "coded/packet" side
>>>> data to AVCodecParameters, so that it can be naturally communicated from
>>>> demuxers to bitstream filters to decoders, and from encoders to
>>>> bitstream filters to muxers. Since AVStream already contains an
>>>> AVCodecParameters instance, the abovementioned AVStream.side_data
>>>> becomes redundant and is deprecated, the muxer/demuxer stream-global
>>>> side data would be passed through AVStream.codecpar.
>>>>
>>>> The original version proposed by James adds a new struct, that bundles
>>>> the side data array+count, and a set of helpers operating on this
>>>> struct. Then this new struct is added to AVCodecParameters and
>>>> AVCodecContext, which replaces the abovementioned AVStream and
>>>> AVCodecContext side data array+count. Notably AVPacket is left as-is,
>>>> because its side data is widely used and replacing array+count by this
>>>> new struct would be a very intrusive break for little gain.
>>>
>>> In the future, packet side data could be made ref counted. For this,
>>> we'd need to add a new AVBufferRef field to AVPacketSideData, which is
>>> currently tied to the ABI.
>>> Ideally, if this happens, sizeof(AVPacketSideData) would be made not a
>>> part of the ABI, putting it in line with the AVFrame counterpart. Doing
>>> this is pretty much not an option for the existing array+count fields in
>>> AVPacket and AVCodecContext, because everyone, from external API users
>>> to our own API users like the CLI last i checked, loop through it
>>> manually as nothing prevents them to. But if a new field of the new set
>>> struct is added as replacement (a struct where the AVPacketSideData
>>> field is an array of pointers, and whose helpers can be defined as "must
>>> use to handle this struct"), then this would not be an issue.
>>>
>>> I probably said it several times, including in the above paragraph, but
>>> one of my objectives is trying to sync frame and packet side data
>>> structs, fields and helpers, since right now both are considerably
>>> different.
>>> Jan has a patchset also introducing a side data set for frames, so this
>>> is the time to try and align both as much as possible (which i already
>>> talked to him about) and laying the ground for future changes.
>>>
>>>>
>>>> My objections to this approach are
>>>> * coded side data is now stored differently in AVPacket and in
>>>> everything else
>>>> * the case for replacing AVCodecContext.coded_side_data does not seem
>>>> sufficiently strong to me
>>>
>>> It's used only by a handful of encoders to export CPB side data, which
>>> probably only lavf looks at, and by no decoder at all. It's a field that
>>> would be completely painless to replace.
>>>
>>>>
>>>> My alternative suggestion was:
>>>> * avoid adding a new struct (which merely bundles array+count)
>>>> * add a set of helpers that accept array+count as parameters and operate
>>>> on them
>>>
>>> See above my comment about sizeof(AVPacketSideData) and users accessing
>>> the array it directly.
>>>
>>>> * use these helpers for all operations on side data across AVPacket,
>>>> AVCodecContext, and AVCodecParameters
>>>>
>>>> I have a moderately strong preference for this approach, but James
>>>> disagrees. More opinions are very much welcome.
>>
>> Ping. We really need opinions on what approach to use.
>>
>> Here's some visual representation of what is being discussed.
>> Currently, things look like this:
>>
>> #########
>> Side data definition
>> -----
>> struct AVPacketSideData {
>> uint8_t *data;
>> size_t size;
>> enum AVPacketSideDataType type;
>> };
>> -----
>>
>> Users
>> -----
>> struct AVCodecContext {
>> [...]
>> }
>>
>> struct AVCodecParameters {
>> [...]
>> }
>>
>> struct AVPacket {
>> [...]
>> AVPacketSideData *side_data;
>> int side_data_elems;
>> [...]
>> }
>> uint8_t* av_packet_new_side_data(AVPacket *pkt, enum
>> AVPacketSideDataType type,
>> size_t size);
>> int av_packet_add_side_data(AVPacket *pkt,
>> enum AVPacketSideDataType type,
>> uint8_t *data, size_t size);
>> uint8_t* av_packet_get_side_data(const AVPacket *pkt,
>> enum AVPacketSideDataType type,
>> size_t *size);
>>
>> struct AVStream {
>> [...]
>> AVCodecParameters *codec_par;
>> AVPacketSideData *side_data;
>> int nb_side_data;
>> [...]
>> };
>> uint8_t* av_stream_new_side_data(AVStream *stream,
>> enum AVPacketSideDataType type,
>> size_t size);
>> int av_stream_add_side_data(AVStream *st,
>> enum AVPacketSideDataType type,
>> uint8_t *data, size_t size);
>> uint8_t* av_stream_get_side_data(const AVStream *stream,
>> enum AVPacketSideDataType type,
>> size_t *size);
>> -----
>> #########
>>
>> The only way for global side data exported by demuxers to hit decoders
>> or bsfs is by injecting it into the first output packet. This means it's
>> not available at init().
>> The solution is obviously being able to store the side data in the
>> decoder context, which the caller would copy from the demuxer context.
>> My suggestion is to introduce a new struct to hold a set of side data,
>> and helpers that work with it, whereas Anton's suggestion is to just
>> storing the pointer to side data array and side data array size directly
>> without wrapping them.
>>
>> My suggestion would have things look like this:
>>
>> #########
>> Side data definition
>> -----
>> struct AVPacketSideData {
>> uint8_t *data;
>> size_t size;
>> enum AVPacketSideDataType type;
>> };
>>
>> struct AVPacketSideDataSet {
>> AVPacketSideData **sd;
>> int nb_sd;
>> };
>>
>> AVPacketSideData *av_packet_side_data_set_new(AVPacketSideDataSet *set,
>> enum AVPacketSideDataType type,
>> size_t size);
>> AVPacketSideData *av_packet_side_data_set_add(AVPacketSideDataSet *set,
>> enum AVPacketSideDataType type,
>> uint8_t *data, size_t size);
>> AVPacketSideData *av_packet_side_data_set_get(AVPacketSideDataSet *set,
>> enum AVPacketSideDataType type);
>> void av_packet_side_data_set_remove(AVPacketSideDataSet *set,
>> enum AVPacketSideDataType type);
>> void av_packet_side_data_set_uninit(AVPacketSideDataSet *set);
>> -----
>>
>> Users
>> -----
>> struct AVCodecContext {
>> [...]
>> AVPacketSideDataSet *side_data;
>> [...]
>> }
>>
>> struct AVCodecParameters {
>> [...]
>> AVPacketSideDataSet *side_data;
>> [...]
>> }
>>
>> AVPacket and its helpers untouched.
>>
>> struct AVStream {
>> [...]
>> AVCodecParameters *codec_par;
>> [...]
>> };
>> -----
>>
>> In which you'd do for example
>>
>> av_packet_side_data_set_new(avctx->side_data,
>> AV_PKT_DATA_DISPLAYMATRIX,
>> sizeof(int32_t) * 9);
>> av_packet_side_data_set_new(st->codec_par->side_data,
>> AV_PKT_DATA_DISPLAYMATRIX,
>> sizeof(int32_t) * 9);
>> av_packet_side_data_set_get(avctx->side_data,
>> AV_PKT_DATA_DISPLAYMATRIX);
>> av_packet_side_data_set_get(st->codec_par->side_data,
>> AV_PKT_DATA_DISPLAYMATRIX);
>> #########
>>
>> Whereas Anton's would have things look like this:
>>
>> #########
>> Side data definition
>> -----
>> struct AVPacketSideData {
>> uint8_t *data;
>> size_t size;
>> enum AVPacketSideDataType type;
>> };
>>
>> AVPacketSideData *av_packet_side_data_set_new(
>> AVPacketSideData **sd, // INOUT
>> size_t *sd_size, // INOUT
>> enum AVPacketSideDataType type,
>> size_t size);
>> AVPacketSideData *av_packet_side_data_set_add(
>> AVPacketSideData **sd, // INOUT
>> size_t *sd_size, // INOUT
>> enum AVPacketSideDataType type,
>> uint8_t *data, size_t size);
>> AVPacketSideData *av_packet_side_data_set_get(AVPacketSideData *sd,
>> size_t sd_size,
>> enum AVPacketSideDataType type);
>> void av_packet_side_data_set_remove(AVPacketSideData *sd,
>> size_t *sd_size,
>> enum AVPacketSideDataType type);
>> void av_packet_side_data_set_uninit(AVPacketSideData *sd);
>> -----
>>
>> Users
>> -----
>> struct AVCodecContext {
>> [...]
>> AVPacketSideData *coded_side_data;
>> int nb_coded_side_data;
>> [...]
>> }
>>
>> struct AVCodecParameters {
>> [...]
>> AVPacketSideData *side_data;
>> int nb_side_data;
>> [...]
>> }
>>
>> AVPacket and its helpers untouched.
>>
>> struct AVStream {
>> [...]
>> AVCodecParameters *codec_par;
>> [...]
>> };
>> -----
>>
>> In which you'd do for example
>>
>> av_packet_side_data_set_new(&avctx->coded_side_data,
>> &avctx->nb_coded_side_data,
>> AV_PKT_DATA_DISPLAYMATRIX,
>> av_packet_side_data_set_new(&st->codec_par->side_data,
>> &st->codec_par->nb_side_data,
>> AV_PKT_DATA_DISPLAYMATRIX,
>> sizeof(int32_t) * 9);
>> av_packet_side_data_set_get(avctx->coded_side_data,
>> avctx->nb_coded_side_data,
>> AV_PKT_DATA_DISPLAYMATRIX);
>> av_packet_side_data_set_get(st->codec_par->side_data,
>> st->codec_par->nb_side_data,
>> AV_PKT_DATA_DISPLAYMATRIX);
>> #########
>>
>> Does anyone have a preference?
>
> Addition of newer API to just reduce number of arguments when
> using it looks to me too much for little gain.
The difference also lies in the fact the new struct will hold an array
of pointers of AVPacketSideData, meaning sizeof(AVPacketSideData) can be
made non ABI fixed in the long run, whereas the side data in avctx and
codecpar are and will be an array of AVPacketSideData, meaning
sizeof(AVPacketSideData) must remain tied to the ABI.
This means adding new fields becomes much harder, and we lose an
opportunity to put AVPacketSideData in line with AVFrameSideData.
And as i stated, Jan will add his own set of side data, for
AVFrameSideData, and that one needs to be in a new set struct as the
alternative is having helpers with arguments like "AVFrameSideData
***side_data".
_______________________________________________
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] 28+ messages in thread