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

[Ivan Uskov]

Hello Mark,

Friday, July 15, 2016, 1:37:54 PM, you wrote:

MT> On 15/07/16 07:15, Chao Liu wrote:
>> Ivan Uskov <ivan.uskov <at> nablet.com> writes:
>> 
>>>
>>> Hello All,
>>>
>>> After   commit  d30cf57a7b2097b565db02ecfffbdc9c16423d0e  qsv-based  
>> decoding
>>> aborts  with  crash,  there  are many incorrect places appeared. The 
>> attached
>>> patch fixes the issues but keeps new method of the 'sync' variable 
>> allocation
>>> introduced in commit d30cf57a7b2097b565db02ecfffbdc9c16423d0e.
>>>
>>> Please review.
>>>
>> 
>> I had the same crashes. After reading the code, you are certainly right.
>> Why nobody review this commit?

MT> Presumably noone was particularly interested at the time, and the submitter did
MT> not pursue it.

MT> Looking at it now, the change looks mostly ok to me.  The error paths could
MT> maybe be cleaned up a bit, but I think that's mostly a preexisting problem.  Can
MT> we loop without *sync being set?  If so, removing the av_freep(&sync); inside
MT> the loop makes it leak in that case.

MT> A slightly clearer commit message might help too.  Maybe something like:

MT> ---
MT> lavc/qsvdec: Fix decoding following incorrect merge

MT> Decoding was broken by d30cf57a7b2097b565db02ecfffbdc9c16423d0e - the
MT> merge didn't properly handle the sync pointers, so it always
MT> segfaulted after submitting a frame to libmfx.
MT> ---

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".



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