On 1/5/2016 11:35 PM, Michael Niedermayer wrote: > On Tue, Jan 05, 2016 at 11:27:25PM -0300, James Almer wrote: >> On 1/5/2016 11:21 PM, Michael Niedermayer wrote: >>> On Tue, Jan 05, 2016 at 11:38:00PM +0300, foo86 wrote: >>>> On Tue, Jan 05, 2016 at 08:45:22PM +0100, Andreas Cadhalpun wrote: >>>>> On 03.01.2016 18:49, foo86 wrote: >>>>>> +// 5.3.1 - Bit stream header >>>>>> +static int parse_frame_header(DCA2CoreDecoder *s) >>>>>> +{ >>>>> [...] >>>>>> + // Source PCM resolution >>>>>> + s->source_pcm_res = ff_dca_bits_per_sample[pcmr_index = >>>>>> get_bits(&s->gb, 3)]; >>>>> >>>>> This can cause an out-of-bounds read if get_bits returns 7, because >>>>> ff_dca_bits_per_sample >>>>> only has 7 elements. >>>> >>>> Fixed locally, thanks. >>>> >>>> P.S. To avoid resending this huge patch, I've put the fixes accumulated >>>> so far in a private dcadec2 branch on github [1] (will be rebased >>>> frequently against FFmpeg master). >>>> >>> >>>> [1]: https://github.com/foo86/FFmpeg/tree/dcadec2 >>> >>> breaks "make fate", something needs to be updated >>> or a new reference sample uploaded if teh one we have is wrong >>> >>> stddev: 297.72 PSNR: 46.85 MAXDIFF: 3474 bytes: 8994816/ 9601024 >>> MAXDIFF: |3474 - 0| >= 1 >>> size: |8994816 - 9601024| >= 0 >>> Test dca-xll failed. Look at tests/data/fate/dca-xll.err for details. >>> make: *** [fate-dca-xll] Error 1 >>> make: *** Waiting for unfinished jobs.... >> >> Was this run using foo86's decoder, or the current one? If the former then >> it's >> not unexpected since the xll test was made for the current decoder, which is >> not >> bitexact. > > i just locally merged the branch and did a make -j12 fate
I see dca2 is above dca in allcodecs.c on this patch so i guess it's using the former, which would explain why it fails. > > >> >> Ideally, once this decoder is committed replacing the current one we'd >> replace >> the samples for the set available here: >> https://github.com/foo86/dcadec-samples > > we can add new fate samples, we cannot replace fate samples > replacing would break previous checkouts and releases > not sure you actually meant to replace any ... Yeah, forgot about that. Kinda makes for a bloated fate suit in the long run... Since the lossy tests (fate-dca-core and fate-dts_es) apparently still work with this decoder then there's no need to remove them, but the new extensions still need the above sample suit. > > [...] > > > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel