James Almer:
> On 9/21/2021 7:18 PM, Andreas Rheinhardt wrote:
>> Currently, the AV_PKT_FLAG_KEY is automatically set for audio encoders;
>> yet this is wrong, as both MLP and TrueHD have non-keyframes. So set it
>> based upon AV_CODEC_PROP_INTRA_ONLY (from the corresponding
>> AVCodecDescriptor) instead. This also sets it for some video codecs,
>> which is intended.
>>
>> Signed-off-by: Andreas Rheinhardt <andreas.rheinha...@outlook.com>
>> ---
>> I was surprised that this did not necessitate FATE changes.
>>
>> Btw: AVCodecContext.codec_descriptor is always set by avcodec_open2(),
>> yet marked as unused for encoding in its doxy. Shall I make document
>> the current behaviour?
>>
>> libavcodec/encode.c | 7 +++----
>> libavcodec/internal.h | 7 +++++++
>> 2 files changed, 10 insertions(+), 4 deletions(-)
>>
>> diff --git a/libavcodec/encode.c b/libavcodec/encode.c
>> index 98dfbfdff3..dd25cf999b 100644
>> --- a/libavcodec/encode.c
>> +++ b/libavcodec/encode.c
>> @@ -235,12 +235,9 @@ static int encode_simple_internal(AVCodecContext
>> *avctx, AVPacket *avpkt)
>> }
>> }
>> if (avctx->codec->type == AVMEDIA_TYPE_AUDIO) {
>> - /* NOTE: if we add any audio encoders which output
>> non-keyframe packets,
>> - * this needs to be moved to the encoders, but for
>> now we can do it
>> - * here to simplify things */
>> - avpkt->flags |= AV_PKT_FLAG_KEY;
>> avpkt->dts = avpkt->pts;
>> }
>> + avpkt->flags |= avci->intra_only_flag;
>
> You could do the check you added to ff_encode_preinit() here, inside the
> audio media type check, instead of adding another single use field to
> AVCodecInternal.
>
That would add a dereference and a branch more for every packet; the
above avoids this.
>> }
>> if (avci->draining && !got_packet)
>> @@ -553,6 +550,8 @@ int ff_encode_preinit(AVCodecContext *avctx)
>> }
>> avctx->sw_pix_fmt = frames_ctx->sw_format;
>> }
>> + if (avctx->codec_descriptor->props & AV_CODEC_PROP_INTRA_ONLY)
>
> fwiw, the doxy for AVCodecContext says codec_descriptor is unused for
> encoding, but i see it's set unconditionally in avcodec_open2(). Should
> the doxy be amended?
>
You overlooked my above comment, I presume.
> It also kinda feels like a superfluous field. Anyone can do
> avcodec_descriptor_get(avctx->codec_id) if required.
>
Agreed.
>> + avctx->internal->intra_only_flag = AV_PKT_FLAG_KEY;
>> return 0;
>> }
>> diff --git a/libavcodec/internal.h b/libavcodec/internal.h
>> index dc60e4bf08..8df622968c 100644
>> --- a/libavcodec/internal.h
>> +++ b/libavcodec/internal.h
>> @@ -155,6 +155,13 @@ typedef struct AVCodecInternal {
>> uint8_t *byte_buffer;
>> unsigned int byte_buffer_size;
>> + /**
>> + * This is set to AV_PKT_FLAG_KEY for encoders that encode
>> intra-only
>> + * formats (i.e. whose codec descriptor has
>> AV_CODEC_PROP_INTRA_ONLY set).
>> + * This is used to set said flag generically for said encoders.
>> + */
>> + int intra_only_flag;
>> +
>> void *frame_thread_encoder;
>> EncodeSimpleContext es;
>>
>
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
