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.us...@nablet.com _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
