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

[James Almer <jamrial@gmail.com>]

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