[FFmpeg-devel] [PATCH] Pass pts values through non-delay encoders by default.

[Reimar Döffinger]

On 23 Jan 2012, at 04:46, Michael Niedermayer <michaelni at gmx.at> wrote:
> On Sun, Jan 22, 2012 at 02:39:52PM +0100, Reimar Döffinger wrote:
>> Avoids having to duplicate the code for trivial, non-reordering
>> encoders.
>> Completely and utterly breaks almost all H.264 conformance tests,
>> sometimes just making the pts start with negative values, sometimes
>> only giving 2 instead of 20 decoded frames or failing with not
>> correctly ordered pts.
>> 
>> Signed-off-by: Reimar Döffinger <Reimar.Doeffinger at gmx.de>
>> ---
>> libavcodec/utils.c |    6 ++++++
>> 1 files changed, 6 insertions(+), 0 deletions(-)
>> 
>> diff --git a/libavcodec/utils.c b/libavcodec/utils.c
>> index a505fa5..25ad3eb 100644
>> --- a/libavcodec/utils.c
>> +++ b/libavcodec/utils.c
>> @@ -1145,6 +1145,12 @@ int attribute_align_arg avcodec_encode_video(AVCodecContext *avctx, uint8_t *buf
>>         int ret = avctx->codec->encode(avctx, buf, buf_size, pict);
>>         avctx->frame_number++;
>>         emms_c(); //needed to avoid an emms_c() call before every return;
>> +        if (!(avctx->codec->capabilities & CODEC_CAP_DELAY)) {
>> +            if (!avctx->coded_frame)
>> +                avctx->coded_frame = pict;
> 
> iiiks
> coded_frame is supposed to be managed by the encoder, setting it from
> outside doesnt feel safe. (a failing malloc and its NULL and a free
> later and theres a double free just as one example)
> also theres a problem with the lifetime of the objects
> coded_frame should be valid till the next avcodec* call at least
> but the pict argument might become invalid sooner than that.

Sorry, but then the API is just idiotic. You do realise that what you describe means that the rawvideo encoder has to make a copy of the frame to behave correctly?
What is coded_frame used for anyway?

> its not as generic but i would suggest to move this logic into
> ffmpeg.c
> or at least limit the hacking to ->pts

Then every other application using FFmpeg needs to duplicate the same code.
I guess saying "null coded_frame means same pts in as out" might work.
(hacking only pts doesn't work I think, some codes rather randomly do or do not set coded_frame I had the impression. Rawvideo seems to set it, but with wrong pts, flashsv seems to not set it at all).