diff mbox

[FFmpeg-devel] amfenc: Add support for pict_type field

Message ID 20190114190220.16236-1-info@xaymar.com
State New
Headers show

Commit Message

Michael Fabian 'Xaymar' Dirks Jan. 14, 2019, 7:02 p.m. UTC 
Adds support for the pict_type field in AVFrame to amf_h264 and amf_h265 simultaneously. This field is needed in cases where the application wishes to override the frame type with another one, such as forcefully inserting a key frame for chapter markers or similar.

Additionally this abuses AV_PICTURE_TYPE_S for marking Skip frames, a special type of frame in AVC, SVC and HEVC which is a flag for the decoder to repeat the last frame.

Signed-off-by: Michael Fabian 'Xaymar' Dirks <info@xaymar.com>
---
 libavcodec/amfenc.c | 46 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 46 insertions(+)

Comments

Carl Eugen Hoyos Jan. 14, 2019, 7:32 p.m. UTC | #1 
2019-01-14 20:02 GMT+01:00, Michael Fabian 'Xaymar' Dirks <info@xaymar.com>:
> Adds support for the pict_type field in AVFrame to amf_h264 and amf_h265
> simultaneously. This field is needed in cases where the application wishes
> to override the frame type with another one, such as forcefully inserting a
> key frame for chapter markers or similar.
>
> Additionally this abuses AV_PICTURE_TYPE_S for marking Skip frames, a
> special type of frame in AVC, SVC and HEVC which is a flag for the decoder
> to repeat the last frame.
>
> Signed-off-by: Michael Fabian 'Xaymar' Dirks <info@xaymar.com>
> ---
>  libavcodec/amfenc.c | 46 +++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 46 insertions(+)
>
> diff --git a/libavcodec/amfenc.c b/libavcodec/amfenc.c
> index 384d8efc92..eb4b65e4f2 100644
> --- a/libavcodec/amfenc.c
> +++ b/libavcodec/amfenc.c
> @@ -680,6 +680,52 @@ int ff_amf_send_frame(AVCodecContext *avctx, const
> AVFrame *frame)
>              break;
>          }
>
> +        // Override Picture Type for Frame
> +        if (avctx->codec->id) {
> +            switch (frame->pict_type) {
> +            case AV_PICTURE_TYPE_I:
> +                AMF_ASSIGN_PROPERTY_INT64(res, surface,
> AMF_VIDEO_ENCODER_FORCE_PICTURE_TYPE, AMF_VIDEO_ENCODER_PICTURE_TYPE_I);
> +                break;
> +            case AV_PICTURE_TYPE_P:
> +                AMF_ASSIGN_PROPERTY_INT64(res, surface,
> AMF_VIDEO_ENCODER_FORCE_PICTURE_TYPE, AMF_VIDEO_ENCODER_PICTURE_TYPE_P);
> +                break;
> +            case AV_PICTURE_TYPE_B:
> +                AMF_ASSIGN_PROPERTY_INT64(res, surface,
> AMF_VIDEO_ENCODER_FORCE_PICTURE_TYPE, AMF_VIDEO_ENCODER_PICTURE_TYPE_B);
> +                break;
> +            case AV_PICTURE_TYPE_S:
> +                AMF_ASSIGN_PROPERTY_INT64(res, surface,
> AMF_VIDEO_ENCODER_FORCE_PICTURE_TYPE, AMF_VIDEO_ENCODER_PICTURE_TYPE_SKIP);
> +                break;
> +            default:
> +                AMF_ASSIGN_PROPERTY_INT64(res, surface,
> AMF_VIDEO_ENCODER_FORCE_PICTURE_TYPE, AMF_VIDEO_ENCODER_PICTURE_TYPE_NONE);
> +                break;
> +            }
> +            // Keyframe overrides previous assignment.
> +            if (frame->key_frame) {
> +                AMF_ASSIGN_PROPERTY_INT64(res, surface,
> AMF_VIDEO_ENCODER_FORCE_PICTURE_TYPE, AMF_VIDEO_ENCODER_PICTURE_TYPE_IDR);
> +            }

> +        } else if (avctx->codec->id == AV_CODEC_ID_HEVC) {

How can this be reached?
(Assuming you tested your patch, is the block unneeded?)

Carl Eugen
Michael Fabian 'Xaymar' Dirks Jan. 14, 2019, 8:15 p.m. UTC | #2 
On 14.01.2019 20:32, Carl Eugen Hoyos wrote:
> 2019-01-14 20:02 GMT+01:00, Michael Fabian 'Xaymar' Dirks <info@xaymar.com>:
>> Adds support for the pict_type field in AVFrame to amf_h264 and amf_h265
>> simultaneously. This field is needed in cases where the application wishes
>> to override the frame type with another one, such as forcefully inserting a
>> key frame for chapter markers or similar.
>>
>> Additionally this abuses AV_PICTURE_TYPE_S for marking Skip frames, a
>> special type of frame in AVC, SVC and HEVC which is a flag for the decoder
>> to repeat the last frame.
>>
>> Signed-off-by: Michael Fabian 'Xaymar' Dirks <info@xaymar.com>
>> ---
>>   libavcodec/amfenc.c | 46 +++++++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 46 insertions(+)
>>
>> diff --git a/libavcodec/amfenc.c b/libavcodec/amfenc.c
>> index 384d8efc92..eb4b65e4f2 100644
>> --- a/libavcodec/amfenc.c
>> +++ b/libavcodec/amfenc.c
>> @@ -680,6 +680,52 @@ int ff_amf_send_frame(AVCodecContext *avctx, const
>> AVFrame *frame)
>>               break;
>>           }
>>
>> +        // Override Picture Type for Frame
>> +        if (avctx->codec->id) {
>> +            switch (frame->pict_type) {
>> +            case AV_PICTURE_TYPE_I:
>> +                AMF_ASSIGN_PROPERTY_INT64(res, surface,
>> AMF_VIDEO_ENCODER_FORCE_PICTURE_TYPE, AMF_VIDEO_ENCODER_PICTURE_TYPE_I);
>> +                break;
>> +            case AV_PICTURE_TYPE_P:
>> +                AMF_ASSIGN_PROPERTY_INT64(res, surface,
>> AMF_VIDEO_ENCODER_FORCE_PICTURE_TYPE, AMF_VIDEO_ENCODER_PICTURE_TYPE_P);
>> +                break;
>> +            case AV_PICTURE_TYPE_B:
>> +                AMF_ASSIGN_PROPERTY_INT64(res, surface,
>> AMF_VIDEO_ENCODER_FORCE_PICTURE_TYPE, AMF_VIDEO_ENCODER_PICTURE_TYPE_B);
>> +                break;
>> +            case AV_PICTURE_TYPE_S:
>> +                AMF_ASSIGN_PROPERTY_INT64(res, surface,
>> AMF_VIDEO_ENCODER_FORCE_PICTURE_TYPE, AMF_VIDEO_ENCODER_PICTURE_TYPE_SKIP);
>> +                break;
>> +            default:
>> +                AMF_ASSIGN_PROPERTY_INT64(res, surface,
>> AMF_VIDEO_ENCODER_FORCE_PICTURE_TYPE, AMF_VIDEO_ENCODER_PICTURE_TYPE_NONE);
>> +                break;
>> +            }
>> +            // Keyframe overrides previous assignment.
>> +            if (frame->key_frame) {
>> +                AMF_ASSIGN_PROPERTY_INT64(res, surface,
>> AMF_VIDEO_ENCODER_FORCE_PICTURE_TYPE, AMF_VIDEO_ENCODER_PICTURE_TYPE_IDR);
>> +            }
>> +        } else if (avctx->codec->id == AV_CODEC_ID_HEVC) {
> How can this be reached?
> (Assuming you tested your patch, is the block unneeded?)
>
> Carl Eugen
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Thanks for pointing that out Carl, I forgot to re-run git format-patch before sending off the patch. The correct code should be:

> +        if (avctx->codec->id) {

should actually be

> +        if (avctx->codec->id == AV_CODEC_ID_H264) {

Should I resubmit with the corrected patch file?

Unfortunately I am currently only able to test the H.264 encoder, as 
initializing the H.265 encoder results in the Driver crashing with and 
without this patch applied. Assuming that ffmpeg doesn't do something 
weird with the AMF runtime, this behaviour should be working as is 
(similar code exists in obs-amd-encoder, the AMF integration for 
obs-studio).

Michael Fabian Dirks
diff mbox

Patch

diff --git a/libavcodec/amfenc.c b/libavcodec/amfenc.c
index 384d8efc92..eb4b65e4f2 100644
--- a/libavcodec/amfenc.c
+++ b/libavcodec/amfenc.c
@@ -680,6 +680,52 @@  int ff_amf_send_frame(AVCodecContext *avctx, const AVFrame *frame)
             break;
         }
 
+        // Override Picture Type for Frame
+        if (avctx->codec->id) {
+            switch (frame->pict_type) {
+            case AV_PICTURE_TYPE_I:
+                AMF_ASSIGN_PROPERTY_INT64(res, surface, AMF_VIDEO_ENCODER_FORCE_PICTURE_TYPE, AMF_VIDEO_ENCODER_PICTURE_TYPE_I);
+                break;
+            case AV_PICTURE_TYPE_P:
+                AMF_ASSIGN_PROPERTY_INT64(res, surface, AMF_VIDEO_ENCODER_FORCE_PICTURE_TYPE, AMF_VIDEO_ENCODER_PICTURE_TYPE_P);
+                break;
+            case AV_PICTURE_TYPE_B:
+                AMF_ASSIGN_PROPERTY_INT64(res, surface, AMF_VIDEO_ENCODER_FORCE_PICTURE_TYPE, AMF_VIDEO_ENCODER_PICTURE_TYPE_B);
+                break;
+            case AV_PICTURE_TYPE_S:
+                AMF_ASSIGN_PROPERTY_INT64(res, surface, AMF_VIDEO_ENCODER_FORCE_PICTURE_TYPE, AMF_VIDEO_ENCODER_PICTURE_TYPE_SKIP);
+                break;
+            default:
+                AMF_ASSIGN_PROPERTY_INT64(res, surface, AMF_VIDEO_ENCODER_FORCE_PICTURE_TYPE, AMF_VIDEO_ENCODER_PICTURE_TYPE_NONE);
+                break;
+            }
+            // Keyframe overrides previous assignment.
+            if (frame->key_frame) {
+                AMF_ASSIGN_PROPERTY_INT64(res, surface, AMF_VIDEO_ENCODER_FORCE_PICTURE_TYPE, AMF_VIDEO_ENCODER_PICTURE_TYPE_IDR);
+            }
+        } else if (avctx->codec->id == AV_CODEC_ID_HEVC) {
+            switch (frame->pict_type) {
+            case AV_PICTURE_TYPE_I:
+                AMF_ASSIGN_PROPERTY_INT64(res, surface, AMF_VIDEO_ENCODER_HEVC_FORCE_PICTURE_TYPE, AMF_VIDEO_ENCODER_HEVC_PICTURE_TYPE_I);
+                break;
+            case AV_PICTURE_TYPE_P:
+                AMF_ASSIGN_PROPERTY_INT64(res, surface, AMF_VIDEO_ENCODER_HEVC_FORCE_PICTURE_TYPE, AMF_VIDEO_ENCODER_HEVC_PICTURE_TYPE_P);
+                break;
+            case AV_PICTURE_TYPE_B:
+                av_log(ctx, AV_LOG_WARNING, "Ignoring B-Frame, unsupported by AMD AMF H.265 Encoder.");
+                break;
+            case AV_PICTURE_TYPE_S:
+                AMF_ASSIGN_PROPERTY_INT64(res, surface, AMF_VIDEO_ENCODER_HEVC_FORCE_PICTURE_TYPE, AMF_VIDEO_ENCODER_HEVC_PICTURE_TYPE_SKIP);
+                break;
+            default:
+                AMF_ASSIGN_PROPERTY_INT64(res, surface, AMF_VIDEO_ENCODER_HEVC_FORCE_PICTURE_TYPE, AMF_VIDEO_ENCODER_HEVC_PICTURE_TYPE_NONE);
+                break;
+            }
+            // Keyframe overrides previous assignment.
+            if (frame->key_frame) {
+                AMF_ASSIGN_PROPERTY_INT64(res, surface, AMF_VIDEO_ENCODER_HEVC_FORCE_PICTURE_TYPE, AMF_VIDEO_ENCODER_HEVC_PICTURE_TYPE_IDR);
+            }
+        }
 
         // submit surface
         res = ctx->encoder->pVtbl->SubmitInput(ctx->encoder, (AMFData*)surface);