diff mbox series

[FFmpeg-devel,2/3] libavcodec/libx264.c: refine code to extract function x264_encode_set_roi

Message ID 1581651132-25279-1-git-send-email-yejun.guo@intel.com
State New
Headers show
Series [FFmpeg-devel,1/3] avutil/frame: add use_last_roi | expand

Checks

Context Check Description
andriy/ffmpeg-patchwork success Make fate finished

Commit Message

Guo, Yejun Feb. 14, 2020, 3:32 a.m. UTC 
Signed-off-by: Guo, Yejun <yejun.guo@intel.com>
---
 libavcodec/libx264.c | 148 ++++++++++++++++++++++++++++-----------------------
 1 file changed, 80 insertions(+), 68 deletions(-)

Comments

Guo, Yejun Feb. 19, 2020, 4:23 a.m. UTC | #1 
> -----Original Message-----
> From: Guo, Yejun
> Sent: Friday, February 14, 2020 11:32 AM
> To: ffmpeg-devel@ffmpeg.org
> Cc: Guo, Yejun <yejun.guo@intel.com>
> Subject: [PATCH 2/3] libavcodec/libx264.c: refine code to extract function
> x264_encode_set_roi
> 
> Signed-off-by: Guo, Yejun <yejun.guo@intel.com>
> ---
>  libavcodec/libx264.c | 148 ++++++++++++++++++++++++++++-----------------------
>  1 file changed, 80 insertions(+), 68 deletions(-)
> 
> diff --git a/libavcodec/libx264.c b/libavcodec/libx264.c
> index ca8f6c0..bad9351 100644
> --- a/libavcodec/libx264.c
> +++ b/libavcodec/libx264.c
> @@ -289,6 +289,83 @@ static void reconfig_encoder(AVCodecContext *ctx,
> const AVFrame *frame)
>      }
>  }
> 
> +static int x264_encode_set_roi(const AVFrame *frame, AVCodecContext *ctx,
> int bit_depth)
> +{

hi, please ignore the other two patches of this patch set, while this patch to refine
the code in libv264.c is valuable and ask for review, thanks.

Yejun
Guo, Yejun Feb. 29, 2020, 4:51 a.m. UTC | #2 
> -----Original Message-----
> From: Guo, Yejun
> Sent: Wednesday, February 19, 2020 12:24 PM
> To: ffmpeg-devel@ffmpeg.org
> Subject: RE: [PATCH 2/3] libavcodec/libx264.c: refine code to extract function
> x264_encode_set_roi
> 
> 
> 
> > -----Original Message-----
> > From: Guo, Yejun
> > Sent: Friday, February 14, 2020 11:32 AM
> > To: ffmpeg-devel@ffmpeg.org
> > Cc: Guo, Yejun <yejun.guo@intel.com>
> > Subject: [PATCH 2/3] libavcodec/libx264.c: refine code to extract function
> > x264_encode_set_roi
> >
> > Signed-off-by: Guo, Yejun <yejun.guo@intel.com>
> > ---
> >  libavcodec/libx264.c | 148
> ++++++++++++++++++++++++++++-----------------------
> >  1 file changed, 80 insertions(+), 68 deletions(-)
> >
> > diff --git a/libavcodec/libx264.c b/libavcodec/libx264.c
> > index ca8f6c0..bad9351 100644
> > --- a/libavcodec/libx264.c
> > +++ b/libavcodec/libx264.c
> > @@ -289,6 +289,83 @@ static void reconfig_encoder(AVCodecContext *ctx,
> > const AVFrame *frame)
> >      }
> >  }
> >
> > +static int x264_encode_set_roi(const AVFrame *frame, AVCodecContext
> *ctx,
> > int bit_depth)
> > +{
> 
> hi, please ignore the other two patches of this patch set, while this patch to
> refine
> the code in libv264.c is valuable and ask for review, thanks.
> 

this patch (not include other patches of this patch set) ask for review, thanks.
diff mbox series

Patch

diff --git a/libavcodec/libx264.c b/libavcodec/libx264.c
index ca8f6c0..bad9351 100644
--- a/libavcodec/libx264.c
+++ b/libavcodec/libx264.c
@@ -289,6 +289,83 @@  static void reconfig_encoder(AVCodecContext *ctx, const AVFrame *frame)
     }
 }
 
