On 8/31/20, Michael Niedermayer <mich...@niedermayer.cc> wrote: > On Mon, Aug 31, 2020 at 11:14:07AM +0200, Paul B Mahol wrote: >> On 8/31/20, Michael Niedermayer <mich...@niedermayer.cc> wrote: >> > This is based on the encoder and a small number of CFHD sample files >> > It should make the decoder more robust against crafted input. >> > Due to the lack of a proper specification it is possible that this >> > may be too strict and may need to be tuned as files not following this >> > ordering are found. >> > >> > Signed-off-by: Michael Niedermayer <mich...@niedermayer.cc> >> > --- >> >> I'm afraid this is not nice solution. >> Please consider sharing samples that causes crash to me. >> IMO I'm afraid that this patch is just band-aid and not proper solution. > > As this ended up discussed on IRC too, heres the relevant log as reference > for the archieve as otherwise noone will find it > an interleaved discussion about AAC with JEEB is removed > > <michaelni> durandal_1707, kierank, i dont have any crashing samples which > are fixed by the cfhd table patch. That patch was intended to restrict > what/where elements can occur so that for example the one specifying the > subband number is in the subband "header" and not the plane "header" and > also not twice and also not missing. > <michaelni> Iam happy to implement this differently if you have a > suggestion > <durandal_1707> ah, this is patch to "fix" timeouts > <michaelni> durandal_1707, why do you write such flaimbait replies? The > patch has nothing to do with timeouts. Its rather that i see multiple fixes > for out of array writes in the history of the CFHD decoder and its header > parser allowing basically anything in any order no matter if it has any > sense. And the patch tries to restrict this. > <michaelni> if you are against the patch just say so and ill work on > something else > <michaelni> if work on this fron continues, the next logic step would be to > ensure each band, plane and so on is coded exactly once > <michaelni> because IIRC ATM the decoder doesnt check this either not just > that any band related field could be in the plane or main header part and so > on > <j-b> I agree with michaelni here. > <j-b> The tone is flamebait-y for no reason. > <durandal_1707> michaelni: restricting that with ugly patch is not nice > > to continue this on the ML which is where patch review belongs > which other way do you prefer ? > It could be implemented in many differnt ways, i cannot read your mind ... > Each method has some advanatges and disadvanatges from ease of > supporting unexpected order in odd files to code complexity to ease of > backporting the patch ... > > The whole loop could be redesigned and split up into 1 function for > each header type (main / plane / band / ...) each such function would > then have a similar loop with the subset of tags that can occur. > if you prefer that ? > but that would probably not be backported far, leaving some releases > without this > or as already said, i can just work on something else if people dont like > this. > Or maybe you want to implement this ? I can ofer to review your patch if > you want to implement this instead.
There are only some restrictions that can be tracked by reading SDK code. I prefer minimal intrusion in code - the minimal restrictions possible. _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
