diff mbox

[FFmpeg-devel,V6,2/2] avcodec/libx264: add support for ROI-based encoding

Message ID 1546890878-7164-1-git-send-email-yejun.guo@intel.com
State Accepted
Headers show

Commit Message

Guo, Yejun Jan. 7, 2019, 7:54 p.m. UTC 
This patch just enables the path from ffmpeg to libx264,
the more encoders can be added later.

Signed-off-by: Guo, Yejun <yejun.guo@intel.com>
---
 libavcodec/libx264.c | 67 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 67 insertions(+)

Comments

Michael Niedermayer Jan. 7, 2019, 7:19 p.m. UTC | #1 
On Tue, Jan 08, 2019 at 03:54:38AM +0800, Guo, Yejun wrote:
> This patch just enables the path from ffmpeg to libx264,
> the more encoders can be added later.
> 
> Signed-off-by: Guo, Yejun <yejun.guo@intel.com>
> ---
>  libavcodec/libx264.c | 67 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 67 insertions(+)
> 
> diff --git a/libavcodec/libx264.c b/libavcodec/libx264.c
> index a68d0a7..9cfbaed 100644
> --- a/libavcodec/libx264.c
> +++ b/libavcodec/libx264.c
> @@ -40,6 +40,10 @@
>  #include <stdlib.h>
>  #include <string.h>
>  
> +// from x264.h, for quant_offsets, Macroblocks are 16x16
> +// blocks of pixels (with respect to the luma plane)
> +#define MB_SIZE 16
> +
>  typedef struct X264Context {
>      AVClass        *class;
>      x264_param_t    params;
> @@ -282,6 +286,7 @@ static int X264_frame(AVCodecContext *ctx, AVPacket *pkt, const AVFrame *frame,
>      x264_picture_t pic_out = {0};
>      int pict_type;
>      int64_t *out_opaque;
> +    AVFrameSideData *sd;
>  
>      x264_picture_init( &x4->pic );
>      x4->pic.img.i_csp   = x4->params.i_csp;
> @@ -345,6 +350,68 @@ 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) {

> +            static int show_aq_warning = 1;
> +            static int show_interlaced_frame_warning = 1;

non constant statics are a bad idea.
there is not neccesarily just a single x264 encoder


> +            if (x4->params.rc.i_aq_mode == X264_AQ_NONE && show_aq_warning) {
> +                show_aq_warning = 0;
> +                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 nb_rois;
> +                    AVRegionOfInterest* roi;
> +                    float* qoffsets;

> +                    qoffsets = (float*)av_mallocz_array(mbx * mby, sizeof(*qoffsets));

the cast is unneeded



> +                    if (!qoffsets)
> +                        return AVERROR(ENOMEM);
> +
> +                    nb_rois = sd->size / sizeof(AVRegionOfInterest);
> +                    roi = (AVRegionOfInterest*)sd->data;
> +                    for (int count = 0; count < nb_rois; count++) {

> +                        int starty = FFMIN(mby, roi->top / MB_SIZE);
> +                        int endy = FFMIN(mby, (roi->bottom + MB_SIZE - 1)/ MB_SIZE);
> +                        int startx = FFMIN(mbx, roi->left / MB_SIZE);
> +                        int endx = FFMIN(mbx, (roi->right + MB_SIZE - 1)/ MB_SIZE);

vertical alignment makes this more readable


> +                        float qoffset;
> +
> +                        if (roi->qoffset.den == 0) {
> +                            av_free(qoffsets);
> +                            av_log(ctx, AV_LOG_ERROR, "AVRegionOfInterest.qoffset.den should not be zero.\n");
> +                            return AVERROR(EINVAL);
> +                        }
> +                        qoffset = roi->qoffset.num * 1.0f / roi->qoffset.den;

> +                        qoffset = FFMIN(qoffset, 1.0f);
> +                        qoffset = FFMAX(qoffset, -1.0f);

av_clipf()

> +
> +                        // 25 is a number that I think it is a possible proper scale value.
> +                        qoffset = qoffset * 25;
> +
> +                        for (int y = starty; y < endy; y++) {
> +                            for (int x = startx; x < endx; x++) {
> +                                qoffsets[x + y*mbx] = qoffset;
> +                            }
> +                        }
> +
> +                        if (roi->self_size == 0) {
> +                            av_free(qoffsets);
> +                            av_log(ctx, AV_LOG_ERROR, "AVRegionOfInterest.self_size should be set to sizeof(AVRegionOfInterest).\n");
> +                            return AVERROR(EINVAL);
> +                        }
> +                        roi = (AVRegionOfInterest*)((char*)roi + roi->self_size);
> +                    }
> +
> +                    x4->pic.prop.quant_offsets = qoffsets;
> +                    x4->pic.prop.quant_offsets_free = av_free;
> +                } else if (show_interlaced_frame_warning) {
> +                    show_interlaced_frame_warning = 0;
> +                    av_log(ctx, AV_LOG_WARNING, "interlaced_frame not supported for ROI encoding yet, skipping ROI.\n");
> +                }
> +            }
> +        }
>      }
>  
>      do {
> -- 
> 2.7.4
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Guo, Yejun Jan. 8, 2019, 1:23 a.m. UTC | #2 
> -----Original Message-----
> From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On Behalf
> Of Michael Niedermayer
> Sent: Tuesday, January 08, 2019 3:20 AM
> To: FFmpeg development discussions and patches <ffmpeg-
> devel@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH V6 2/2] avcodec/libx264: add support
> for ROI-based encoding
> 
> On Tue, Jan 08, 2019 at 03:54:38AM +0800, Guo, Yejun wrote:
> > This patch just enables the path from ffmpeg to libx264, the more
> > encoders can be added later.
> >
> > Signed-off-by: Guo, Yejun <yejun.guo@intel.com>
> > ---
> >  libavcodec/libx264.c | 67
> > ++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 67 insertions(+)
> >
> > diff --git a/libavcodec/libx264.c b/libavcodec/libx264.c index
> > a68d0a7..9cfbaed 100644
> > --- a/libavcodec/libx264.c
> > +++ b/libavcodec/libx264.c
> > @@ -40,6 +40,10 @@
> >  #include <stdlib.h>
> >  #include <string.h>
> >
> > +// from x264.h, for quant_offsets, Macroblocks are 16x16 // blocks of
> > +pixels (with respect to the luma plane) #define MB_SIZE 16
> > +
> >  typedef struct X264Context {
> >      AVClass        *class;
> >      x264_param_t    params;
> > @@ -282,6 +286,7 @@ static int X264_frame(AVCodecContext *ctx,
> AVPacket *pkt, const AVFrame *frame,
> >      x264_picture_t pic_out = {0};
> >      int pict_type;
> >      int64_t *out_opaque;
> > +    AVFrameSideData *sd;
> >
> >      x264_picture_init( &x4->pic );
> >      x4->pic.img.i_csp   = x4->params.i_csp;
> > @@ -345,6 +350,68 @@ 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) {
> 
> > +            static int show_aq_warning = 1;
> > +            static int show_interlaced_frame_warning = 1;
> 
> non constant statics are a bad idea.
> there is not neccesarily just a single x264 encoder

ok, I'll revert it.

> 
> 
> > +            if (x4->params.rc.i_aq_mode == X264_AQ_NONE &&
> show_aq_warning) {
> > +                show_aq_warning = 0;
> > +                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 nb_rois;
> > +                    AVRegionOfInterest* roi;
> > +                    float* qoffsets;
> 
> > +                    qoffsets = (float*)av_mallocz_array(mbx * mby,
> > + sizeof(*qoffsets));
> 
> the cast is unneeded

thanks, will remove it.

> 
> 
> 
> > +                    if (!qoffsets)
> > +                        return AVERROR(ENOMEM);
> > +
> > +                    nb_rois = sd->size / sizeof(AVRegionOfInterest);
> > +                    roi = (AVRegionOfInterest*)sd->data;
> > +                    for (int count = 0; count < nb_rois; count++) {
> 
> > +                        int starty = FFMIN(mby, roi->top / MB_SIZE);
> > +                        int endy = FFMIN(mby, (roi->bottom + MB_SIZE - 1)/
> MB_SIZE);
> > +                        int startx = FFMIN(mbx, roi->left / MB_SIZE);
> > +                        int endx = FFMIN(mbx, (roi->right + MB_SIZE -
> > + 1)/ MB_SIZE);
> 
> vertical alignment makes this more readable

yes, will refine it.

> 
> 
> > +                        float qoffset;
> > +
> > +                        if (roi->qoffset.den == 0) {
> > +                            av_free(qoffsets);
> > +                            av_log(ctx, AV_LOG_ERROR,
> "AVRegionOfInterest.qoffset.den should not be zero.\n");
> > +                            return AVERROR(EINVAL);
> > +                        }
> > +                        qoffset = roi->qoffset.num * 1.0f /
> > + roi->qoffset.den;
> 
> > +                        qoffset = FFMIN(qoffset, 1.0f);
> > +                        qoffset = FFMAX(qoffset, -1.0f);
> 
> av_clipf()

thanks, will change to it.

> 
> > +
> > +                        // 25 is a number that I think it is a possible proper scale value.
> > +                        qoffset = qoffset * 25;
> > +
> > +                        for (int y = starty; y < endy; y++) {
> > +                            for (int x = startx; x < endx; x++) {
> > +                                qoffsets[x + y*mbx] = qoffset;
> > +                            }
> > +                        }
> > +
> > +                        if (roi->self_size == 0) {
> > +                            av_free(qoffsets);
> > +                            av_log(ctx, AV_LOG_ERROR, "AVRegionOfInterest.self_size
> should be set to sizeof(AVRegionOfInterest).\n");
> > +                            return AVERROR(EINVAL);
> > +                        }
> > +                        roi = (AVRegionOfInterest*)((char*)roi + roi->self_size);
> > +                    }
> > +
> > +                    x4->pic.prop.quant_offsets = qoffsets;
> > +                    x4->pic.prop.quant_offsets_free = av_free;
> > +                } else if (show_interlaced_frame_warning) {
> > +                    show_interlaced_frame_warning = 0;
> > +                    av_log(ctx, AV_LOG_WARNING, "interlaced_frame not
> supported for ROI encoding yet, skipping ROI.\n");
> > +                }
> > +            }
> > +        }
> >      }
> >
> >      do {
> > --
> > 2.7.4
> >
> > _______________________________________________
> > ffmpeg-devel mailing list
> > ffmpeg-devel@ffmpeg.org
> > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> --
> Michael     GnuPG fingerprint:
> 9FF2128B147EF6730BADF133611EC787040B0FAB
> 
> Everything should be made as simple as possible, but not simpler.
> -- Albert Einstein
diff mbox

Patch

diff --git a/libavcodec/libx264.c b/libavcodec/libx264.c
index a68d0a7..9cfbaed 100644
--- a/libavcodec/libx264.c
+++ b/libavcodec/libx264.c
@@ -40,6 +40,10 @@ 
 #include <stdlib.h>
 #include <string.h>
 
+// from x264.h, for quant_offsets, Macroblocks are 16x16
+// blocks of pixels (with respect to the luma plane)
+#define MB_SIZE 16
+
 typedef struct X264Context {
     AVClass        *class;
     x264_param_t    params;
@@ -282,6 +286,7 @@  static int X264_frame(AVCodecContext *ctx, AVPacket *pkt, const AVFrame *frame,
     x264_picture_t pic_out = {0};
     int pict_type;
     int64_t *out_opaque;
+    AVFrameSideData *sd;
 
     x264_picture_init( &x4->pic );
     x4->pic.img.i_csp   = x4->params.i_csp;
@@ -345,6 +350,68 @@  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) {
+            static int show_aq_warning = 1;
+            static int show_interlaced_frame_warning = 1;
+            if (x4->params.rc.i_aq_mode == X264_AQ_NONE && show_aq_warning) {
+                show_aq_warning = 0;
+                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 nb_rois;
+                    AVRegionOfInterest* roi;
+                    float* qoffsets;
+                    qoffsets = (float*)av_mallocz_array(mbx * mby, sizeof(*qoffsets));
+                    if (!qoffsets)
+                        return AVERROR(ENOMEM);
+
+                    nb_rois = sd->size / sizeof(AVRegionOfInterest);
+                    roi = (AVRegionOfInterest*)sd->data;
+                    for (int count = 0; count < nb_rois; count++) {
+                        int starty = FFMIN(mby, roi->top / MB_SIZE);
+                        int endy = FFMIN(mby, (roi->bottom + MB_SIZE - 1)/ MB_SIZE);
+                        int startx = FFMIN(mbx, roi->left / MB_SIZE);
+                        int endx = FFMIN(mbx, (roi->right + MB_SIZE - 1)/ MB_SIZE);
+                        float qoffset;
+
+                        if (roi->qoffset.den == 0) {
+                            av_free(qoffsets);
+                            av_log(ctx, AV_LOG_ERROR, "AVRegionOfInterest.qoffset.den should not be zero.\n");
+                            return AVERROR(EINVAL);
+                        }
+                        qoffset = roi->qoffset.num * 1.0f / roi->qoffset.den;
+                        qoffset = FFMIN(qoffset, 1.0f);
+                        qoffset = FFMAX(qoffset, -1.0f);
+
+                        // 25 is a number that I think it is a possible proper scale value.
+                        qoffset = qoffset * 25;
+
+                        for (int y = starty; y < endy; y++) {
+                            for (int x = startx; x < endx; x++) {
+                                qoffsets[x + y*mbx] = qoffset;
+                            }
+                        }
+
+                        if (roi->self_size == 0) {
+                            av_free(qoffsets);
+                            av_log(ctx, AV_LOG_ERROR, "AVRegionOfInterest.self_size should be set to sizeof(AVRegionOfInterest).\n");
+                            return AVERROR(EINVAL);
+                        }
+                        roi = (AVRegionOfInterest*)((char*)roi + roi->self_size);
+                    }
+
+                    x4->pic.prop.quant_offsets = qoffsets;
+                    x4->pic.prop.quant_offsets_free = av_free;
+                } else if (show_interlaced_frame_warning) {
+                    show_interlaced_frame_warning = 0;
+                    av_log(ctx, AV_LOG_WARNING, "interlaced_frame not supported for ROI encoding yet, skipping ROI.\n");
+                }
+            }
+        }
     }
 
     do {