Hi, On Mon, Nov 13, 2017 at 11:06 PM, Thilo Borgmann <thilo.borgm...@mail.de> wrote: > Hi, > >> On Mon, Nov 13, 2017 at 1:09 AM, Carl Eugen Hoyos <ceffm...@gmail.com> wrote: >>> 2017-11-12 20:30 GMT+01:00 Umair Khan <omerj...@gmail.com>: >>>> Hi, >>>> >>>> On Mon, Nov 13, 2017 at 12:45 AM, Carl Eugen Hoyos <ceffm...@gmail.com> >>>> wrote: >>>>> 2017-11-12 20:05 GMT+01:00 Umair Khan <omerj...@gmail.com>: >>>>> >>>>>> The attached patch fixes the address sanitizer issue. >>>>> >>>>> Breaks compilation here, how did you test? >>>>> >>>>> libavcodec/alsdec.c: In function ‘decode_var_block_data’: >>>>> libavcodec/alsdec.c:938:7: error: expected ‘}’ before ‘else’ >>>> >>>> Sorry for the faulty patch. Here is the fixed one. >>> >>> The commit message of your patch is: >>> libavcodec/als: fix address sanitization error in decoder >>> >>> Is there an error in current FFmpeg git head that asan >>> shows? If not, the commit message makes no sense. >>> >>> I believe you should send two patches that are meant >>> to be committed together, one of them fixing ticket #6297. >> >> This is the complete patchset. > > I need some days to find time to test this, earliest during the weekend I > fear... > > What happens for > block_length < residual_index < opt_order?
I didn't really understand this case. What's residual_index? Can you point to the source exactly? As far as the case where opt_order is more than block_length, my second patch handles that case only. The file which Michael sent was having asan issues because of the case when block_length < opt_order. > Another way of asking would be, where is the second loop from specs page 30 > for that case? > (ISO/IEC 14496) The second loop is just converting parcor to lpc coefficients which is done here - https://github.com/FFmpeg/FFmpeg/blob/master/libavcodec/alsdec.c#L935 > I think what puzzles CE is, that the problematic if() from the other patches > is still untouched by your patch. So how could this be a valid solution even > if your patch would actually improve the prediction part... > And I wonder the same ;) As said, it is valid to have opt_order greater than block_length. However, the decoder loop needs to be checked because we won't predict values more than the length of the block i.e., block_length. We use last K (prediction order, opt_order) values to predict the original K values of the current block. > Did you run FATE with your patch applied? I assume a big difference in output > at the first glance (means FATE aks the conformance files should fail...) Yes. I did run FATE. It passes perfectly. > Thanks for driving this forward anyway :) I think the two patches fix the issues completely. I don't see any harm in applying this patchset. :) -Umair _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
