diff mbox series

[FFmpeg-devel,1/3] avcodec: add an AVCodecContext field to signal types of packet, frame, and coded stream side data to export

Message ID 20200202222307.10074-1-jamrial@gmail.com
State New
Headers show
Series [FFmpeg-devel,1/3] avcodec: add an AVCodecContext field to signal types of packet, frame, and coded stream side data to export
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 fail Make fate failed

Commit Message

James Almer Feb. 2, 2020, 10:23 p.m. UTC
Deprecate flags2's export_mvs and add a replacement for it in export_side_data.

Signed-off-by: James Almer <jamrial@gmail.com>
---
TODO: Version bump and APIChanges entry.

 libavcodec/avcodec.h       | 24 +++++++++++++++++++++++-
 libavcodec/mpegpicture.c   |  2 +-
 libavcodec/mpegutils.c     |  2 +-
 libavcodec/options_table.h |  4 +++-
 libavcodec/snowdec.c       |  2 +-
 libavcodec/utils.c         |  7 +++++++
 libavcodec/version.h       |  3 +++
 7 files changed, 39 insertions(+), 5 deletions(-)

Comments

Carl Eugen Hoyos Feb. 4, 2020, 1:03 a.m. UTC | #1
Am So., 2. Feb. 2020 um 23:31 Uhr schrieb James Almer <jamrial@gmail.com>:

> -{"export_mvs", "export motion vectors through frame side data", 0, AV_OPT_TYPE_CONST, {.i64 = AV_CODEC_FLAG2_EXPORT_MVS}, INT_MIN, INT_MAX, V|D, "flags2"},
> +{"export_mvs", "export motion vectors through frame side data (Deprecated, see export_side_data)", 0, AV_OPT_TYPE_CONST, {.i64 = AV_CODEC_FLAG2_EXPORT_MVS}, INT_MIN, INT_MAX, V|D, "flags2"},

Is there a technical reason why this functionality has to be updated
every few years?

Carl Eugen
James Almer Feb. 4, 2020, 1:20 a.m. UTC | #2
On 2/3/2020 10:03 PM, Carl Eugen Hoyos wrote:
> Am So., 2. Feb. 2020 um 23:31 Uhr schrieb James Almer <jamrial@gmail.com>:
> 
>> -{"export_mvs", "export motion vectors through frame side data", 0, AV_OPT_TYPE_CONST, {.i64 = AV_CODEC_FLAG2_EXPORT_MVS}, INT_MIN, INT_MAX, V|D, "flags2"},
>> +{"export_mvs", "export motion vectors through frame side data (Deprecated, see export_side_data)", 0, AV_OPT_TYPE_CONST, {.i64 = AV_CODEC_FLAG2_EXPORT_MVS}, INT_MIN, INT_MAX, V|D, "flags2"},
> 
> Is there a technical reason why this functionality has to be updated
> every few years?
> 
> Carl Eugen

I was suggested to move the flag from flags2 to the new field, as it was
a better fit for it.
It doesn't "have" to, but i guess based on your concern that this flag
had the bad luck of having been moved around before?
Carl Eugen Hoyos Feb. 4, 2020, 1:23 a.m. UTC | #3
Am Di., 4. Feb. 2020 um 02:20 Uhr schrieb James Almer <jamrial@gmail.com>:
>
> On 2/3/2020 10:03 PM, Carl Eugen Hoyos wrote:
> > Am So., 2. Feb. 2020 um 23:31 Uhr schrieb James Almer <jamrial@gmail.com>:
> >
> >> -{"export_mvs", "export motion vectors through frame side data", 0, AV_OPT_TYPE_CONST, {.i64 = AV_CODEC_FLAG2_EXPORT_MVS}, INT_MIN, INT_MAX, V|D, "flags2"},
> >> +{"export_mvs", "export motion vectors through frame side data (Deprecated, see export_side_data)", 0, AV_OPT_TYPE_CONST, {.i64 = AV_CODEC_FLAG2_EXPORT_MVS}, INT_MIN, INT_MAX, V|D, "flags2"},
> >
> > Is there a technical reason why this functionality has to be updated
> > every few years?

