On Sat, May 01, 2021 at 09:41:55AM -0300, James Almer wrote: > On 5/1/2021 4:01 AM, Michael Niedermayer wrote: > > On Thu, Apr 22, 2021 at 02:02:33PM -0300, James Almer wrote: > > > With JPEG-LS PAL8 samples, the JPEG-LS extension parameters signaled with > > > the LSE marker show up after SOF but before SOS. For those, the pixel > > > format > > > chosen by get_format() in SOF is GRAY8, and then replaced by PAL8 in LSE. > > > This has not been an issue given both pixel formats allocate the second > > > data > > > plane for the palette, but after the upcoming soname bump, GRAY8 will no > > > longer > > > do that. This will result in segfauls when ff_jpegls_decode_lse() > > > attempts to > > > write the palette on a buffer originally allocated as a GRAY8 one. > > > > > > Work around this by calling ff_get_buffer() after the actual pixel format > > > is > > > known. > > > > What if the LSE occurs after the SOS ? > > What if there are 2 LSE ? > > It seems allowed by the specification > > > > "The LSE marker segment may be present where tables or miscellaneous marker > > segments may appear. If tables specified > > in this marker segment for a given table ID appear more than once, each > > specification shall replace the previous > > specification." > > > > Maybe iam missing something but a implemenattion of this would seem to > > require scanning through the image, finding all LSE and checking if they all > > are compatible with the PAL8 format before allocating the image in SOF > > > > The implemenattion here which delays allocation slightly seems to make > > the code much more unstable allowing the frame configuration or allocation > > to be partly done, double done, redone, without the matching partner. > > Also it does not seem to implement the related specification completely. > > Well, it was a hack to replace a previous hack (allocate a gray buffer then > silently treat it as PAL8 once an LSE showed up). It's no wonder it may not > perfectly follow the spec and consider all potential cases.
yes and i wouldnt mind but the new hack is leading to crashes ... and the fixes to the crashes all in all start looking a bit ugly and iam not sure if any more remain. So id like to take a bit a step back here and see if there isnt a easier/cleaner solution. > > > > > A complete implemenattion may reqire RGB24 or 48 to be selected in some > > cases > > like when each scan uses its own palette, which unless iam missing something > > seems allowed. > > But then maybe iam missing something, in which case please correct me! > > You're probably not wrong, since unlike me you actually know this code and > format. I tried to workaround an issue and it seemed to work and not break > any file anyone here could test. > > If you think it's not good enough then just revert it and maybe allocate the > data[1] pointer for the palette with av_buffer_alloc(), get_buffer2() custom > implementations be damned. But i don't think the previous code even did half > the things you listed above. how can i reproduce the issue you where trying to fix with this patch ? thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB I have never wished to cater to the crowd; for what I know they do not approve, and what they approve I do not know. -- Epicurus
signature.asc
Description: PGP signature
_______________________________________________ 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".
