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

[Michael Niedermayer]

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


[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Into a blind darkness they enter who follow after the Ignorance,
they as if into a greater darkness enter who devote themselves
to the Knowledge alone. -- Isha Upanishad
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20160727/d56266dc/attachment.sig>