> I was suggested to move the flag from flags2 to the new field, as it was
> a better fit for it.

In my understanding (not being a native speaker) this sentence implies
that there (already) is another "field" that is more suitable for the
feature, I believe this is not the case.

> It doesn't "have" to, but i guess based on your concern that this flag
> had the bad luck of having been moved around before?

Correct.

In addition, I wonder how this - relatively simple - patch can block another
one. I suggest the blocked patch gets committed first, then we discuss if
this repeated movement of options makes any sense.

Carl Eugen
James Almer Feb. 4, 2020, 1:55 a.m. UTC | #4
On 2/3/2020 10:23 PM, Carl Eugen Hoyos wrote:
> Am Di., 4. Feb. 2020 um 02:20 Uhr schrieb James Almer <jamrial@gmail.com>:
>>
>> On 2/3/2020 10:03 PM, Carl Eugen Hoyos wrote:
>>> Am So., 2. Feb. 2020 um 23:31 Uhr schrieb James Almer <jamrial@gmail.com>:
>>>
>>>> -{"export_mvs", "export motion vectors through frame side data", 0, AV_OPT_TYPE_CONST, {.i64 = AV_CODEC_FLAG2_EXPORT_MVS}, INT_MIN, INT_MAX, V|D, "flags2"},
>>>> +{"export_mvs", "export motion vectors through frame side data (Deprecated, see export_side_data)", 0, AV_OPT_TYPE_CONST, {.i64 = AV_CODEC_FLAG2_EXPORT_MVS}, INT_MIN, INT_MAX, V|D, "flags2"},
>>>
>>> Is there a technical reason why this functionality has to be updated
>>> every few years?
> 
>> I was suggested to move the flag from flags2 to the new field, as it was
>> a better fit for it.
> 
> In my understanding (not being a native speaker) this sentence implies
> that there (already) is another "field" that is more suitable for the
> feature, I believe this is not the case.

What i tried to say is that the new field i'm introducing,
export_side_data, is a more adequate place for the export_mvs
flag/functionality than the generic flags2 field.

> 
>> It doesn't "have" to, but i guess based on your concern that this flag
>> had the bad luck of having been moved around before?
> 
> Correct.
> 
> In addition, I wonder how this - relatively simple - patch can block another
> one. I suggest the blocked patch gets committed first, then we discuss if
> this repeated movement of options makes any sense.

Sure, i can do that.

> 
> Carl Eugen
> _______________________________________________
> 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 mbox series

Patch

diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
index 0e7ca1db4d..edd22f3cbe 100644
--- a/libavcodec/avcodec.h
+++ b/libavcodec/avcodec.h
@@ -956,10 +956,14 @@  typedef struct RcOverride{
  * Show all frames before the first keyframe
  */
 #define AV_CODEC_FLAG2_SHOW_ALL       (1 << 22)
+
+#if FF_API_FLAGS2_EXPORT_MVS
 /**
- * Export motion vectors through frame side data
+ * Export motion vectors through frame side data. DEPRECATED!!!!
  */
 #define AV_CODEC_FLAG2_EXPORT_MVS     (1 << 28)
+#endif
+
 /**
  * Do not skip samples and export skip information as frame side data
  */
@@ -1098,6 +1102,14 @@  typedef struct RcOverride{
  */
 #define AV_CODEC_CAP_ENCODER_REORDERED_OPAQUE (1 << 20)
 
+/* Exported side data.
+   These flags can be passed in AVCodecContext.export_side_data before initialization.
+*/
+/**
+ * Export motion vectors through frame side data
+ */
+#define AV_CODEC_EXPORT_DATA_MVS         (1 << 0)
+
 /**
  * Pan Scan area.
  * This specifies the area which should be displayed.
@@ -3395,6 +3407,16 @@  typedef struct AVCodecContext {
      * - encoding: set by user
      */
     int64_t max_samples;
+
+    /**
+     * Bit set of AV_CODEC_EXPORT_DATA_* flags, which affects the kind of
+     * metadata exported in frame, packet, or coded stream side data by
+     * decoders and encoders.
+     *
+     * - decoding: set by user
+     * - encoding: set by user
+     */
+    int export_side_data;
 } AVCodecContext;
 
 #if FF_API_CODEC_GET_SET
diff --git a/libavcodec/mpegpicture.c b/libavcodec/mpegpicture.c
index ecbd77d50e..5fce25ec6e 100644
--- a/libavcodec/mpegpicture.c
+++ b/libavcodec/mpegpicture.c
@@ -211,7 +211,7 @@  static int alloc_picture_tables(AVCodecContext *avctx, Picture *pic, int encodin
 #if FF_API_DEBUG_MV
         avctx->debug_mv ||
 #endif
-        (avctx->flags2 & AV_CODEC_FLAG2_EXPORT_MVS)) {
+        (avctx->export_side_data & AV_CODEC_EXPORT_DATA_MVS)) {
         int mv_size        = 2 * (b8_array_size + 4) * sizeof(int16_t);
         int ref_index_size = 4 * mb_array_size;
 
diff --git a/libavcodec/mpegutils.c b/libavcodec/mpegutils.c
index 3f94540616..c0ee3aae85 100644
--- a/libavcodec/mpegutils.c
+++ b/libavcodec/mpegutils.c
@@ -105,7 +105,7 @@  void ff_print_debug_info2(AVCodecContext *avctx, AVFrame *pict, uint8_t *mbskip_
                          int *low_delay,
                          int mb_width, int mb_height, int mb_stride, int quarter_sample)
 {
-    if ((avctx->flags2 & AV_CODEC_FLAG2_EXPORT_MVS) && mbtype_table && motion_val[0]) {
+    if ((avctx->export_side_data & AV_CODEC_EXPORT_DATA_MVS) && mbtype_table && motion_val[0]) {
         const int shift = 1 + quarter_sample;
         const int scale = 1 << shift;
         const int mv_sample_log2 = avctx->codec_id == AV_CODEC_ID_H264 || avctx->codec_id == AV_CODEC_ID_SVQ3 ? 2 : 1;
diff --git a/libavcodec/options_table.h b/libavcodec/options_table.h
index d4c0cdeb48..ec17b20078 100644
--- a/libavcodec/options_table.h
+++ b/libavcodec/options_table.h
@@ -75,9 +75,11 @@  static const AVOption avcodec_options[] = {
 {"local_header", "place global headers at every keyframe instead of in extradata", 0, AV_OPT_TYPE_CONST, {.i64 = AV_CODEC_FLAG2_LOCAL_HEADER }, INT_MIN, INT_MAX, V|E, "flags2"},
 {"chunks", "Frame data might be split into multiple chunks", 0, AV_OPT_TYPE_CONST, {.i64 = AV_CODEC_FLAG2_CHUNKS }, INT_MIN, INT_MAX, V|D, "flags2"},
 {"showall", "Show all frames before the first keyframe", 0, AV_OPT_TYPE_CONST, {.i64 = AV_CODEC_FLAG2_SHOW_ALL }, INT_MIN, INT_MAX, V|D, "flags2"},
-{"export_mvs", "export motion vectors through frame side data", 0, AV_OPT_TYPE_CONST, {.i64 = AV_CODEC_FLAG2_EXPORT_MVS}, INT_MIN, INT_MAX, V|D, "flags2"},
+{"export_mvs", "export motion vectors through frame side data (Deprecated, see export_side_data)", 0, AV_OPT_TYPE_CONST, {.i64 = AV_CODEC_FLAG2_EXPORT_MVS}, INT_MIN, INT_MAX, V|D, "flags2"},
 {"skip_manual", "do not skip samples and export skip information as frame side data", 0, AV_OPT_TYPE_CONST, {.i64 = AV_CODEC_FLAG2_SKIP_MANUAL}, INT_MIN, INT_MAX, A|D, "flags2"},
 {"ass_ro_flush_noop", "do not reset ASS ReadOrder field on flush", 0, AV_OPT_TYPE_CONST, {.i64 = AV_CODEC_FLAG2_RO_FLUSH_NOOP}, INT_MIN, INT_MAX, S|D, "flags2"},
+{"export_side_data", "Export metadata as side data", OFFSET(export_side_data), AV_OPT_TYPE_FLAGS, {.i64 = DEFAULT}, 0, UINT_MAX, V|A|E|D|S, "export_side_data"},
+{"mvs", "export motion vectors through frame side data", 0, AV_OPT_TYPE_CONST, {.i64 = AV_CODEC_EXPORT_DATA_MVS}, INT_MIN, INT_MAX, V|D, "export_side_data"},
 {"time_base", NULL, OFFSET(time_base), AV_OPT_TYPE_RATIONAL, {.dbl = 0}, 0, INT_MAX},
 {"g", "set the group of picture (GOP) size", OFFSET(gop_size), AV_OPT_TYPE_INT, {.i64 = 12 }, INT_MIN, INT_MAX, V|E},
 {"ar", "set audio sampling rate (in Hz)", OFFSET(sample_rate), AV_OPT_TYPE_INT, {.i64 = DEFAULT }, 0, INT_MAX, A|D|E},
diff --git a/libavcodec/snowdec.c b/libavcodec/snowdec.c
index 59bd24e881..519e377a11 100644
--- a/libavcodec/snowdec.c
+++ b/libavcodec/snowdec.c
@@ -502,7 +502,7 @@  static int decode_frame(AVCodecContext *avctx, void *data, int *got_frame,
               );
 
     av_assert0(!s->avmv);
-    if (s->avctx->flags2 & AV_CODEC_FLAG2_EXPORT_MVS) {
+    if (s->avctx->export_side_data & AV_CODEC_EXPORT_DATA_MVS) {
         s->avmv = av_malloc_array(s->b_width * s->b_height, sizeof(AVMotionVector) << (s->block_max_depth*2));
     }
     s->avmv_index = 0;
diff --git a/libavcodec/utils.c b/libavcodec/utils.c
index c685b9c9d7..357e7d1db6 100644
--- a/libavcodec/utils.c
+++ b/libavcodec/utils.c
@@ -947,6 +947,13 @@  FF_ENABLE_DEPRECATION_WARNINGS
         && avctx->codec_descriptor->type == AVMEDIA_TYPE_VIDEO)
         av_log(avctx, AV_LOG_WARNING,
                "gray decoding requested but not enabled at configuration time\n");
+#if FF_API_FLAGS2_EXPORT_MVS
+    if (avctx->flags2 & AV_CODEC_FLAG2_EXPORT_MVS) {
+        av_log(avctx, AV_LOG_WARNING,
+               "flags2 export_mvs is deprecated. Use export_side_data mvs instead\n");
+        avctx->export_side_data |= AV_CODEC_EXPORT_DATA_MVS;
+    }
+#endif
 
     if (   avctx->codec->init && (!(avctx->active_thread_type&FF_THREAD_FRAME)
         || avci->frame_thread_encoder)) {
diff --git a/libavcodec/version.h b/libavcodec/version.h
index 2fba26e8d0..aa6c6ae030 100644
--- a/libavcodec/version.h
+++ b/libavcodec/version.h
@@ -135,6 +135,9 @@ 
 #ifndef FF_API_UNSANITIZED_BITRATES
 #define FF_API_UNSANITIZED_BITRATES (LIBAVCODEC_VERSION_MAJOR < 59)
 #endif
+#ifndef FF_API_FLAGS2_EXPORT_MVS
+#define FF_API_FLAGS2_EXPORT_MVS (LIBAVCODEC_VERSION_MAJOR < 59)
+#endif
 
 
 #endif /* AVCODEC_VERSION_H */