[FFmpeg-devel] [PATCH] Add FITS Encoder

[Reimar Döffinger]

On 21.07.2017, at 16:26, Paras Chadha <paraschadha18 at gmail.com> wrote:

> On Fri, Jul 21, 2017 at 3:27 AM, Carl Eugen Hoyos <ceffmpeg at gmail.com>
> wrote:
> 
>> 2017-07-20 21:46 GMT+02:00 Paras Chadha <paraschadha18 at gmail.com>:
>> 
>>> +            case AV_PIX_FMT_RGB24:
>>> +            case AV_PIX_FMT_RGBA:
>>> +                for (k = 0; k < naxis3; k++) {
>>> +                    for (i = 0; i < avctx->height; i++) {
>>> +                        ptr = p->data[0] + (avctx->height - i - 1) *
>> p->linesize[0] + k;
>>> +                        for (j = 0; j < avctx->width; j++) {
>>> +                            bytestream_put_byte(&bytestream, ptr[0]);
>>> +                            ptr += naxis3;
>> 
>> (Sorry if this is nonsense:)
>> Shouldn't you be using PIX_FMT_GBRP and PIX_FMT_GBRAP?
>> 
> 
> No, the current pixel formats are working fine. I have tested using GIMP
> too. It is showing all the RGB images (and others too) correctly.

I think that's not what he meant.
You are explicitly requesting a packed format that has the data in RGBRGBRGB... order, and you then need these loops to split it into RRRR... GGGG... BBBB...
He's suggesting you should simply request the format storing the data like that, then all you need to do is the unsigned -> signed conversion (which is also unnecessarily complicated since the subtraction is equivalent with a single bit flip which can be done more efficiently with an XOR, and probably we have an assembler-optimized version somewhere, since it's needed a lot for audio).