diff mbox

[FFmpeg-devel,v2,1/1] avcodec/vaapi_encode: add frame-skip func

Message ID 1550658795-25142-1-git-send-email-jing.a.sun@intel.com
State New
Headers show

Commit Message

Jing SUN Feb. 20, 2019, 10:33 a.m. UTC 
This implements app controlled frame skipping
in vaapi encoding. To make a frame skipped,
allocate its frame side data of the newly
added AV_FRAME_DATA_SKIP_FRAME type and set
its value to 1.

Signed-off-by: Jing SUN <jing.a.sun@intel.com>
---
 libavcodec/vaapi_encode.c | 112 ++++++++++++++++++++++++++++++++++++++++++++--
 libavcodec/vaapi_encode.h |   5 +++
 libavutil/frame.c         |   1 +
 libavutil/frame.h         |   5 +++
 4 files changed, 119 insertions(+), 4 deletions(-)

Comments

Mark Thompson Feb. 20, 2019, 9:34 p.m. UTC | #1 
On 20/02/2019 10:33, Jing SUN wrote:
> This implements app controlled frame skipping
> in vaapi encoding. To make a frame skipped,
> allocate its frame side data of the newly
> added AV_FRAME_DATA_SKIP_FRAME type and set
> its value to 1.
> 
> Signed-off-by: Jing SUN <jing.a.sun@intel.com>
> ---
>  libavcodec/vaapi_encode.c | 112 ++++++++++++++++++++++++++++++++++++++++++++--
>  libavcodec/vaapi_encode.h |   5 +++
>  libavutil/frame.c         |   1 +
>  libavutil/frame.h         |   5 +++
>  4 files changed, 119 insertions(+), 4 deletions(-)

Have a look at <https://lists.ffmpeg.org/pipermail/ffmpeg-devel/2019-February/239989.html>, which tries to implement this feature in a more general way without adding any ad-hoc API.

- Mark
Jing SUN Feb. 22, 2019, 3:09 a.m. UTC | #2 
-----Original Message-----
From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On Behalf Of Mark Thompson

Sent: Thursday, February 21, 2019 5:35 AM
To: ffmpeg-devel@ffmpeg.org
Subject: Re: [FFmpeg-devel] [PATCH v2 1/1] avcodec/vaapi_encode: add frame-skip func

On 20/02/2019 10:33, Jing SUN wrote:
> This implements app controlled frame skipping in vaapi encoding. To 

> make a frame skipped, allocate its frame side data of the newly added 

> AV_FRAME_DATA_SKIP_FRAME type and set its value to 1.

> 

> Signed-off-by: Jing SUN <jing.a.sun@intel.com>

> ---

>  libavcodec/vaapi_encode.c | 112 ++++++++++++++++++++++++++++++++++++++++++++--

>  libavcodec/vaapi_encode.h |   5 +++

>  libavutil/frame.c         |   1 +

>  libavutil/frame.h         |   5 +++

>  4 files changed, 119 insertions(+), 4 deletions(-)


Have a look at <https://lists.ffmpeg.org/pipermail/ffmpeg-devel/2019-February/239989.html>, which tries to implement this feature in a more general way without adding any ad-hoc API.

- Mark
-----Original Message-----

Hi Mark,

I think the 239989 patch is quite a good one to implement VFR by adapting VAAPI frame-skip feature, but it does not fulfill the feature requirement that we have been asked to achieve, which is to skip the frames that are selected by apps, and the purpose to skip any frames is not just to control frame-rate, but also to skip duplicated ones, or to skip abandoned ones due to some changes just made, etc. Could we please keep both and let users decide which way they want to use?

Regards,
SUN, Jing
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Mark Thompson Feb. 26, 2019, 12:01 a.m. UTC | #3 
On 22/02/2019 03:09, Sun, Jing A wrote:
> -----Original Message-----
> From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On Behalf Of Mark Thompson
> Sent: Thursday, February 21, 2019 5:35 AM
> To: ffmpeg-devel@ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH v2 1/1] avcodec/vaapi_encode: add frame-skip func
> 
> On 20/02/2019 10:33, Jing SUN wrote:
>> This implements app controlled frame skipping in vaapi encoding. To 
>> make a frame skipped, allocate its frame side data of the newly added 
>> AV_FRAME_DATA_SKIP_FRAME type and set its value to 1.
>>
>> Signed-off-by: Jing SUN <jing.a.sun@intel.com>
>> ---
>>  libavcodec/vaapi_encode.c | 112 ++++++++++++++++++++++++++++++++++++++++++++--
>>  libavcodec/vaapi_encode.h |   5 +++
>>  libavutil/frame.c         |   1 +
>>  libavutil/frame.h         |   5 +++
>>  4 files changed, 119 insertions(+), 4 deletions(-)
> 
> Have a look at <https://lists.ffmpeg.org/pipermail/ffmpeg-devel/2019-February/239989.html>, which tries to implement this feature in a more general way without adding any ad-hoc API.
> 
> - Mark
> -----Original Message-----
> 
> Hi Mark,
> 
> I think the 239989 patch is quite a good one to implement VFR by adapting VAAPI frame-skip feature, but it does not fulfill the feature requirement that we have been asked to achieve, which is to skip the frames that are selected by apps, and the purpose to skip any frames is not just to control frame-rate, but also to skip duplicated ones, or to skip abandoned ones due to some changes just made, etc. Could we please keep both and let users decide which way they want to use?

