On 2023-02-16 12:22 pm, Andreas Rheinhardt wrote:
Gyan Doshi:
Helper to transfer programs from one muxing context to another.
---
doc/APIchanges | 3 ++
libavformat/avformat.c | 70 ++++++++++++++++++++++++++++++++++++++++++
libavformat/avformat.h | 14 +++++++++
libavformat/version.h | 2 +-
4 files changed, 88 insertions(+), 1 deletion(-)
diff --git a/doc/APIchanges b/doc/APIchanges
index 6baf914760..4916d1abe8 100644
--- a/doc/APIchanges
+++ b/doc/APIchanges
@@ -14,6 +14,9 @@ libavutil: 2021-04-27
API changes, most recent first:
+2023-02-xx - xxxxxxxxxx - lavf 59.39.100 - avformat.h
+ Add av_program_copy()
+
2023-0x-xx - xxxxxxxxxx - lavc 59.63.100
Allow AV_CODEC_FLAG_COPY_OPAQUE to be used with decoders.
diff --git a/libavformat/avformat.c b/libavformat/avformat.c
index 19c7219471..d3c8def170 100644
--- a/libavformat/avformat.c
+++ b/libavformat/avformat.c
@@ -359,6 +359,76 @@ void av_program_add_stream_index(AVFormatContext *ac, int
progid, unsigned idx)
}
}
+int av_program_copy(AVFormatContext *dst, AVFormatContext *src, int progid, int overwrite)
+{
+ AVProgram *src_prog = NULL;
Should be const.
+ AVProgram *dst_prog = NULL;
+ int i, j, ret;
+ int idx = -1;
+
+ for (i = 0; i < src->nb_programs; i++)
1. nb_programs is unsigned and therefore the iterator should be, too.
2. Use a smaller scope for the iterator.
(Same for all other iterators.)
+ if (src->programs[i]->id == progid)
+ src_prog = src->programs[i];
+
+ if (!src_prog) {
+ av_log(src, AV_LOG_ERROR, "source program not found: id=0x%04x\n",
progid);
+ return AVERROR(EINVAL);
+ }
+
+ for (i = 0; i < dst->nb_programs; i++) {
+ if (dst->programs[i]->id == progid)
+ idx = i;
+ }
+
+ if (idx >= 0 && !overwrite) {
+ av_log(src, AV_LOG_ERROR, "target muxer already has program with id=0x%04x;
not overwriting\n", progid);
+ return AVERROR(EINVAL);
This should not be an error.
+ }
+
+ av_log(src, AV_LOG_TRACE, "%s program: id=0x%04x\n", idx >= 0 ? "overwriting" :
"copying", progid);
+
+ if (idx >= 0) {
+ dst_prog = dst->programs[idx];
+ av_dict_free(&dst_prog->metadata);
+ av_freep(&dst_prog->stream_index);
+ dst_prog->nb_stream_indexes = 0;
+ } else {
+ dst_prog = av_mallocz(sizeof(*dst_prog));
+ if (!dst_prog)
+ return AVERROR(ENOMEM);
+ ret = av_dynarray_add_nofree(&dst->programs, &dst->nb_programs,
dst_prog);
av_dynarray_add_nofree() presumes that dst->programs points to a buffer
for a power-of-two pointers (or to NULL); what if a user has reallocated
dst->programs himself and moved AVPrograms from another AVFormatContext
manually? Do we treat this as API violation?
That scenario should be treated the same way as in av_new_program().
But I presume that if a user knows of and uses this helper, they won't
have done that.
I can add it in the doxy as a caveat.
+ if (ret < 0) {
+ av_free(dst_prog);
+ return AVERROR(ENOMEM);
+ }
+ }
+
+ /* public fields */
+ dst_prog->id = src_prog->id;
+ dst_prog->flags = src_prog->flags;
+ dst_prog->discard = src_prog->discard;
+ dst_prog->program_num = src_prog->program_num;
+ dst_prog->pmt_pid = src_prog->pmt_pid;
+ dst_prog->pcr_pid = src_prog->pcr_pid;
+ dst_prog->pmt_version = src_prog->pmt_version;
+
+ for (i = 0; i < dst->nb_streams; i++) {
+ for (j = 0; j < src_prog->nb_stream_indexes; j++)
+ if (dst->streams[i]->id ==
src->streams[src_prog->stream_index[j]]->id)
The documentation should mention that AVStream.id is used for
stream-matching.
+ av_program_add_stream_index(dst, dst_prog->id, i);
This involves a realloction whose success can't be checked due to a
design bug in av_program_add_stream_index(). Add an
ff_program_add_stream_index() without this design bug and turn
av_program_add_stream_index() into a wrapper for it.
Isn't it better to deprecate this and add av_program_add_stream_index2()
without the bug?
+ }
+
+ av_dict_copy(&dst_prog->metadata, src_prog->metadata, 0);
Missing error check.
Ok, but as a note, most usages (80%+) in the codebase omit the check,
including
lavu/opt.c, lavu/tests/dict.c, lavf/mux.c and fftools/*
Do I restore the destination program (when overwriting) or bow out?
+
+ /* private fields */
+ dst_prog->start_time = src_prog->start_time;
+ dst_prog->end_time = src_prog->end_time;
+ dst_prog->pts_wrap_reference = src_prog->pts_wrap_reference;
+ dst_prog->pts_wrap_behavior = src_prog->pts_wrap_behavior;
These private fields are demuxer-only; there is no need to copy them for
muxers.
+
+ return 0;
+}
+
AVProgram *av_find_program_from_stream(AVFormatContext *ic, AVProgram *last,
int s)
{
for (unsigned i = 0; i < ic->nb_programs; i++) {
diff --git a/libavformat/avformat.h b/libavformat/avformat.h
index 1d97d56ac5..2b7ed3abd8 100644
--- a/libavformat/avformat.h
+++ b/libavformat/avformat.h
@@ -1996,6 +1996,20 @@ uint8_t *av_stream_get_side_data(const AVStream *stream,
AVProgram *av_new_program(AVFormatContext *s, int id);
+/**
+ * Copy an AVProgram from one AVFormatContext to another.
+ *
+ * @param dst pointer to the target muxer context
+ * @param src pointer to the source muxer context
+ * @param progid ID of the program to be copied
+ * @param overwrite whether to overwrite if target muxer already
+ * contains a program with the same ID
+ *
+ * @return 0 in case of success, a negative AVERROR code in case of
+ * failure
Better make >= 0 for success for extensibility.
+ */
+int av_program_copy(AVFormatContext *dst, AVFormatContext *src, int progid,
int overwrite);
+
3. Why is this public? Which user is supposed to need this?
Someone implementing tee or similar manually.
Ok to the rest.
Regards,
Gyan
4. src should be const.
/**
* @}
*/
diff --git a/libavformat/version.h b/libavformat/version.h
index 134cdb2b89..9aba356e09 100644
--- a/libavformat/version.h
+++ b/libavformat/version.h
@@ -31,7 +31,7 @@
#include "version_major.h"
-#define LIBAVFORMAT_VERSION_MINOR 38
+#define LIBAVFORMAT_VERSION_MINOR 39
#define LIBAVFORMAT_VERSION_MICRO 100
#define LIBAVFORMAT_VERSION_INT AV_VERSION_INT(LIBAVFORMAT_VERSION_MAJOR, \
_______________________________________________
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".
