Pierre-Anthony Lemieux: > On Thu, Aug 4, 2022 at 3:22 PM Andreas Rheinhardt > <andreas.rheinha...@outlook.com> wrote: >> >> Pierre-Anthony Lemieux: >>> On Thu, Aug 4, 2022 at 10:15 AM Andreas Rheinhardt >>> <andreas.rheinha...@outlook.com> wrote: >>>> >>>> Pierre-Anthony Lemieux: >>>>> On Thu, Aug 4, 2022 at 9:53 AM Andreas Rheinhardt >>>>> <andreas.rheinha...@outlook.com> wrote: >>>>>> >>>>>> p...@sandflow.com: >>>>>>> From: Pierre-Anthony Lemieux <p...@palemieux.com> >>>>>>> >>>>>>> As discussed at >>>>>>> https://ffmpeg.org/pipermail/ffmpeg-devel/2022-July/298491.html. >>>>>>> Note that ff_stream_params_copy() does not copy: >>>>>>> * the index field >>>>>>> * the attached_pic if its size is 0 >>>>>>> >>>>>>> Addresses >>>>>>> http://ffmpeg.org/pipermail/ffmpeg-devel/2022-August/299514.html >>>>>>> >>>>>>> --- >>>>>>> libavformat/avformat.c | 40 ++++++++++++++++++++++++++++++++++++++++ >>>>>>> libavformat/fifo.c | 2 +- >>>>>>> libavformat/internal.h | 12 ++++++++++++ >>>>>>> libavformat/mux.h | 9 --------- >>>>>>> libavformat/mux_utils.c | 28 ---------------------------- >>>>>>> libavformat/segment.c | 2 +- >>>>>>> libavformat/tee.c | 2 +- >>>>>>> libavformat/webm_chunk.c | 2 +- >>>>>>> 8 files changed, 56 insertions(+), 41 deletions(-) >>>>>>> >>>>>>> diff --git a/libavformat/avformat.c b/libavformat/avformat.c >>>>>>> index 30d6ea6a49..242187c359 100644 >>>>>>> --- a/libavformat/avformat.c >>>>>>> +++ b/libavformat/avformat.c >>>>>>> @@ -235,6 +235,46 @@ int ff_stream_side_data_copy(AVStream *dst, const >>>>>>> AVStream *src) >>>>>>> return 0; >>>>>>> } >>>>>>> >>>>>>> +int ff_stream_params_copy(AVStream *dst, const AVStream *src) >>>>>>> +{ >>>>>>> + int ret; >>>>>>> + >>>>>>> + dst->id = src->id; >>>>>>> + dst->time_base = src->time_base; >>>>>>> + dst->start_time = src->start_time; >>>>>>> + dst->duration = src->duration; >>>>>>> + dst->nb_frames = src->nb_frames; >>>>>>> + dst->disposition = src->disposition; >>>>>>> + dst->discard = src->discard; >>>>>>> + dst->sample_aspect_ratio = src->sample_aspect_ratio; >>>>>>> + dst->avg_frame_rate = src->avg_frame_rate; >>>>>>> + dst->event_flags = src->event_flags; >>>>>>> + dst->r_frame_rate = src->r_frame_rate; >>>>>>> + dst->pts_wrap_bits = src->pts_wrap_bits; >>>>>>> + >>>>>>> + av_dict_free(&dst->metadata); >>>>>>> + ret = av_dict_copy(&dst->metadata, src->metadata, 0); >>>>>>> + if (ret < 0) >>>>>>> + return ret; >>>>>>> + >>>>>>> + ret = avcodec_parameters_copy(dst->codecpar, src->codecpar); >>>>>>> + 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.size > 0) { >>>>>>> + ret = av_packet_ref(&dst->attached_pic, &src->attached_pic); >>>>>>> + if (ret < 0) >>>>>>> + return ret; >>>>>>> + } >>>>>>> + >>>>>>> + return 0; >>>>>>> +} >>>>>>> + >>>>>>> AVProgram *av_new_program(AVFormatContext *ac, int id) >>>>>>> { >>>>>>> AVProgram *program = NULL; >>>>>>> diff --git a/libavformat/fifo.c b/libavformat/fifo.c >>>>>>> index ead2bdc5cf..fef116d040 100644 >>>>>>> --- a/libavformat/fifo.c >>>>>>> +++ b/libavformat/fifo.c >>>>>>> @@ -509,7 +509,7 @@ static int fifo_mux_init(AVFormatContext *avf, >>>>>>> const AVOutputFormat *oformat, >>>>>>> if (!st) >>>>>>> return AVERROR(ENOMEM); >>>>>>> >>>>>>> - ret = ff_stream_encode_params_copy(st, avf->streams[i]); >>>>>>> + ret = ff_stream_params_copy(st, avf->streams[i]); >>>>>>> if (ret < 0) >>>>>>> return ret; >>>>>>> } >>>>>>> diff --git a/libavformat/internal.h b/libavformat/internal.h >>>>>>> index b6b8fbf56f..87a00bbae3 100644 >>>>>>> --- a/libavformat/internal.h >>>>>>> +++ b/libavformat/internal.h >>>>>>> @@ -625,6 +625,18 @@ enum AVCodecID ff_get_pcm_codec_id(int bps, int >>>>>>> flt, int be, int sflags); >>>>>>> */ >>>>>>> int ff_stream_side_data_copy(AVStream *dst, const AVStream *src); >>>>>>> >>>>>>> +/** >>>>>>> + * Copy all stream parameters from source to destination stream, with >>>>>>> the >>>>>>> + * exception of: >>>>>>> + * * the index field, which is usually set by avformat_new_stream() >>>>>>> + * * the attached_pic field, if attached_pic.size is 0 or less >>>>>>> + * >>>>>>> + * @param dst pointer to destination AVStream >>>>>>> + * @param src pointer to source AVStream >>>>>>> + * @return >=0 on success, AVERROR code on error >>>>>>> + */ >>>>>>> +int ff_stream_params_copy(AVStream *dst, const AVStream *src); >>>>>>> + >>>>>>> /** >>>>>>> * Wrap ffurl_move() and log if error happens. >>>>>>> * >>>>>>> diff --git a/libavformat/mux.h b/libavformat/mux.h >>>>>>> index c01da82194..1bfcaf795f 100644 >>>>>>> --- a/libavformat/mux.h >>>>>>> +++ b/libavformat/mux.h >>>>>>> @@ -113,15 +113,6 @@ int ff_format_shift_data(AVFormatContext *s, >>>>>>> int64_t read_start, int shift_size) >>>>>>> */ >>>>>>> int ff_format_output_open(AVFormatContext *s, const char *url, >>>>>>> AVDictionary **options); >>>>>>> >>>>>>> -/** >>>>>>> - * Copy encoding parameters from source to destination stream >>>>>>> - * >>>>>>> - * @param dst pointer to destination AVStream >>>>>>> - * @param src pointer to source AVStream >>>>>>> - * @return >=0 on success, AVERROR code on error >>>>>>> - */ >>>>>>> -int ff_stream_encode_params_copy(AVStream *dst, const AVStream *src); >>>>>>> - >>>>>>> /** >>>>>>> * Parse creation_time in AVFormatContext metadata if exists and warn >>>>>>> if the >>>>>>> * parsing fails. >>>>>>> diff --git a/libavformat/mux_utils.c b/libavformat/mux_utils.c >>>>>>> index eb8ea3d560..2fa2ab5b0f 100644 >>>>>>> --- a/libavformat/mux_utils.c >>>>>>> +++ b/libavformat/mux_utils.c >>>>>>> @@ -121,34 +121,6 @@ int ff_format_output_open(AVFormatContext *s, >>>>>>> const char *url, AVDictionary **op >>>>>>> return 0; >>>>>>> } >>>>>>> >>>>>>> -int ff_stream_encode_params_copy(AVStream *dst, const AVStream *src) >>>>>>> -{ >>>>>>> - int ret; >>>>>>> - >>>>>>> - dst->id = src->id; >>>>>>> - dst->time_base = src->time_base; >>>>>>> - dst->nb_frames = src->nb_frames; >>>>>>> - dst->disposition = src->disposition; >>>>>>> - dst->sample_aspect_ratio = src->sample_aspect_ratio; >>>>>>> - dst->avg_frame_rate = src->avg_frame_rate; >>>>>>> - dst->r_frame_rate = src->r_frame_rate; >>>>>>> - >>>>>>> - av_dict_free(&dst->metadata); >>>>>>> - ret = av_dict_copy(&dst->metadata, src->metadata, 0); >>>>>>> - if (ret < 0) >>>>>>> - return ret; >>>>>>> - >>>>>>> - ret = avcodec_parameters_copy(dst->codecpar, src->codecpar); >>>>>>> - if (ret < 0) >>>>>>> - return ret; >>>>>>> - >>>>>>> - ret = ff_stream_side_data_copy(dst, src); >>>>>>> - if (ret < 0) >>>>>>> - return ret; >>>>>>> - >>>>>>> - return 0; >>>>>>> -} >>>>>>> - >>>>>>> int ff_parse_creation_time_metadata(AVFormatContext *s, int64_t >>>>>>> *timestamp, int return_seconds) >>>>>>> { >>>>>>> AVDictionaryEntry *entry; >>>>>>> diff --git a/libavformat/segment.c b/libavformat/segment.c >>>>>>> index fa435d9f32..a8f3e94714 100644 >>>>>>> --- a/libavformat/segment.c >>>>>>> +++ b/libavformat/segment.c >>>>>>> @@ -169,7 +169,7 @@ static int segment_mux_init(AVFormatContext *s) >>>>>>> >>>>>>> if (!(st = avformat_new_stream(oc, NULL))) >>>>>>> return AVERROR(ENOMEM); >>>>>>> - ret = ff_stream_encode_params_copy(st, ist); >>>>>>> + ret = ff_stream_params_copy(st, ist); >>>>>>> if (ret < 0) >>>>>>> return ret; >>>>>>> opar = st->codecpar; >>>>>>> diff --git a/libavformat/tee.c b/libavformat/tee.c >>>>>>> index f1f2a9d2a5..dbfad604d0 100644 >>>>>>> --- a/libavformat/tee.c >>>>>>> +++ b/libavformat/tee.c >>>>>>> @@ -289,7 +289,7 @@ static int open_slave(AVFormatContext *avf, char >>>>>>> *slave, TeeSlave *tee_slave) >>>>>>> goto end; >>>>>>> } >>>>>>> >>>>>>> - ret = ff_stream_encode_params_copy(st2, st); >>>>>>> + ret = ff_stream_params_copy(st2, st); >>>>>>> if (ret < 0) >>>>>>> goto end; >>>>>>> } >>>>>>> diff --git a/libavformat/webm_chunk.c b/libavformat/webm_chunk.c >>>>>>> index d69db3a004..39f21fce7a 100644 >>>>>>> --- a/libavformat/webm_chunk.c >>>>>>> +++ b/libavformat/webm_chunk.c >>>>>>> @@ -94,7 +94,7 @@ static int webm_chunk_init(AVFormatContext *s) >>>>>>> if (!(st = avformat_new_stream(oc, NULL))) >>>>>>> return AVERROR(ENOMEM); >>>>>>> >>>>>>> - if ((ret = ff_stream_encode_params_copy(st, ost)) < 0) >>>>>>> + if ((ret = ff_stream_params_copy(st, ost)) < 0) >>>>>>> return ret; >>>>>>> >>>>>>> if (wc->http_method) >>>>>> >>>>>> Looking at these callers shows that they all have one thing in common: >>>>>> They create a stream and immediately afterwards copy stream parameters. >>>>>> The caller that you intend to add in #2 does the same. How about we make >>>>>> a function that is equivalent to >>>>>> avformat_new_stream+ff_stream_params_copy above and use that? (But >>>>>> please leave ff_stream_params_copy as it is in the form of a static >>>>>> function.) >>>>> >>>>> avformat_clone_stream()? >>>>> >>>> >>>> I was not thinking about a public function. But clone_stream sounds good. >>> >>> Something like this? >>> >>> int ff_stream_clone(AVFormatContext *s, AVStream **dst, const AVStream *src) >>> { >>> AVStream *st; >>> int ret; >>> >>> st = avformat_new_stream(s, NULL); >>> if (!st) >>> return AVERROR(ENOMEM); >>> >>> ret = stream_params_copy(st, src); >>> if (ret < 0) >>> return ret; >>> >>> *dst = st; >>> >>> return 0; >>> } >>> >> >> I'd use AVStream *ff_stream_clone(AVFormatContext *dst_ctx, const >> AVStream *src); > > How does one recover a pointer to the stream that was just created? > Isn't there a potential race condition with dst_ctx[nb_streams - 1]? >
? 1. The function is supposed to return the stream just created; just like avformat_new_stream does. 2. If there are multiple threads adding streams to dst_ctx, we'd already be screwed anyway (avformat_new_stream does not use any lock). - Andreas _______________________________________________ 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".