diff mbox series

[FFmpeg-devel,1/7] mpegvideo: use the AVVideoEncParams API for exporting QP tables

Message ID 20201002180331.20416-1-anton@khirnov.net
State New
Headers show
Series [FFmpeg-devel,1/7] mpegvideo: use the AVVideoEncParams API for exporting QP tables | expand

Checks

Context Check Description
andriy/default pending
andriy/make success Make finished
andriy/make_fate success Make fate finished

Commit Message

Anton Khirnov Oct. 2, 2020, 6:03 p.m. UTC 
Do it only when requested with the AV_CODEC_EXPORT_DATA_VIDEO_ENC_PARAMS
flag.

Drop previous code using the long-deprecated AV_FRAME_DATA_QP_TABLE*
API. Temporarily disable fate-filter-pp, fate-filter-pp7,
fate-filter-spp. They will be reenabled once these filters are converted
in following commits.
---
 doc/APIchanges               |  3 +++
 libavcodec/h263dec.c         |  2 ++
 libavcodec/mpeg12dec.c       |  1 +
 libavcodec/mpegpicture.c     |  2 ++
 libavcodec/mpegpicture.h     |  1 +
 libavcodec/mpegvideo.c       | 35 ++++++++++++++++++++++++++++-------
 libavcodec/rv34.c            |  1 +
 libavutil/version.h          |  2 +-
 libavutil/video_enc_params.h |  8 ++++++++
 tests/fate/filter-video.mak  |  6 +++---
 10 files changed, 50 insertions(+), 11 deletions(-)

Comments

Michael Niedermayer Oct. 3, 2020, 6:26 p.m. UTC | #1 
On Fri, Oct 02, 2020 at 08:03:25PM +0200, Anton Khirnov wrote:
> Do it only when requested with the AV_CODEC_EXPORT_DATA_VIDEO_ENC_PARAMS
> flag.
> 
> Drop previous code using the long-deprecated AV_FRAME_DATA_QP_TABLE*
> API. Temporarily disable fate-filter-pp, fate-filter-pp7,
> fate-filter-spp. They will be reenabled once these filters are converted
> in following commits.

This patchset updates pp, pp7, spp, fspp, qp, codecview
it seems uspp is missed (as it lacks a fate test i guess)

thx

[...]
Anton Khirnov Oct. 5, 2020, 8:40 a.m. UTC | #2 
Quoting Michael Niedermayer (2020-10-03 20:26:20)
> On Fri, Oct 02, 2020 at 08:03:25PM +0200, Anton Khirnov wrote:
> > Do it only when requested with the AV_CODEC_EXPORT_DATA_VIDEO_ENC_PARAMS
> > flag.
> > 
> > Drop previous code using the long-deprecated AV_FRAME_DATA_QP_TABLE*
> > API. Temporarily disable fate-filter-pp, fate-filter-pp7,
> > fate-filter-spp. They will be reenabled once these filters are converted
> > in following commits.
> 
> This patchset updates pp, pp7, spp, fspp, qp, codecview
> it seems uspp is missed (as it lacks a fate test i guess)

No, I skipped uspp because it uses more deprecated APIs than just the QP
table one. Specifically, the old encoding API and
AVCodecContext.coded_frame.

Given that:
- coded_frame is dropped without replacement
- uspp is EXTREMELY slow
- we have several other very similar filters
I have to wonder whether continued maintenance of uspp makes sense.

So I suggest that either
- someone cares enough about this filter to maintain it (i.e. convert it
  to non-deprecated APIs)
- we drop it
Michael Niedermayer Oct. 5, 2020, 10:20 p.m. UTC | #3 
On Mon, Oct 05, 2020 at 10:40:59AM +0200, Anton Khirnov wrote:
> Quoting Michael Niedermayer (2020-10-03 20:26:20)
> > On Fri, Oct 02, 2020 at 08:03:25PM +0200, Anton Khirnov wrote:
> > > Do it only when requested with the AV_CODEC_EXPORT_DATA_VIDEO_ENC_PARAMS
> > > flag.
> > > 
> > > Drop previous code using the long-deprecated AV_FRAME_DATA_QP_TABLE*
> > > API. Temporarily disable fate-filter-pp, fate-filter-pp7,
> > > fate-filter-spp. They will be reenabled once these filters are converted
> > > in following commits.
> > 
> > This patchset updates pp, pp7, spp, fspp, qp, codecview
> > it seems uspp is missed (as it lacks a fate test i guess)
> 
> No, I skipped uspp because it uses more deprecated APIs than just the QP
> table one. Specifically, the old encoding API and
> AVCodecContext.coded_frame.

right, i had forgotten about that
consider my uspp review comment to be withdrawn for now.


[...]
diff mbox series

Patch

diff --git a/doc/APIchanges b/doc/APIchanges
index f2830968bb..97a0f92dca 100644
--- a/doc/APIchanges
+++ b/doc/APIchanges
@@ -15,6 +15,9 @@  libavutil:     2017-10-21
 
 API changes, most recent first:
 
+2020-xx-xx - xxxxxxxxxx - lavu 56.61.100 - video_enc_params.h
+  Add AV_VIDEO_ENC_PARAMS_MPEG2
+
 2020-xx-xx - xxxxxxxxxx - lavu 56.60.100 - buffer.h
   Add a av_buffer_replace() convenience function.
 
diff --git a/libavcodec/h263dec.c b/libavcodec/h263dec.c
index 3b29a189e9..f6b6c33109 100644
--- a/libavcodec/h263dec.c
+++ b/libavcodec/h263dec.c
@@ -28,6 +28,8 @@ 
 #define UNCHECKED_BITSTREAM_READER 1
 
 #include "libavutil/cpu.h"
+#include "libavutil/video_enc_params.h"
+
 #include "avcodec.h"
 #include "error_resilience.h"
 #include "flv.h"
diff --git a/libavcodec/mpeg12dec.c b/libavcodec/mpeg12dec.c
index 2494226aa3..d380b376a6 100644
--- a/libavcodec/mpeg12dec.c
+++ b/libavcodec/mpeg12dec.c
@@ -32,6 +32,7 @@ 
 #include "libavutil/imgutils.h"
 #include "libavutil/internal.h"
 #include "libavutil/stereo3d.h"
+#include "libavutil/video_enc_params.h"
 
 #include "avcodec.h"
 #include "bytestream.h"
diff --git a/libavcodec/mpegpicture.c b/libavcodec/mpegpicture.c
index 13c11ec492..e495e315e6 100644
--- a/libavcodec/mpegpicture.c
+++ b/libavcodec/mpegpicture.c
@@ -220,6 +220,7 @@  static int alloc_picture_tables(AVCodecContext *avctx, Picture *pic, int encodin
 
     pic->alloc_mb_width  = mb_width;
     pic->alloc_mb_height = mb_height;
+    pic->alloc_mb_stride = mb_stride;
 
     return 0;
 }
@@ -346,6 +347,7 @@  int ff_update_picture_tables(Picture *dst, Picture *src)
 
     dst->alloc_mb_width  = src->alloc_mb_width;
     dst->alloc_mb_height = src->alloc_mb_height;
+    dst->alloc_mb_stride = src->alloc_mb_stride;
 
     return 0;
 }
