[FFmpeg-devel] [PATCH] ffmpeg: fix overriding packet duration warning

[Muhammad Faiz]

On Mon, Nov 16, 2015 at 8:27 AM, Michael Niedermayer
<michael at niedermayer.cc> wrote:
> On Mon, Nov 16, 2015 at 12:26:28AM +0700, Muhammad Faiz wrote:
>> On Sun, Nov 15, 2015 at 5:55 PM, Michael Niedermayer
>> <michael at niedermayer.cc> wrote:
>> > On Sun, Nov 15, 2015 at 04:04:42PM +0700, Muhammad Faiz wrote:
>> >> no warning when packet duration is valid
>> >>
>> >> patch attached
>> >
>> >> From 789e9f0e93a246fd820401e6c298835bf40dc0c3 Mon Sep 17 00:00:00 2001
>> >> From: Muhammad Faiz <mfcc64 at gmail.com>
>> >> Date: Sun, 15 Nov 2015 15:25:43 +0700
>> >> Subject: [PATCH] ffmpeg: fix overriding packet duration warning
>> >>
>> >> no warning when packet duration is valid
>> >> ---
>> >>  ffmpeg.c | 7 ++++---
>> >>  1 file changed, 4 insertions(+), 3 deletions(-)
>> >>
>> >> diff --git a/ffmpeg.c b/ffmpeg.c
>> >> index 3341777..5b9e38e 100644
>> >> --- a/ffmpeg.c
>> >> +++ b/ffmpeg.c
>> >> @@ -673,10 +673,11 @@ static void write_frame(AVFormatContext *s, AVPacket *pkt, OutputStream *ost)
>> >>          }
>> >>
>> >>          if (ost->frame_rate.num && ost->is_cfr) {
>> >> -            if (pkt->duration > 0)
>> >> +            int64_t new_duration = av_rescale_q(1, av_inv_q(ost->frame_rate),
>> >> +                                                ost->st->time_base);
>> >> +            if (pkt->duration > 0 && pkt->duration != new_duration)
>> >>                  av_log(NULL, AV_LOG_WARNING, "Overriding packet duration by frame rate, this should not happen\n");
>> >> -            pkt->duration = av_rescale_q(1, av_inv_q(ost->frame_rate),
>> >> -                                         ost->st->time_base);
>> >> +            pkt->duration = new_duration;
>> >
>> > does it work to leave the duration instead of overriding if its
>> > already set ?
>> > i mean the new cfr duration code is certainly not correct for many
>> > common cases (like 24000/1001 framerates at a 90khz timebase) so
>> > overriding things by it if something else already set a (more correct?)
>> > duration might be a bad idea
>>
>> It just change warning message, when actually pkt->duration is equal
>> to new_duration, no warning is generated. It does not change the
>> previous behavior.
>>
>
>> About overriding, don't ask me. I don't know for that. That is not from me.
>
> yes but you seem t have found some cases where this code triggers
> so i was wondering if you also saw cases where they differ and if
> so what values in that case are more correct
>
My patch about wrapped_avframe trigger it on yuv4mpegenc, because
it set packet duration based on frame duration (I think it is not
forbidden). Fortunately both duration don't differ.