[FFmpeg-devel] [PATCH 03/10] avformat/avformat: use the side data from AVStream.codecpar

[Andreas Rheinhardt]

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 at 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);
>>>   
>>