[FFmpeg-devel] QSV Decoding - Issues and Regressions

[Ivan Uskov]

Hello Michael, Hendrik,

I would like to make sure about following moment regarding handling of
sequence header changing. The decoder has buffering of decoded frames,
so when new frame dimensions are detected we can not just reset
decoder, we should to deffer re-init and switch decoder  to some
"flushing" state and still deliver "old" frames until buffer will
empty.
Main issue that upstream  still deliver new packets at this time. I
tried to return 0 from ff_qsv_decode() to indicate that packet not
consumed at all and should be re-send later but upstream does not
understand this at least if  I return non-zero 'got_frame' flag and
decoded frame. 

Looks like decoder should to buffer income packets at at flushing
state. Else decoder loses first gop after new sequence header detected.
Is it acceptable way? Any other alternatives. By my private opinion it
would be much better if upstream did not try to push new packet if 0
was returned from decoder together with decoded frame.

By the way, about old implementation which "worked fine".
It just did drop all buffered frames at decoder re-init on new
sequence header, there is nice comment inside old qsvdec_h264.c:
/* TODO: flush delayed frames on reinit */

Of course, I can provide same behavior for first time but it very ugly solution.



Monday, August 3, 2015, 3:44:17 PM, you wrote:

MN> On Mon, Aug 03, 2015 at 02:18:04PM +0200, Hendrik Leppkes wrote:
>> Hey,
>> 
>> after a discussion on IRC about the declining quality of the QSV
>> decoders, I decided to do actual tests and document the results, so we
>> can get these fixed.
>> 
>> A lot of these issues are regressions from recent changes, and work
>> fine in the original qsv decoders merged from libav, so they should be
>> given extra attention.
>> 
>> 1)
>> Content with frame size changes is not handled at all.
>> A change to a smaller frame size will result in image corruption, and
>> a change to a bigger frame size will cause decoding to fail entirely.
>> 
>> Sample (large to small):
>> http://fate-suite.ffmpeg.org/h264/reinit-large_420_8-to-small_420_8.h264
>> 
>> In this case, as hardware buffers are re-used, the images after the
>> change to a smaller size result in showing parts of the old image and
>> the new smaller image in the top left corner instead.
>> 
>> 2)
>> Seeking is broken. For some reason refactoring of the qsv decoders has
>> resulted in their flush functions to become void. Seeking using ffplay
>> and the qsv decoder results in serious playback disruption.
>> 
>> 3)
>> AV_PIX_FMT_QSV output is no longer functional. Refactoring has removed
>> the ff_get_format call, which results in API users no longer being
>> able to select the QSV pix fmt.
>> 
>> These 3 issues are regression from the original decoder, and should be
>> corrected before further decoders are merged. As additional code using
>> the common qsv may require further changes in the future, its best to
>> fix the common code before adding even more.
>> 

>> I'll make sure to review future qsv patches more actively to hopefully
>> avoid big regressions like these in the future as well. I have a
>> system setup now to test qsv, so that should be easier in the future.

MN> thanks, ill leave qsv review & patch apply to you.
MN> I had until recently not even a setup to build qsv as the stuff didnt
MN> like my main development box. And still dont have a setup to runtime
MN> test, just to build.

MN> [...]



-- 
Best regards,
 Ivan                            mailto:ivan.uskov at nablet.com