+static int x264_encode_set_roi(const AVFrame *frame, AVCodecContext *ctx, int bit_depth)
+{
+    AVFrameSideData *sd = av_frame_get_side_data(frame, AV_FRAME_DATA_REGIONS_OF_INTEREST);
+    X264Context *x4 = ctx->priv_data;
+    int mbx = (frame->width + MB_SIZE - 1) / MB_SIZE;
+    int mby = (frame->height + MB_SIZE - 1) / MB_SIZE;
+    int qp_range = 51 + 6 * (bit_depth - 8);
+    int nb_rois;
+    const AVRegionOfInterest *roi;
+    uint32_t roi_size;
+    float *qoffsets;
+
+    if (!sd) {
+        return 0;
+    }
+
+    if (x4->params.rc.i_aq_mode == X264_AQ_NONE) {
+        if (!x4->roi_warned) {
+            x4->roi_warned = 1;
+            av_log(ctx, AV_LOG_WARNING, "Adaptive quantization must be enabled to use ROI encoding, skipping ROI.\n");
+        }
+        return 0;
+    }
+
+    if (frame->interlaced_frame != 0) {
+        if (!x4->roi_warned) {
+            x4->roi_warned = 1;
+            av_log(ctx, AV_LOG_WARNING, "interlaced_frame not supported for ROI encoding yet, skipping ROI.\n");
+        }
+        return 0;
+    }
+
+    roi = (const AVRegionOfInterest*)sd->data;
+    roi_size = roi->self_size;
+    if (!roi_size || sd->size % roi_size != 0) {
+        av_log(ctx, AV_LOG_ERROR, "Invalid AVRegionOfInterest.self_size.\n");
+        return AVERROR(EINVAL);
+    }
+    nb_rois = sd->size / roi_size;
+
+    qoffsets = av_mallocz_array(mbx * mby, sizeof(*qoffsets));
+    if (!qoffsets)
+        return AVERROR(ENOMEM);
+
+    // This list must be iterated in reverse because the first
+    // region in the list applies when regions overlap.
+    for (int i = nb_rois - 1; i >= 0; i--) {
+        int startx, endx, starty, endy;
+        float qoffset;
+
+        roi = (const AVRegionOfInterest*)(sd->data + roi_size * i);
+
+        starty = FFMIN(mby, roi->top / MB_SIZE);
+        endy   = FFMIN(mby, (roi->bottom + MB_SIZE - 1)/ MB_SIZE);
+        startx = FFMIN(mbx, roi->left / MB_SIZE);
+        endx   = FFMIN(mbx, (roi->right + MB_SIZE - 1)/ MB_SIZE);
+
+        if (roi->qoffset.den == 0) {
+            av_free(qoffsets);
+            av_log(ctx, AV_LOG_ERROR, "AVRegionOfInterest.qoffset.den must not be zero.\n");
+            return AVERROR(EINVAL);
+        }
+        qoffset = roi->qoffset.num * 1.0f / roi->qoffset.den;
+        qoffset = av_clipf(qoffset * qp_range, -qp_range, +qp_range);
+
+        for (int y = starty; y < endy; y++) {
+            for (int x = startx; x < endx; x++) {
+                qoffsets[x + y*mbx] = qoffset;
+            }
+        }
+    }
+
+    x4->pic.prop.quant_offsets = qoffsets;
+    x4->pic.prop.quant_offsets_free = av_free;
+    return 0;
+}
+
 static int X264_frame(AVCodecContext *ctx, AVPacket *pkt, const AVFrame *frame,
                       int *got_packet)
 {
@@ -300,7 +377,6 @@  static int X264_frame(AVCodecContext *ctx, AVPacket *pkt, const AVFrame *frame,
     int bit_depth;
     int64_t wallclock = 0;
     X264Opaque *out_opaque;
-    AVFrameSideData *sd;
 
     x264_picture_init( &x4->pic );
     x4->pic.img.i_csp   = x4->params.i_csp;
@@ -367,73 +443,9 @@  static int X264_frame(AVCodecContext *ctx, AVPacket *pkt, const AVFrame *frame,
             }
         }
 
-        sd = av_frame_get_side_data(frame, AV_FRAME_DATA_REGIONS_OF_INTEREST);
-        if (sd) {
-            if (x4->params.rc.i_aq_mode == X264_AQ_NONE) {
-                if (!x4->roi_warned) {
-                    x4->roi_warned = 1;
-                    av_log(ctx, AV_LOG_WARNING, "Adaptive quantization must be enabled to use ROI encoding, skipping ROI.\n");
-                }
-            } else {
-                if (frame->interlaced_frame == 0) {
-                    int mbx = (frame->width + MB_SIZE - 1) / MB_SIZE;
-                    int mby = (frame->height + MB_SIZE - 1) / MB_SIZE;
-                    int qp_range = 51 + 6 * (bit_depth - 8);
-                    int nb_rois;
-                    const AVRegionOfInterest *roi;
-                    uint32_t roi_size;
-                    float *qoffsets;
-
-                    roi = (const AVRegionOfInterest*)sd->data;
-                    roi_size = roi->self_size;
-                    if (!roi_size || sd->size % roi_size != 0) {
-                        av_log(ctx, AV_LOG_ERROR, "Invalid AVRegionOfInterest.self_size.\n");
-                        return AVERROR(EINVAL);
-                    }
-                    nb_rois = sd->size / roi_size;
-
-                    qoffsets = av_mallocz_array(mbx * mby, sizeof(*qoffsets));
-                    if (!qoffsets)
-                        return AVERROR(ENOMEM);
-
-                    // This list must be iterated in reverse because the first
-                    // region in the list applies when regions overlap.
-                    for (int i = nb_rois - 1; i >= 0; i--) {
-                        int startx, endx, starty, endy;
-                        float qoffset;
-
-                        roi = (const AVRegionOfInterest*)(sd->data + roi_size * i);
-
-                        starty = FFMIN(mby, roi->top / MB_SIZE);
-                        endy   = FFMIN(mby, (roi->bottom + MB_SIZE - 1)/ MB_SIZE);
-                        startx = FFMIN(mbx, roi->left / MB_SIZE);
-                        endx   = FFMIN(mbx, (roi->right + MB_SIZE - 1)/ MB_SIZE);
-
-                        if (roi->qoffset.den == 0) {
-                            av_free(qoffsets);
-                            av_log(ctx, AV_LOG_ERROR, "AVRegionOfInterest.qoffset.den must not be zero.\n");
-                            return AVERROR(EINVAL);
-                        }
-                        qoffset = roi->qoffset.num * 1.0f / roi->qoffset.den;
-                        qoffset = av_clipf(qoffset * qp_range, -qp_range, +qp_range);
-
-                        for (int y = starty; y < endy; y++) {
-                            for (int x = startx; x < endx; x++) {
-                                qoffsets[x + y*mbx] = qoffset;
-                            }
-                        }
-                    }
-
-                    x4->pic.prop.quant_offsets = qoffsets;
-                    x4->pic.prop.quant_offsets_free = av_free;
-                } else {
-                    if (!x4->roi_warned) {
-                        x4->roi_warned = 1;
-                        av_log(ctx, AV_LOG_WARNING, "interlaced_frame not supported for ROI encoding yet, skipping ROI.\n");
-                    }
-                }
-            }
-        }
+        ret = x264_encode_set_roi(frame, ctx, bit_depth);
+        if (ret < 0)
+            return ret;
     }
 
     do {