I think these use-cases are all supported by the same method used for other encoders in libavcodec, by not sending the skipped frames to the encoder.  The linked patch is only wanted to fix the rate control in these cases - that's otherwise broken because VAAPI lacks any support for timestamps.

Can you explain a specific use-case which wants the extra message?

- Mark
diff mbox

Patch

diff --git a/libavcodec/vaapi_encode.c b/libavcodec/vaapi_encode.c
index b4e9fad..debcfa6 100644
--- a/libavcodec/vaapi_encode.c
+++ b/libavcodec/vaapi_encode.c
@@ -23,6 +23,7 @@ 
 #include "libavutil/common.h"
 #include "libavutil/log.h"
 #include "libavutil/pixdesc.h"
+#include "libavutil/intreadwrite.h"
 
 #include "vaapi_encode.h"
 #include "avcodec.h"
@@ -103,6 +104,41 @@  static int vaapi_encode_make_param_buffer(AVCodecContext *avctx,
     return 0;
 }
 
+static int vaapi_encode_check_if_skip(AVCodecContext *avctx,
+                                      VAAPIEncodePicture *pic)
+{
+    AVFrameSideData *fside = NULL;
+    VAAPIEncodeContext *ctx = avctx->priv_data;
+    VAAPIEncodePicture *cur = NULL;
+    int i = 0;
+    if (!pic || !pic->input_image)
+        return AVERROR(EINVAL);
+    fside = av_frame_get_side_data(pic->input_image, AV_FRAME_DATA_SKIP_FRAME);
+    if (fside)
+        pic->skipped_flag = AV_RL8(fside->data);
+    else
+        pic->skipped_flag = 0;
+    if (0 == pic->skipped_flag)
+        return 0;
+    if ((pic->type == PICTURE_TYPE_IDR) || (pic->type == PICTURE_TYPE_I)) {
+        av_log(avctx, AV_LOG_INFO, "Can't skip IDR/I pic %"PRId64"/%"PRId64".\n",
+               pic->display_order, pic->encode_order);
+        pic->skipped_flag = 0;
+        return 0;
+    }
+    for (cur = ctx->pic_start; cur; cur = cur->next) {
+        for (i=0; i < cur->nb_refs; ++i) {
+            if (cur->refs[i] == pic) {
+                av_log(avctx, AV_LOG_INFO, "Can't skip ref pic %"PRId64"/%"PRId64".\n",
+                       pic->display_order, pic->encode_order);
+                pic->skipped_flag = 0;
+                return 0;
+            }
+        }
+    }
+    return 0;
+}
+
 static int vaapi_encode_wait(AVCodecContext *avctx,
                              VAAPIEncodePicture *pic)
 {
@@ -412,6 +448,50 @@  static int vaapi_encode_issue(AVCodecContext *avctx,
         }
     }
 
