[FFmpeg-devel] [PATCH] libavcodec/qsvdec.c: Restoring decoding functionality after unsuccessful merge from libav.

[Ivan Uskov]

Hello Mark,

Saturday, July 23, 2016, 11:42:46 PM, you wrote:

MT> On 23/07/16 20:33, Ivan Uskov wrote:
>> If  you are use qsv, I would like to recommend to roll-back to version before
>> d30cf57a7b2097b565db02ecfffbdc9c16423d0e
>> Really  the  d30cf57a7b2097b565db02ecfffbdc9c16423d0e is useless and only
>> makes code complex and work slow, the sync variable is not mandatory to be
>> allocated  on  heap  at  all.  libav  guys  did a big mistake when have added
>> such "feature".

MT> Are you sure that works correctly with all streams, maybe one with some
MT> sort of delay?  It looks like you can add multiple things to the fifo in
MT> one call and but then only ever remove one, so using the stack to hold
MT> them across invocations is not appropriate (though failure may be very subtle).
Yes, I'm sure.
There are following arguments:
1. Intel's samples by itself use general variable for the 'sync' arg, not
heap allocated.
2. Intel documentation says nothing special for this arg.
3. I did several qsv-based transcoders for windows and linux no any issues
without this questionable trick.
4. A fifo object used previously to keep 'sync' handles already keeps data on
heap :-)

There only one known issue into initial implementation, the fifo size may be not
enough an there is no check for overflow. So at some cases wrapper may
overwrite sync entries in fifo and some decoded frames may be lost. But it is
not related with discussed problem.

MT> (Note that I'm not sufficiently familiar with Media SDK to confidently
MT> assert anything about how this code should behave, so please don't read too much into what I say.)

MT> In general I think if you are happy that the previous implementation in
MT> ffmpeg was correct then please do revert, because the code as it is now is completely broken.
Ok, will do.

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