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 <jamr...@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-requ...@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-requ...@ffmpeg.org with subject "unsubscribe".

Reply via email to