| Message ID | 20200113185459.8908-2-nicolas.gaullier@cji.paris |
|---|---|
| State | New |
| Headers | show |
| Series | Fix mpeg1/2 stream copy | expand |
| Context | Check | Description |
|---|---|---|
| andriy/ffmpeg-patchwork | success | Make fate finished |
Quoting Nicolas Gaullier (2020-01-13 19:54:55)
> This will allow avformat_find_stream_info() get side data from the codec context.
If the intention is to use this code from avformat_find_stream_info(),
then why should it be public? The documentation is very unclear as to
when/how an API user is supposed to call it.
>If the intention is to use this code from avformat_find_stream_info(), >then why should it be public? The documentation is very unclear as to >when/how an API user is supposed to call it. > >Anton Khirnov Sorry about that, I should have explained this in the cover letter, or maybe even in the commit msg. The reason is that this code already exists in ffmpeg.c (executed in case of non-codec copy if I remember correctly) and now I need it to be shared and used in avformat_find_stream_info also (for probing in general, and for codec copy in particular) to avoid code duplication. The fact is that initially, I had regrouped all this refactoring in a single commit, thus resulting in a clear full picture but it was not correct to mix libav and fftools changes, so I split the commit but an explanation was lacking, I missed that, sorry. I propose to describe this in the commit msg, this should be enough as the code/usage itself already exists in ffmpeg.c so maybe it is not necessary to add public documentation. Nicolas
Quoting Gaullier Nicolas (2020-01-13 23:16:44) > >If the intention is to use this code from avformat_find_stream_info(), > >then why should it be public? The documentation is very unclear as to > >when/how an API user is supposed to call it. > > > >Anton Khirnov > > Sorry about that, I should have explained this in the cover letter, or maybe even in the commit msg. > The reason is that this code already exists in ffmpeg.c (executed in case of non-codec copy if I remember correctly) > and now I need it to be shared and used in avformat_find_stream_info also (for probing in general, and for codec copy in particular) to avoid code duplication. > The fact is that initially, I had regrouped all this refactoring in a single commit, thus resulting in a clear full picture but it was not correct to mix libav and fftools changes, so I split the commit but an explanation was lacking, I missed that, sorry. > I propose to describe this in the commit msg, this should be enough as the code/usage itself already exists in ffmpeg.c so maybe it is not necessary to add public documentation. The issue here is that adding a public function is kind of a big deal, since once it's there it's hard to change or remove it. It's not something to be done lightly. So every new public API function must have a well-defined purpose and use case. Since this function is not all that well-defined, and also very small and simple I think it's better to just duplicate it.
diff --git a/doc/APIchanges b/doc/APIchanges index 3c24dc6fbc..b3656f2671 100644 --- a/doc/APIchanges +++ b/doc/APIchanges @@ -15,6 +15,9 @@ libavutil: 2017-10-21 API changes, most recent first: +2020-01-13 - xxxxxxxxxx - lavf 58.36.100 - avformat.h + Add av_stream_add_coded_side_data(). + 2019-12-27 - xxxxxxxxxx - lavu 56.38.100 - eval.h Add av_expr_count_func(). diff --git a/libavformat/avformat.h b/libavformat/avformat.h index d4d9a3b06e..4dee1a272a 100644 --- a/libavformat/avformat.h +++ b/libavformat/avformat.h @@ -2191,6 +2191,17 @@ int av_stream_add_side_data(AVStream *st, enum AVPacketSideDataType type, */ uint8_t *av_stream_new_side_data(AVStream *stream, enum AVPacketSideDataType type, int size); + +/** + * Add stream side_data from the coded_side_data of the supplied context. + * + * @param stream stream + * @param avctx the codec context that may contain coded_side_data + * @return >= 0 in case of success, a negative AVERROR code in case of + * failure + */ +int av_stream_add_coded_side_data(AVStream *stream, AVCodecContext *avctx); + /** * Get side information from stream. * diff --git a/libavformat/utils.c b/libavformat/utils.c index f3d71642c3..3270d971c6 100644 --- a/libavformat/utils.c +++ b/libavformat/utils.c @@ -5554,6 +5554,21 @@ uint8_t *av_stream_new_side_data(AVStream *st, enum AVPacketSideDataType type, return data; } +int av_stream_add_coded_side_data(AVStream *st, AVCodecContext *avctx) +{ + int i; + + for (i = 0; i < avctx->nb_coded_side_data; i++) { + const AVPacketSideData *sd_src = &avctx->coded_side_data[i]; + uint8_t *dst_data; + dst_data = av_stream_new_side_data(st, sd_src->type, sd_src->size); + if (!dst_data) + return AVERROR(ENOMEM); + memcpy(dst_data, sd_src->data, sd_src->size); + } + return 0; +} + int ff_stream_add_bitstream_filter(AVStream *st, const char *name, const char *args) { int ret; diff --git a/libavformat/version.h b/libavformat/version.h index 43f3811df1..f72fb9478a 100644 --- a/libavformat/version.h +++ b/libavformat/version.h @@ -32,8 +32,8 @@ // Major bumping may affect Ticket5467, 5421, 5451(compatibility with Chromium) // Also please add any ticket numbers that you believe might be affected here #define LIBAVFORMAT_VERSION_MAJOR 58 -#define LIBAVFORMAT_VERSION_MINOR 35 -#define LIBAVFORMAT_VERSION_MICRO 102 +#define LIBAVFORMAT_VERSION_MINOR 36 +#define LIBAVFORMAT_VERSION_MICRO 100 #define LIBAVFORMAT_VERSION_INT AV_VERSION_INT(LIBAVFORMAT_VERSION_MAJOR, \ LIBAVFORMAT_VERSION_MINOR, \