[FFmpeg-devel] [PATCH] movenc: calculate track_duration without packet duration

[Gyan]

On 18-06-2019 02:00 PM, Alfred E. Heggestad wrote:
> On 17/06/2019 10:23, Gyan wrote:
>>
>>
>> On 17-06-2019 01:37 PM, Alfred E. Heggestad wrote:
>>> From c69b63a7af5531257753754e64ac33b7ef530e75 Mon Sep 17 00:00:00 2001
>>> From: "Alfred E. Heggestad" <alfred.heggestad at gmail.com>
>>> Date: Mon, 17 Jun 2019 10:04:08 +0200
>>> Subject: [PATCH] movenc: calculate track_duration without packet 
>>> duration
>>>
>>> ---
>>>  libavformat/movenc.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/libavformat/movenc.c b/libavformat/movenc.c
>>> index 46d314ff17..fa5833962b 100644
>>> --- a/libavformat/movenc.c
>>> +++ b/libavformat/movenc.c
>>> @@ -5486,7 +5486,7 @@ int ff_mov_write_packet(AVFormatContext *s, 
>>> AVPacket *pkt)
>>>                     "this case.\n",
>>>                     pkt->stream_index, pkt->dts);
>>>      }
>>> -    trk->track_duration = pkt->dts - trk->start_dts + pkt->duration;
>>> +    trk->track_duration = pkt->dts - trk->start_dts;
>>
>> Why?
>>
>>> trk->last_sample_is_subtitle_end = 0;
>>>
>>>      if (pkt->pts == AV_NOPTS_VALUE) {
>>
>
> the background for this change is the check done in check_pkt():
>
>    if (trk->entry) {
>         ref = trk->cluster[trk->entry - 1].dts;
>     } else if (   trk->start_dts != AV_NOPTS_VALUE
>                && !trk->frag_discont) {
>         ref = trk->start_dts + trk->track_duration;
>     } else
>         ref = pkt->dts; // Skip tests for the first packet
>
>     if (trk->dts_shift != AV_NOPTS_VALUE) {
>         /* With negative CTS offsets we have set an offset to the DTS,
>          * reverse this for the check. */
>         ref -= trk->dts_shift;
>     }
>
>     duration = pkt->dts - ref;
>     if (pkt->dts < ref || duration >= INT_MAX) {
>         av_log(s, AV_LOG_ERROR, "Application provided duration: 
> %"PRId64" / timestamp: %"PRId64" is out of range for mov/mp4 format\n",
>             duration, pkt->dts
>         );
>
>         pkt->dts = ref + 1;
>         pkt->pts = AV_NOPTS_VALUE;
>     }
>
>
>
> it requires that the DTS of the packet is larger than
> "ref" which in some cases is equal to track_duration.
>
>
> line 5015:
>
>      track->track_duration = pkt.dts - track->start_dts;
>
> line 5489:
>
>      trk->track_duration = pkt->dts - trk->start_dts + pkt->duration;
>
>
> line 5615:
>
>      trk->track_duration = pkt->dts - trk->start_dts;
>
>
>
> setting track_duration is inconsistent; some times it includes
> duration and some times not.

It may be best to check the commits for these assignments to see if the 
inconsistency is deliberate.
The track duration is written into the media header box for the track. I 
also see it being used elsewhere to adjust dts. Do those roles remain 
intact?

Does FATE pass?

Gyan

> here is an example with a stream of AVPacket's that increment
> the DTS by approx. 10 (and duration is set to 0):
>
> pkt:  dts:
>  1     0
>  2    10
>  3    20
>  4    30
>  5    40
>  6    50
>
>
> after packet 6 has arrived, the "ref" value in check_pkt will now
> be calculated to 50 + 10 = 60. the code assumes a duration of 10.
>
> when packet 7 arrives it has a DTS of 59 (which is valid).
> but this packet will fail in check_pkt and print a warning,
> because 59 < 60.