[FFmpeg-devel] [PATCH] avformat/utils: Fix find_stream_info not considering the extradata it found

Anssi Hannula anssi.hannula at iki.fi
Thu Jul 28 01:27:38 EEST 2016


27.07.2016, 02:04, Michael Niedermayer kirjoitti:
> On Tue, Jul 26, 2016 at 08:39:03PM +0300, Anssi Hannula wrote:
>> 26.07.2016, 17:59, Michael Niedermayer kirjoitti:
>>> On Tue, Jul 26, 2016 at 03:56:13PM +0300, Anssi Hannula wrote:
>>>> Commit 9200514ad8717c6 ("lavf: replace AVStream.codec with
>>>> AVStream.codecpar") merged in commit 6f69f7a8bf6a0d01 changed
>>>> avformat_find_stream_info() to put the extradata it got from
>>>> st->parser->parser->split() to st->internal->avctx instead of st->codec
>>>> (from where it will be later copied to st->codecpar).
>>>>
>>>> However, in the same function, the "is stream ready?" check was changed
>>>> to check for extradata in st->codecpar instead of st->codec.
>>>>
>>>> Extradata retrieved from split() is therefore not considered anymore,
>>>> and avformat_find_stream_info() will therefore needlessly continue
>>>> probing in some cases.
>>>>
>>>> Fix that by checking for the extradata at st->internal->avctx where it
>>>> is actually put.
>>>>
>>>> ---
>>>>
>>>> Michael Niedermayer wrote:
>>>>> seems to break fate here:
>>>> [...]
>>>>
>>>> Oops, seems I messed up running fate and missed the "warning: only a
>>>> subset of the fate tests will be run because SAMPLES is not specified"
>>>> warning it gave... Thanks for catching that.
>>>>
>>>> Seems this reverse fix is actually needed, as extradata is actually
>>>> copied in the other direction (from st->internal->avctx to
>>>> st->codecpar).
>>>
>>> it seems this causes some changes, quite possibly thats simply due to
>>> less packets being read
>>>
>>> libavformat/tests/seek Matrix.Reloaded.Trailer-640x346-XviD-1.0beta2-HE_AAC_subtitled.mkv -duration 400
>>> with http://samples.ffmpeg.org/Matroska/matrix/Matrix.Reloaded.Trailer-640x346-XviD-1.0beta2-HE_AAC_subtitled.mkv
>>
>> @@ -9,7 +9,7 @@
>>  ret: 0         st:13 flags:1 dts: 86.750000 pts: 86.750000 pos:     -1
>> size: 50436
>>  ret:-1         st: 1 flags:0  ts: 250.577000
>>  ret: 0         st: 1 flags:1  ts: 13.471000
>> -ret: 0         st:13 flags:1 dts: 5.909000 pts: 5.909000 pos:     -1
>> size: 50436
>> +ret: 0         st:13 flags:1 dts: 1.776000 pts: 1.776000 pos:     -1
>> size: 50436
>>  ret:-1         st: 2 flags:0  ts: 176.365000
>>  ret: 0         st: 2 flags:1  ts: 339.259000
>>  ret: 0         st:13 flags:1 dts: 145.080000 pts: 145.080000 pos:
>> -1 size: 50436
>>
>> Seems to indeed be due to less packets being read in
>> avformat_find_stream_info().
>>
>> Matroska demuxer seems to add keyframes to the index as it encounters
>> them, and in this case more keyframes are encountered in
>> avformat_stream_info() phase, which seems to result in more accurate
>> seeking in the early seconds of the file.
>>
>>> for tickets//2254/ttvHD_vlc_sample.ts the tbr changes from
>>> 1001/30000 to 1/30
>>
>> Looks like the timestamps are jittery in that sample:
>>
>> 36072.231911, dts = prev + 3002
>> 36072.265289, dts = prev + 3004
>> 36072.298656, dts = prev + 3003
>> 36072.331433, dts = prev + 2950
>> 36072.364800, dts = prev + 3003
>> 36072.398167, dts = prev + 3003
>> 36072.431533, dts = prev + 3003
>> 36072.464900, dts = prev + 3003
>> 36072.498267, dts = prev + 3003
>> 36072.531633, dts = prev + 3003
>> 36072.565000, dts = prev + 3003
>> 36072.598367, dts = prev + 3003
>> 36072.630667, dts = prev + 2907
>> 36072.664033, dts = prev + 3003
>> 36072.697400, dts = prev + 3003
>> 36072.730767, dts = prev + 3003
>> 36072.764133, dts = prev + 3003
>> 36072.797500, dts = prev + 3003
>> 36072.830867, dts = prev + 3003
>> 36072.864233, dts = prev + 3003
>> 36072.897600, dts = prev + 3003
>> 36072.939278, dts = prev + 3751
>> 36072.972644, dts = prev + 3003
>> 36073.006011, dts = prev + 3003
>> 36073.034478, dts = prev + 2562
>> 36073.067844, dts = prev + 3003
>>
>> And this throws the ff_rfps_calculate() algorithm off unless it is
>> called with fpsprobesize (fps_analyze_framecount) of 160 or more
>> (default is 20).
>>
>> Not sure if something could/should be done to keep tbr of that file to
>> be detected as 1001/3000.
>>
>>
>> For both of these cases I verified that FFmpeg 3.0 had the same behavior
>> as with this patch, i.e. their behavior was unintendedly changed by the
>> original commit 6f69f7a8bf6a0d01.
> 
> thanks alot for the deep analysis
> i have no objections to the change
> 
> 
> [...]

Applied.


-- 
Anssi Hannula


More information about the ffmpeg-devel mailing list