Hi Philipp, On Lu, 2019-11-25 at 17:36 +0100, Philipp Zabel wrote: > > > > 1. It is necessary to support ARGB (4 components) > Ok. I'll add support for 4-component images. Thanks. > Is there a reasonable > default color encoding for 4-component images if the APP14 marker > segment is not present? Besides APP14, I did not find anything else that works, without APP14, RGB/ARGB colors are distorted, the only essplanation for this was the one from Rec T.872. > > > > > 2. It is necessary to support extended sequential (parse SOF1) > Do you require arithmetic coding or 12-bit sample precision as well, > or > just extended sequential with Huffman coding and 8-bit sample > precision? The imx8 jpeg supports both 8-bit and 12-bit sample precision. I only tested with 8-bit samples, some little adjustments might be necessary for 12-bit in the imx8 jpeg driver, having something for it in the common code would make it easier. > I see you used the "APP14 marker segment for colour encoding" as > specified in Recommendation T.872 [1]. I'll add support for this to > the > common code. Thanks. > > > > > 4. It is necessary to be able to modify/patch the component ID's > > inside > > SOF & SOS segments; this is due to a hardware limitation that the > > component ID's must be 0..3 or 1..4, however it is possible to > > decode a > > jpeg that violates this condition, if the component ID's are > > patched to > > accepted values. > Interesting. I'm not sure if this is something we should do > unconditionally in the common code. Maybe this warrants a flag. I forgot to mention, mxc_jpeg_valid_comp_id is where I did this hack, and that patching of the component IDs happens directly over the source (OUTPUT) buffer. If there won't be a helper for it, I will still have to be able to parse the SOF/SOS segments, which I was hoping to drop after using the common helpers. > > > > > I have a concern related to performance, about parsing the jpeg > > like > > that, but I did not get to measure anything yet, as I could not > > fully > > integrate imx8 jpeg driver with the helpers, I > > used v4l2_jpeg_parse_header, but I also had to keep my old > > structures. > > Please take a look in my imx8 patch, at mxc-jpeg.h, struct > > mxc_jpeg_sof/struct mxc_jpeg_sos, these are __packed structures, > > they > > work quite well via a simple cast and allow modifications too, the > > downside is that fields bigger than u8 might require swapping. > We can't use bitfields for external data in portable code, and I'm > not a > big fan of using __be16 in the API, but we could certainly use this > internally and see if that speeds up parsing. There are quite a few > superfluous bounds checks right now that can be optimized away. > I'd still like to copy the parsed headers into a structure provided > by > the caller. Ok, I'll try to do some measurements after the next version, with versus without the helpers. > > > > > Please also see below my comments. > I'll take these into account for the next version. Thank you for the > feedback! Thank you! > > regards > Philipp >