+    err = vaapi_encode_check_if_skip(avctx, pic);
+    if (err != 0)
+        av_log(avctx, AV_LOG_ERROR, "Fail to check if skip.\n");
+
+#if VA_CHECK_VERSION(0,38,1)
+    if (pic->skipped_flag) {
+        av_log(avctx, AV_LOG_INFO, "Skip pic %"PRId64"/%"PRId64" as requested.\n",
+               pic->display_order, pic->encode_order);
+        ++ctx->skipped_pic_count;
+        pic->encode_issued = 1;
+        return 0;
+    } else if (ctx->skipped_pic_count > 0) {
+        VAEncMiscParameterBuffer *misc_param = NULL;
+        VAEncMiscParameterSkipFrame *skip_param = NULL;
+
+        misc_param = av_malloc(sizeof(VAEncMiscParameterBuffer) + sizeof(VAEncMiscParameterSkipFrame));
+        misc_param->type = (VAEncMiscParameterType)VAEncMiscParameterTypeSkipFrame;
+        skip_param = (VAEncMiscParameterSkipFrame *)misc_param->data;
+
+        skip_param->skip_frame_flag = 1;
+        skip_param->num_skip_frames = ctx->skipped_pic_count;
+        skip_param->size_skip_frames = 0;
+
+        err = vaapi_encode_make_param_buffer(avctx, pic,
+                  VAEncMiscParameterBufferType, (void *)misc_param,
+                  (sizeof(VAEncMiscParameterBuffer) +
+                  sizeof(VAEncMiscParameterSkipFrame)));
+
+        free(misc_param);
+
+        if (err < 0)
+            goto fail;
+
+        ctx->skipped_pic_count = 0;
+    }
+#else
+    if (pic->skipped_flag) {
+        av_log(avctx, AV_LOG_INFO, "Skip-frame isn't supported and pic %"PRId64"/%"PRId64" isn't skipped.\n",
+               pic->display_order, pic->encode_order);
+        pic->skipped_flag = 0;
+        ctx->skipped_pic_count = 0;
+    }
+#endif
+
     vas = vaBeginPicture(ctx->hwctx->display, ctx->va_context,
                          pic->input_surface);
     if (vas != VA_STATUS_SUCCESS) {
@@ -491,9 +571,23 @@  static int vaapi_encode_output(AVCodecContext *avctx,
     VAStatus vas;
     int err;
 
-    err = vaapi_encode_wait(avctx, pic);
-    if (err < 0)
-        return err;
+    if (!pic->skipped_flag) {
+        err = vaapi_encode_wait(avctx, pic);
+        if (err < 0)
+            return err;
+    } else {
+        av_frame_free(&pic->input_image);
+        pic->encode_complete = 1;
+        err = av_new_packet(pkt, 0);
+        if (err < 0)
+            goto fail;
+        pkt->pts = pic->pts;
+        av_buffer_unref(&pic->output_buffer_ref);
+        pic->output_buffer = VA_INVALID_ID;
+        av_log(avctx, AV_LOG_DEBUG, "Output 0 byte for pic %"PRId64"/%"PRId64".\n",
+           pic->display_order, pic->encode_order);
+        return 0;
+    }
 
     buf_list = NULL;
     vas = vaMapBuffer(ctx->hwctx->display, pic->output_buffer,
@@ -514,6 +608,9 @@  static int vaapi_encode_output(AVCodecContext *avctx,
             goto fail_mapped;
 
         memcpy(pkt->data, buf->buf, buf->size);
+
+        memset(buf->buf, 0, buf->size);
+        buf->size = 0;
     }
 
     if (pic->type == PICTURE_TYPE_IDR)
@@ -547,7 +644,12 @@  fail:
 static int vaapi_encode_discard(AVCodecContext *avctx,
                                 VAAPIEncodePicture *pic)
 {
-    vaapi_encode_wait(avctx, pic);
+    if (!pic->skipped_flag) {
+        vaapi_encode_wait(avctx, pic);
+    } else {
+        av_frame_free(&pic->input_image);
+        pic->encode_complete = 1;
+    }
 
     if (pic->output_buffer_ref) {
         av_log(avctx, AV_LOG_DEBUG, "Discard output for pic "
@@ -2034,6 +2136,8 @@  av_cold int ff_vaapi_encode_init(AVCodecContext *avctx)
         }
     }
 
+    ctx->skipped_pic_count = 0;
+
     return 0;
 
 fail:
diff --git a/libavcodec/vaapi_encode.h b/libavcodec/vaapi_encode.h
index a420686..bcbbd1a 100644
--- a/libavcodec/vaapi_encode.h
+++ b/libavcodec/vaapi_encode.h
@@ -112,6 +112,8 @@  typedef struct VAAPIEncodePicture {
 
     int          nb_slices;
     VAAPIEncodeSlice *slices;
+
+    uint8_t skipped_flag;
 } VAAPIEncodePicture;
 
 typedef struct VAAPIEncodeProfile {
@@ -270,6 +272,9 @@  typedef struct VAAPIEncodeContext {
     int idr_counter;
     int gop_counter;
     int end_of_stream;
+
+    // Skipped frame info
+    unsigned int skipped_pic_count;
 } VAAPIEncodeContext;
 
 enum {
diff --git a/libavutil/frame.c b/libavutil/frame.c
index dcf1fc3..1c38ebe 100644
--- a/libavutil/frame.c
+++ b/libavutil/frame.c
@@ -842,6 +842,7 @@  const char *av_frame_side_data_name(enum AVFrameSideDataType type)
 #endif
     case AV_FRAME_DATA_DYNAMIC_HDR_PLUS: return "HDR Dynamic Metadata SMPTE2094-40 (HDR10+)";
     case AV_FRAME_DATA_REGIONS_OF_INTEREST: return "Regions Of Interest";
+    case AV_FRAME_DATA_SKIP_FRAME: return "Skip frame";
     }
     return NULL;
 }
diff --git a/libavutil/frame.h b/libavutil/frame.h
index 8aa3e88..9022aba 100644
--- a/libavutil/frame.h
+++ b/libavutil/frame.h
@@ -179,6 +179,11 @@  enum AVFrameSideDataType {
      * array element is implied by AVFrameSideData.size / AVRegionOfInterest.self_size.
      */
     AV_FRAME_DATA_REGIONS_OF_INTEREST,
+
+    /**
+     * VAAPI Encode skip-frame indicator.
+     */
+    AV_FRAME_DATA_SKIP_FRAME,
 };
 
 enum AVActiveFormatDescription {