[FFmpeg-devel] [PATCH v3] avformat/mov: add option max_stts_delta

[Gyan Doshi]

On 2021-12-27 05:21 am, Michael Niedermayer wrote:
> On Sun, Dec 26, 2021 at 09:30:44PM +0530, Gyan Doshi wrote:
>> Very high stts sample deltas may occasionally be intended but usually
>> they are written in error or used to store a negative value for dts correction
>> when treated as signed 32-bit integers.
>>
>> This option lets the user set an upper limit, beyond which the delta is clamped to 1.
>> Values greater than the limit if negative when cast to int32 are used to adjust onward dts.
>>
>> Unit is the track time scale. Default is UINT_MAX - 48000*10 which
>> allows upto a 10 second dts correction for 48 kHz audio streams while
>> accommodating 99.9% of uint32 range.
>> ---
>> v3 changes:
>>
>> factored out loop
>> simplified correction logic
> this looks more sane now
> i guess this cannot be easily split into a seperate patch ?

No, all stts corrections depend on context of earlier corrections.


> [...]
>> @@ -2965,11 +2967,34 @@ static int mov_read_stts(MOVContext *c, AVIOContext *pb, MOVAtom atom)
>>           sc->stts_data[i].count= sample_count;
>>           sc->stts_data[i].duration= sample_duration;
>>   
>> -        av_log(c->fc, AV_LOG_TRACE, "sample_count=%d, sample_duration=%d\n",
>> +        av_log(c->fc, AV_LOG_TRACE, "sample_count=%u, sample_duration=%u\n",
>>                   sample_count, sample_duration);
>>   
>> -        duration+=(int64_t)sample_duration*(uint64_t)sample_count;
>> -        total_sample_count+=sample_count;
>> +        /* STTS sample offsets are uint32 but some files store it as int32
>> +         * with negative values used to correct DTS delays.
>> +           There may be abnormally large values as well. */
>> +        if (sample_duration > c->max_stts_delta) {
>> +            // assume high delta is a correction if negative when cast as int32
>> +            int32_t delta_magnitude = (int32_t)sample_duration;
>> +            av_log(c->fc, AV_LOG_WARNING, "Too large sample offset %u in stts entry %u with count %u in st:%d. Clipping to 1.\n",
>> +                   sample_duration, i, sample_count, st->index);
>> +            sc->stts_data[i].duration = 1;
>> +            corrected_dts += (delta_magnitude < 0 ? (int64_t)delta_magnitude : 1) * sample_count;
>> +        } else {
>> +            corrected_dts += sample_duration * sample_count;
>> +        }
>> +
>> +        current_dts += sc->stts_data[i].duration * sample_count;
>> +
>> +        if (current_dts > corrected_dts) {
>> +            int64_t drift = (current_dts - corrected_dts)/sample_count;
> division by 0

A sample count of 0 is nonsensical. Sent a separate patch for 0 values 
in stts. Will rebase this one on top.

Regards,
Gyan