[FFmpeg-devel] [PATCH 3/5] lavc/libkvazaar: export encoded frame stats

[James Almer]

On 8/15/2020 5:48 AM, mypopy at gmail.com wrote:
> On Sun, Aug 9, 2020 at 6:07 AM Mark Thompson <sw at jkqxz.net> wrote:
>>
>> On 26/07/2020 13:26, Jun Zhao wrote:
>>> From: Jun Zhao <barryjzhao at tencent.com>
>>>
>>> Export choosen pict_type and qp.
>>>
>>> Signed-off-by: Jun Zhao <barryjzhao at tencent.com>
>>> ---
>>>   libavcodec/libkvazaar.c | 30 ++++++++++++++++++++++++++++++
>>>   1 file changed, 30 insertions(+)
>>>
>>> diff --git a/libavcodec/libkvazaar.c b/libavcodec/libkvazaar.c
>>> index 71c9c8f..9032547 100644
>>> --- a/libavcodec/libkvazaar.c
>>> +++ b/libavcodec/libkvazaar.c
>>> @@ -37,6 +37,7 @@
>>>
>>>   #include "avcodec.h"
>>>   #include "internal.h"
>>> +#include "packet_internal.h"
>>>
>>>   typedef struct LibkvazaarContext {
>>>       const AVClass *class;
>>> @@ -170,6 +171,7 @@ static int libkvazaar_encode(AVCodecContext *avctx,
>>>       kvz_data_chunk *data_out = NULL;
>>>       uint32_t len_out = 0;
>>>       int retval = 0;
>>> +    int pict_type;
>>>
>>>       *got_packet_ptr = 0;
>>>
>>> @@ -257,6 +259,34 @@ static int libkvazaar_encode(AVCodecContext *avctx,
>>>               avpkt->flags |= AV_PKT_FLAG_KEY;
>>>           }
>>>
>>> +        switch (frame_info.slice_type) {
>>> +        case KVZ_SLICE_I:
>>> +            pict_type = AV_PICTURE_TYPE_I;
>>> +            break;
>>> +        case KVZ_SLICE_P:
>>> +            pict_type = AV_PICTURE_TYPE_P;
>>> +            break;
>>> +        case KVZ_SLICE_B:
>>> +            pict_type = AV_PICTURE_TYPE_B;
>>> +            break;
>>> +        default:
>>> +            av_log(avctx, AV_LOG_ERROR, "Unknown picture type encountered.\n");
>>> +            return AVERROR_EXTERNAL;
>>> +        }
>>> +#if FF_API_CODED_FRAME
>>> +FF_DISABLE_DEPRECATION_WARNINGS
>>> +        avctx->coded_frame->pict_type = pict_type;
>>> +FF_ENABLE_DEPRECATION_WARNINGS
>>> +#endif
>>
>> Is there some value to setting the deprecated fields?  They will disappear on the next version bump, which probably isn't very far away.
>>
> I think we can keep this part, if we want to remove the
> avctx->coded_frame->pict_type from next version bump, we can drop this
> part from all codec one-time

No, the usage of deprecated features should not be expanded. It's kept
in old encoders for compatibility reasons, but new ones should not use
it at all. So please remove it.

>>> +
>>> +        ff_side_data_set_encoder_stats(avpkt, frame_info.qp * FF_QP2LAMBDA, NULL, 0, pict_type);
>>
>> I'm not familiar with kvazaar - is the QP value here actually reflective of the global quality in a useful way?  The documentation is not very good...
>>
> Yes, it's a global quality based on Frame in kvazaar
> 
>> (Your following patch for OpenH264 uses a field "AverageFrameQP", which has an much more usefully-suggestive name.)
>>
>> Zero is a valid QP value, but you shouldn't be passing it here.  Possibly it needs some more scaling to get the range to [1, FF_LAMDBA_MAX].
>>
> I know vpx encoder wrapper used the zero QP value in side data, maybe
> we can keep the same action
>>> +
>>> +#if FF_API_CODED_FRAME
>>> +FF_DISABLE_DEPRECATION_WARNINGS
>>> +        avctx->coded_frame->quality = frame_info.qp * FF_QP2LAMBDA;
>>> +FF_ENABLE_DEPRECATION_WARNINGS
>>> +#endif
>>> +
>>>           *got_packet_ptr = 1;
>>>       }
>>>
>>>
>> - Mark
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
>