diff --git a/libavcodec/mpegpicture.h b/libavcodec/mpegpicture.h
index 2db3d6733a..4bcd666797 100644
--- a/libavcodec/mpegpicture.h
+++ b/libavcodec/mpegpicture.h
@@ -69,6 +69,7 @@  typedef struct Picture {
 
     int alloc_mb_width;         ///< mb_width used to allocate tables
     int alloc_mb_height;        ///< mb_height used to allocate tables
+    int alloc_mb_stride;        ///< mb_stride used to allocate tables
 
     AVBufferRef *mb_mean_buf;
     uint8_t *mb_mean;           ///< Table for MB luminance
diff --git a/libavcodec/mpegvideo.c b/libavcodec/mpegvideo.c
index c28d1adef7..8cc21920e6 100644
--- a/libavcodec/mpegvideo.c
+++ b/libavcodec/mpegvideo.c
@@ -32,6 +32,8 @@ 
 #include "libavutil/imgutils.h"
 #include "libavutil/internal.h"
 #include "libavutil/motion_vector.h"
+#include "libavutil/video_enc_params.h"
+
 #include "avcodec.h"
 #include "blockdsp.h"
 #include "h264chroma.h"
@@ -1425,14 +1427,33 @@  void ff_print_debug_info(MpegEncContext *s, Picture *p, AVFrame *pict)
 
 int ff_mpv_export_qp_table(MpegEncContext *s, AVFrame *f, Picture *p, int qp_type)
 {
-    AVBufferRef *ref = av_buffer_ref(p->qscale_table_buf);
-    int offset = 2*s->mb_stride + 1;
-    if(!ref)
+    AVVideoEncParams *par;
+    int mult = (qp_type == FF_QSCALE_TYPE_MPEG1) ? 2 : 1;
+    unsigned int nb_mb = p->alloc_mb_height * p->alloc_mb_width;
+    unsigned int x, y;
+
+    if (!(s->avctx->export_side_data & AV_CODEC_EXPORT_DATA_VIDEO_ENC_PARAMS))
+        return 0;
+
+    par = av_video_enc_params_create_side_data(f, AV_VIDEO_ENC_PARAMS_MPEG2, nb_mb);
+    if (!par)
         return AVERROR(ENOMEM);
-    av_assert0(ref->size >= offset + s->mb_stride * ((f->height+15)/16));
-    ref->size -= offset;
-    ref->data += offset;
-    return av_frame_set_qp_table(f, ref, s->mb_stride, qp_type);
+
+    for (y = 0; y < p->alloc_mb_height; y++)
+        for (x = 0; x < p->alloc_mb_width; x++) {
+            const unsigned int block_idx = y * p->alloc_mb_width + x;
+            const unsigned int     mb_xy = y * p->alloc_mb_stride + x;
+            AVVideoBlockParams *b = av_video_enc_params_block(par, block_idx);
+
+            b->src_x = x * 16;
+            b->src_y = y * 16;
+            b->w     = 16;
+            b->h     = 16;
+
+            b->delta_qp = p->qscale_table[mb_xy] * mult;
+        }
+
+    return 0;
 }
 
 static inline int hpel_motion_lowres(MpegEncContext *s,
diff --git a/libavcodec/rv34.c b/libavcodec/rv34.c
index ec0cd27916..44d9581ac9 100644
--- a/libavcodec/rv34.c
+++ b/libavcodec/rv34.c
@@ -26,6 +26,7 @@ 
 
 #include "libavutil/imgutils.h"
 #include "libavutil/internal.h"
+#include "libavutil/video_enc_params.h"
 
 #include "avcodec.h"
 #include "error_resilience.h"
diff --git a/libavutil/version.h b/libavutil/version.h
index 1aca823650..55a59d6c58 100644
--- a/libavutil/version.h
+++ b/libavutil/version.h
@@ -79,7 +79,7 @@ 
  */
 
 #define LIBAVUTIL_VERSION_MAJOR  56
-#define LIBAVUTIL_VERSION_MINOR  60
+#define LIBAVUTIL_VERSION_MINOR  61
 #define LIBAVUTIL_VERSION_MICRO 100
 
 #define LIBAVUTIL_VERSION_INT   AV_VERSION_INT(LIBAVUTIL_VERSION_MAJOR, \
diff --git a/libavutil/video_enc_params.h b/libavutil/video_enc_params.h
index e3b422d6f3..fc0c3bc1a5 100644
--- a/libavutil/video_enc_params.h
+++ b/libavutil/video_enc_params.h
@@ -55,6 +55,14 @@  enum AVVideoEncParamsType {
      *   as AVVideoBlockParams.qp_delta.
      */
     AV_VIDEO_ENC_PARAMS_H264,
+
+    /*
+     * MPEG-2-compatible quantizer.
+     *
+     * Summing the frame-level qp with the per-block delta_qp gives the
+     * resulting quantizer for the block.
+     */
+    AV_VIDEO_ENC_PARAMS_MPEG2,
 };
 
 /**
diff --git a/tests/fate/filter-video.mak b/tests/fate/filter-video.mak
index 639f957748..900e43ac68 100644
--- a/tests/fate/filter-video.mak
+++ b/tests/fate/filter-video.mak
@@ -558,7 +558,7 @@  fate-filter-idet: CMD = framecrc -flags bitexact -idct simple -i $(SRC) -vf idet
 FATE_FILTER_VSYNTH-$(CONFIG_PAD_FILTER) += fate-filter-pad
 fate-filter-pad: CMD = video_filter "pad=iw*1.5:ih*1.5:iw*0.3:ih*0.2"
 
-FATE_FILTER_PP = fate-filter-pp fate-filter-pp1 fate-filter-pp2 fate-filter-pp3 fate-filter-pp4 fate-filter-pp5 fate-filter-pp6
+#FATE_FILTER_PP = fate-filter-pp fate-filter-pp1 fate-filter-pp2 fate-filter-pp3 fate-filter-pp4 fate-filter-pp5 fate-filter-pp6
 FATE_FILTER_VSYNTH-$(CONFIG_PP_FILTER) += $(FATE_FILTER_PP)
 $(FATE_FILTER_PP): fate-vsynth1-mpeg4-qprd
 
@@ -570,11 +570,11 @@  fate-filter-pp4: CMD = video_filter "pp=be/ci"
 fate-filter-pp5: CMD = video_filter "pp=md"
 fate-filter-pp6: CMD = video_filter "pp=be/fd"
 
-FATE_FILTER_VSYNTH-$(CONFIG_PP7_FILTER) += fate-filter-pp7
+#FATE_FILTER_VSYNTH-$(CONFIG_PP7_FILTER) += fate-filter-pp7
 fate-filter-pp7: fate-vsynth1-mpeg4-qprd
 fate-filter-pp7: CMD = framecrc -flags bitexact -idct simple -i $(TARGET_PATH)/tests/data/fate/vsynth1-mpeg4-qprd.avi -frames:v 5 -flags +bitexact -vf "pp7"
 
-FATE_FILTER_VSYNTH-$(CONFIG_SPP_FILTER) += fate-filter-spp
+#FATE_FILTER_VSYNTH-$(CONFIG_SPP_FILTER) += fate-filter-spp
 fate-filter-spp: fate-vsynth1-mpeg4-qprd
 fate-filter-spp: CMD = framecrc -flags bitexact -idct simple -i $(TARGET_PATH)/tests/data/fate/vsynth1-mpeg4-qprd.avi -frames:v 5 -flags +bitexact -vf "spp=idct=simple:dct=int"