| Message ID | 20220731215157.22022-1-pal@sandflow.com |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel,v5,1/2] avformat: refactor ff_stream_encode_params_copy() to ff_stream_params_copy() | expand |
| Context | Check | Description |
|---|---|---|
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
pal@sandflow.com: > From: Pierre-Anthony Lemieux <pal@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.) - Andreas
On Thu, Aug 4, 2022 at 9:53 AM Andreas Rheinhardt <andreas.rheinhardt@outlook.com> wrote: > > pal@sandflow.com: > > From: Pierre-Anthony Lemieux <pal@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()? > > - 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-request@ffmpeg.org with subject "unsubscribe".
Pierre-Anthony Lemieux: > On Thu, Aug 4, 2022 at 9:53 AM Andreas Rheinhardt > <andreas.rheinhardt@outlook.com> wrote: >> >> pal@sandflow.com: >>> From: Pierre-Anthony Lemieux <pal@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. - Andreas
On Thu, Aug 4, 2022 at 10:15 AM Andreas Rheinhardt <andreas.rheinhardt@outlook.com> wrote: > > Pierre-Anthony Lemieux: > > On Thu, Aug 4, 2022 at 9:53 AM Andreas Rheinhardt > > <andreas.rheinhardt@outlook.com> wrote: > >> > >> pal@sandflow.com: > >>> From: Pierre-Anthony Lemieux <pal@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; } > > - 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-request@ffmpeg.org with subject "unsubscribe".
Pierre-Anthony Lemieux: > On Thu, Aug 4, 2022 at 10:15 AM Andreas Rheinhardt > <andreas.rheinhardt@outlook.com> wrote: >> >> Pierre-Anthony Lemieux: >>> On Thu, Aug 4, 2022 at 9:53 AM Andreas Rheinhardt >>> <andreas.rheinhardt@outlook.com> wrote: >>>> >>>> pal@sandflow.com: >>>>> From: Pierre-Anthony Lemieux <pal@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); - Andreas
On Thu, Aug 4, 2022 at 3:22 PM Andreas Rheinhardt <andreas.rheinhardt@outlook.com> wrote: > > Pierre-Anthony Lemieux: > > On Thu, Aug 4, 2022 at 10:15 AM Andreas Rheinhardt > > <andreas.rheinhardt@outlook.com> wrote: > >> > >> Pierre-Anthony Lemieux: > >>> On Thu, Aug 4, 2022 at 9:53 AM Andreas Rheinhardt > >>> <andreas.rheinhardt@outlook.com> wrote: > >>>> > >>>> pal@sandflow.com: > >>>>> From: Pierre-Anthony Lemieux <pal@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]? > > - 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-request@ffmpeg.org with subject "unsubscribe".
Pierre-Anthony Lemieux: > On Thu, Aug 4, 2022 at 3:22 PM Andreas Rheinhardt > <andreas.rheinhardt@outlook.com> wrote: >> >> Pierre-Anthony Lemieux: >>> On Thu, Aug 4, 2022 at 10:15 AM Andreas Rheinhardt >>> <andreas.rheinhardt@outlook.com> wrote: >>>> >>>> Pierre-Anthony Lemieux: >>>>> On Thu, Aug 4, 2022 at 9:53 AM Andreas Rheinhardt >>>>> <andreas.rheinhardt@outlook.com> wrote: >>>>>> >>>>>> pal@sandflow.com: >>>>>>> From: Pierre-Anthony Lemieux <pal@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
On Thu, Aug 4, 2022 at 4:13 PM Andreas Rheinhardt <andreas.rheinhardt@outlook.com> wrote: > > Pierre-Anthony Lemieux: > > On Thu, Aug 4, 2022 at 3:22 PM Andreas Rheinhardt > > <andreas.rheinhardt@outlook.com> wrote: > >> > >> Pierre-Anthony Lemieux: > >>> On Thu, Aug 4, 2022 at 10:15 AM Andreas Rheinhardt > >>> <andreas.rheinhardt@outlook.com> wrote: > >>>> > >>>> Pierre-Anthony Lemieux: > >>>>> On Thu, Aug 4, 2022 at 9:53 AM Andreas Rheinhardt > >>>>> <andreas.rheinhardt@outlook.com> wrote: > >>>>>> > >>>>>> pal@sandflow.com: > >>>>>>> From: Pierre-Anthony Lemieux <pal@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. Oh. I missed the return of AVStream*. We lose the error code returned by stream_params_copy(). Works for me though. > 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-request@ffmpeg.org with subject "unsubscribe".
On Thu, Aug 4, 2022 at 4:24 PM Pierre-Anthony Lemieux <pal@sandflow.com> wrote: > > On Thu, Aug 4, 2022 at 4:13 PM Andreas Rheinhardt > <andreas.rheinhardt@outlook.com> wrote: > > > > Pierre-Anthony Lemieux: > > > On Thu, Aug 4, 2022 at 3:22 PM Andreas Rheinhardt > > > <andreas.rheinhardt@outlook.com> wrote: > > >> > > >> Pierre-Anthony Lemieux: > > >>> On Thu, Aug 4, 2022 at 10:15 AM Andreas Rheinhardt > > >>> <andreas.rheinhardt@outlook.com> wrote: > > >>>> > > >>>> Pierre-Anthony Lemieux: > > >>>>> On Thu, Aug 4, 2022 at 9:53 AM Andreas Rheinhardt > > >>>>> <andreas.rheinhardt@outlook.com> wrote: > > >>>>>> > > >>>>>> pal@sandflow.com: > > >>>>>>> From: Pierre-Anthony Lemieux <pal@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. > > Oh. I missed the return of AVStream*. > > We lose the error code returned by stream_params_copy(). Works for me though. Addressed at v6. > > > 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-request@ffmpeg.org with subject "unsubscribe".
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)
From: Pierre-Anthony Lemieux <pal@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(-)