[FFmpeg-devel] [PATCH 1/1] avcodec/vaapi_encode: add frame-skip func
Mark Thompson
sw at jkqxz.net
Sun Dec 9 20:40:24 EET 2018
On 06/12/2018 10:04, Sun, Jing A wrote:
> Hi Mark,
>
> This patch is not trying to support VFR. Some frames, after which are just produced, could be considered as not needed by theirs producer and will get skipped in the encoding process. And in my opinion the existing timing information is not sufficient to support the case.
A skipped frame will still have a timestamp, so if a frame is skipped before the encoder then no frame with that timestamp will be given to the encoder. For CFR content this can be detected in the encoder to reconstruct your skip-frames information exactly - a skip has occurred if the gap between two frames does not match the framerate, and the size of the gap tells you how many frames were skipped. Avoiding a requirement that the gap sizes exactly match makes it implement a simple VFR scheme too, since skips can be inserted to keep the rate controller correct as long as you never have frames closer together than the configured framerate.
(Please avoid top-posting on this list.)
- Mark
> -----Original Message-----
> From: ffmpeg-devel [mailto:ffmpeg-devel-bounces at ffmpeg.org] On Behalf Of Mark Thompson
> Sent: Wednesday, December 5, 2018 7:50 AM
> To: ffmpeg-devel at ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH 1/1] avcodec/vaapi_encode: add frame-skip func
>
> On 04/12/2018 01:46, Sun, Jing A wrote:
>> Hi Mark,
>>
>> Is there any issue that I need to fix for this patch please?
>
> See comments in the message you quoted below? I think the most important point is that the existing timing information appears to contain all the information you want for VFR, so you probably shouldn't need the ad-hoc side-data type.
>
> - Mark
>
>
>> -----Original Message-----
>> From: ffmpeg-devel [mailto:ffmpeg-devel-bounces at ffmpeg.org] On Behalf
>> Of Sun, Jing A
>> Sent: Friday, November 23, 2018 5:37 PM
>> To: FFmpeg development discussions and patches
>> <ffmpeg-devel at ffmpeg.org>
>> Subject: Re: [FFmpeg-devel] [PATCH 1/1] avcodec/vaapi_encode: add
>> frame-skip func
>>
>> Hi Mark,
>>
>> In some cases, that is useful. For example, an online content distributer, who keeps encoding the captured video frames by ffmpeg and sending them out. At times, there is no update of source, which makes one or several captured source frames are exactly the same as the last one, and the distributer wants to just skip such frames, without stopping the encoding process.
>>
>> Regards,
>> SUN, Jing
>>
>>
>> -----Original Message-----
>> From: ffmpeg-devel [mailto:ffmpeg-devel-bounces at ffmpeg.org] On Behalf
>> Of Mark Thompson
>> Sent: Tuesday, November 20, 2018 4:07 AM
>> To: ffmpeg-devel at ffmpeg.org
>> Subject: Re: [FFmpeg-devel] [PATCH 1/1] avcodec/vaapi_encode: add
>> frame-skip func
>>
>> On 19/11/18 09:04, Jing SUN wrote:
>>> frame-skip is required to implement network bandwidth self-adaptive
>>> vaapi encoding.
>>> To make a frame skipped, allocate its frame side data of
>>> AV_FRAME_DATA_SKIP_FRAME type and set its value to 1.
>>
>> So if I'm reading this correctly the idea is to implement partial VFR by having a special new side-data type which indicates where frames would have been had the input actually matched the configured CFR behaviour?
>>
>> Why is the user meant to create these special frames? It seems to me that the existing method of looking at the timestamps would be a better way to find any gaps.
>>
>> (Or, even better, add timestamps to VAAPI so that it can support VFR
>> in a sensible way rather than adding hacks like this to allow partial
>> VFR with weird constraints.)
>>
>>> Signed-off-by: Jing SUN <jing.a.sun at intel.com>
>>> ---
>>> libavcodec/vaapi_encode.c | 142 ++++++++++++++++++++++++++++++++++++++++++++--
>>> libavcodec/vaapi_encode.h | 5 ++
>>> libavutil/frame.c | 1 +
>>> libavutil/frame.h | 5 ++
>>> 4 files changed, 149 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/libavcodec/vaapi_encode.c b/libavcodec/vaapi_encode.c
>>> index 2fe8501..a401d61 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,47 @@ 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) { @@ -418,6
>>> +460,69 @@ 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");
>>> +
>>> +#ifdef VAEncMiscParameterSkipFrame
>>> + 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) {
>>> + VABufferID skip_param_id;
>>> + VAEncMiscParameterBuffer *misc_param;
>>> + VAEncMiscParameterSkipFrame *skip_param;
>>> +
>>> + err = vaapi_encode_make_param_buffer(avctx, pic,
>>> + VAEncMiscParameterBufferType, NULL,
>>> + (sizeof(VAEncMiscParameterBuffer) +
>>> + sizeof(VAEncMiscParameterSkipFrame)));
>>> + if (err < 0)
>>> + goto fail;
>>> +
>>> + skip_param_id = pic->param_buffers[pic->nb_param_buffers-1];
>>> +
>>> + vas = vaMapBuffer(ctx->hwctx->display,
>>> + skip_param_id,
>>> + (void **)&misc_param);
>>> + if (vas != VA_STATUS_SUCCESS) {
>>> + av_log(avctx, AV_LOG_ERROR, "Failed to map skip-frame buffer: "
>>> + "%d (%s).\n", vas, vaErrorStr(vas));
>>> + err = AVERROR(EIO);
>>> + goto fail;
>>> + }
>>> +
>>> + misc_param->type =
>>> + (VAEncMiscParameterType)VAEncMiscParameterTypeSkipFrame;
>>
>> You need to check VAConfigAttribEncSkipFrame to make sure this type is actually supported before using it.
>>
>>> + 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;
>>> +
>>> + vas = vaUnmapBuffer(ctx->hwctx->display, skip_param_id);
>>> + if (vas != VA_STATUS_SUCCESS) {
>>> + av_log(avctx, AV_LOG_ERROR, "Failed to unmap skip-frame buffer: "
>>> + "%d (%s).\n", vas, vaErrorStr(vas));
>>> + err = AVERROR(EIO);
>>> + goto fail;
>>> + }
>>
>> make_param_buffer can be called with the intended data directly - there is no need for this map/unmap.
>>
>>> +
>>> + 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) {
>>> @@ -500,9 +605,28 @@ 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);
>>
>> Why is it helpful to create a zero-byte packet in this case? Since no frame was encoded, I think no packet should be produced.
>>
>>> +
>>> + return 0;
>>> + }
>>>
>>> buf_list = NULL;
>>> vas = vaMapBuffer(ctx->hwctx->display, pic->output_buffer, @@
>>> -523,6 +647,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;
>>
>> Seems unrelated?
>>
>>> }
>>>
>>> if (pic->type == PICTURE_TYPE_IDR) @@ -556,7 +683,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 "
>>> @@ -1994,6 +2126,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 965fe65..bcee0b6 100644
>>> --- a/libavcodec/vaapi_encode.h
>>> +++ b/libavcodec/vaapi_encode.h
>>> @@ -92,6 +92,8 @@ typedef struct VAAPIEncodePicture {
>>>
>>> int nb_slices;
>>> VAAPIEncodeSlice *slices;
>>> +
>>> + uint8_t skipped_flag;
>>> } VAAPIEncodePicture;
>>>
>>> typedef struct VAAPIEncodeProfile {
>>> @@ -246,6 +248,9 @@ typedef struct VAAPIEncodeContext {
>>> int gop_counter;
>>> int p_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
>>> 9b3fb13..c5fa205 100644
>>> --- a/libavutil/frame.c
>>> +++ b/libavutil/frame.c
>>> @@ -840,6 +840,7 @@ const char *av_frame_side_data_name(enum AVFrameSideDataType type)
>>> case AV_FRAME_DATA_QP_TABLE_PROPERTIES: return "QP table properties";
>>> case AV_FRAME_DATA_QP_TABLE_DATA: return "QP table data";
>>> #endif
>>> + case AV_FRAME_DATA_SKIP_FRAME: return "Skip frame";
>>> }
>>> return NULL;
>>> }
>>> diff --git a/libavutil/frame.h b/libavutil/frame.h index
>>> 66f27f4..8ef6475 100644
>>> --- a/libavutil/frame.h
>>> +++ b/libavutil/frame.h
>>> @@ -166,6 +166,11 @@ enum AVFrameSideDataType {
>>> * function in libavutil/timecode.c.
>>> */
>>> AV_FRAME_DATA_S12M_TIMECODE,
>>> +
>>> + /**
>>> + * VAAPI Encode skip-frame indicator.
>>> + */
>>> + AV_FRAME_DATA_SKIP_FRAME,
>>> };
>>>
>>> enum AVActiveFormatDescription {
>>>
More information about the ffmpeg-devel
mailing list