diff mbox series

[FFmpeg-devel,v6,1/5] avformat: Add av_stream_add_coded_side_data()

Message ID 20200113185459.8908-2-nicolas.gaullier@cji.paris
State New
Headers show
Series Fix mpeg1/2 stream copy
Related show

Checks

Context Check Description
andriy/ffmpeg-patchwork pending
andriy/ffmpeg-patchwork success Applied patch
andriy/ffmpeg-patchwork success Configure finished
andriy/ffmpeg-patchwork success Make finished
andriy/ffmpeg-patchwork success Make fate finished

Commit Message

Gaullier Nicolas Jan. 13, 2020, 6:54 p.m. UTC
This will allow avformat_find_stream_info() get side data from the codec context.
---
 doc/APIchanges         |  3 +++
 libavformat/avformat.h | 11 +++++++++++
 libavformat/utils.c    | 15 +++++++++++++++
 libavformat/version.h  |  4 ++--
 4 files changed, 31 insertions(+), 2 deletions(-)

Comments

Anton Khirnov Jan. 13, 2020, 7:33 p.m. UTC | #1
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.
Gaullier Nicolas Jan. 13, 2020, 10:16 p.m. UTC | #2
>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
Anton Khirnov Jan. 14, 2020, 7:39 p.m. UTC | #3
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 mbox series

Patch

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, \