[FFmpeg-devel] [PATCH v2 5/8] ffmpeg: use new decode API

[Michael Niedermayer]

On Thu, Mar 24, 2016 at 03:34:58PM +0100, wm4 wrote:
> On Wed, 23 Mar 2016 18:31:56 +0100
> Michael Niedermayer <michael at niedermayer.cc> wrote:
> 
> > On Wed, Mar 23, 2016 at 02:02:12PM +0100, wm4 wrote:
> > > This is a bit messy, mainly due to timestamp handling.
> > > 
> > > decode_video() relied on the fact that it could set dts on a flush/drain
> > > packet. This is not possible with the old API, and won't be. (I think
> > > doing this was very questionable with the old API. Flush packets should
> > > not contain any information; they just cause a FIFO to be emptied.) This
> > > is replaced with checking the best_effort_timestamp for AV_NOPTS_VALUE,
> > > and using the suggested DTS in the drain case.
> > > 
> > > The fate-cavs test still fails due to dropping the last frame. This
> > > happens because the timestamp of the last frame goes backwards
> > > (ffprobe -show_frames shows the same thing). I suspect that this
> > > "worked" due to the best effort timestamp logic picking the DTS
> > > over the decreasing PTS. Since this logic is in libavcodec (where
> > > it probably shouldn't be), this can't be easily fixed. The timestamps
> > > of the cavs samples are weird anyway, so I chose not to fix it.
> > > 
> > > Another strange thing is the timestamp handling in the video path of
> > > process_input_packet (after the decode_video() call). It looks like
> > > the code to increase next_dts and next_pts should be run every time
> > > a frame is decoded - but it's needed even if output is skipped.
> > > ---
> > >  ffmpeg.c            | 126 +++++++++++++++++++++++++++++++---------------------
> > >  tests/ref/fate/cavs |   1 -
> > >  2 files changed, 75 insertions(+), 52 deletions(-)  
> > 
> > this does not pass fate
> > make -j12 fate -k THREAD_TYPE=frame THREADS=5
> > passes before this commit but fails afterwards
> > 
> > make: *** [fate-filter-hq2x] Error 1
> > make: *** [fate-filter-3xbr] Error 1
> > make: *** [fate-filter-hq4x] Error 1
> > make: *** [fate-filter-4xbr] Error 1
> > make: *** [fate-filter-2xbr] Error 1
> > make: *** [fate-filter-hq3x] Error 1
> > make: *** [fate-h264-reinit-large_420_8-to-small_420_8] Error 1
> > make: *** [fate-h264-reinit-small_420_9-to-small_420_8] Error 1
> > make: *** [fate-h264-reinit-small_420_8-to-large_444_10] Error 1
> > make: *** [fate-h264-reinit-small_422_9-to-small_420_9] Error 1
> > make: *** [fate-hevc-paramchange-yuv420p-yuv420p10] Error 1
> > 
> > [...]
> 
> It's the timestamp insanity again. Patch: http://sprunge.us/AYgA
> 
> The hqx/xbr checks fail because somehow it doesn't make sure it
> drains the filter graph on parameter changes. The frames sent to the
> filter graph are exactly the same before and after my change. I don't
> know why it gets the the output frame soon enough out the graph before
> my patches. The solution is not running these tests with threading
> enabled, or to fix filter draining on parameter changes. At least it
> looks to me like it's broken in the general case and before my patches,
> and it worked by luck.

it seems with this patch (with and withut the update above) sometimes
more frames are passed through the filter chain than before

is that intended ?

example: (see the number of lines generated from cropdetect)
./ffmpeg -i ~/tickets/3030/cropdetect.mp4 -vframes 15 -an  -vf "cropdetect" -f null -

the actual number of frames written to the output does not change
though

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The misfortune of the wise is better than the prosperity of the fool.
-- Epicurus
-------------- 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/20160324/e67ee6ff/attachment.sig>