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

[Anssi Hannula]

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.


> are these in intended / expected ?
> 
> I can confirm that this reduces the amount of data read in many files


-- 
Anssi Hannula