diff mbox

[FFmpeg-devel] Add FITS Encoder

Message ID 1500051873-7378-1-git-send-email-paraschadha18@gmail.com
State Superseded
Headers show

Commit Message

Paras July 14, 2017, 5:04 p.m. UTC 
Signed-off-by: Paras Chadha <paraschadha18@gmail.com>
---
 doc/general.texi       |   2 +
 libavcodec/Makefile    |   1 +
 libavcodec/allcodecs.c |   1 +
 libavcodec/fitsenc.c   | 259 +++++++++++++++++++++++++++++++++++++++++++++++++
 libavformat/img2enc.c  |   1 +
 5 files changed, 264 insertions(+)
 create mode 100644 libavcodec/fitsenc.c

Comments

Reimar Döffinger July 15, 2017, 6:43 a.m. UTC | #1 
On 14.07.2017, at 19:04, Paras Chadha <paraschadha18@gmail.com> wrote:

> +static int write_keyword_value(char * header, const char * keyword, int value)
> +{
> +    int i, len, ret;
> +    len = strlen(keyword);
> +    for (i = 0; i < len; i++) {
> +        header[i] = keyword[i];
> +    }

would suggest memcpy (especially in combination with next suggestion).

> +    char header_line[80];

I would suggest memset to fill with spaces, to avoid most loops.

> +    if (fitsctx->first_image) {
> +        strncpy(header_line, "SIMPLE  = ", 10);
> +        for (i = 10; i < 80; i++) {
> +            header_line[i] = ' ';
> +        }
> +        header_line[29] = 'T';
> +    } else {
> +        strncpy(header_line, "XTENSION= 'IMAGE   '", 20);
> +        for (i = 20; i < 80; i++) {
> +            header_line[i] = ' ';
> +        }
> +    }

Never use strncpy, it has utterly broken behaviour that never makes sense.
av_strlcpy could be used, but just memcpy is probably best here.

> +    bytestream2_put_buffer(&pbc, header_line, 80);
> +    lines_written++;

Admittedly it provides some protection against buffer overflow
(but then, overflowing an on-stack buffer like header_line would be worse),
but it just feels wrong to construct the header in header_line just to then copy
it over into the packet.
It could just be written directly into the packet data, either manually or via bytestream2 functions (but using those memset with space probably can't be used to avoid a lot of code).

> +    write_keyword_value(header_line, "NAXIS", naxis);

Either here or maybe rather in the switch it might be good to explain a bit what the naxis stuff is used for.
For someone coming across the format for the first time, the number of components being called "axis" seems really weird.

> +    if (bitpix == 16) {
> +        write_keyword_value(header_line, "BZERO", bzero);
> +        bytestream2_put_buffer(&pbc, header_line, 80);
> +        lines_written++;
> +    }

This might also be worth a comment.

> +        switch (avctx->pix_fmt) {
> +        #define case_n(cas, dref) \

Macro definitions should start in column 0 (I think they actually have to?) and be in all-uppercase.

> +            case cas: \
> +                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++) { \
> +                            bytestream2_put_byte(&pbc, dref(ptr) - bzero); \
> +                            ptr += naxis3; \
> +                        } \
> +                    } \
> +                } \
> +                break
> +            case_n(AV_PIX_FMT_RGB24, *);
> +            case_n(AV_PIX_FMT_RGBA, *);
> +            case_n(AV_PIX_FMT_RGB48BE, AV_RB16);
> +            case_n(AV_PIX_FMT_RGBA64BE, AV_RB16);

The macro will generate the same code for RGB24 and RGBA, and respectively for the other 2 cases.
Doing this without a macro and merging the cases with identical code seems much better.

> +            if (bitpix == 16) {
> +                for (j = 0; j < avctx->width; j++) {
> +                    bytestream2_put_be16(&pbc, AV_RB16(ptr) - bzero);
> +                    ptr += 2;
> +                }

Can't bzero be chosen as 0? Because then this is just a normal memcpy...

> +    t = padded_data_size - data_size;
> +    while (t--) {
> +        bytestream2_put_byte(&pbc, 0);
> +    }

Might be worth getting the bytestream pointer and memset (or adding memset functionality to bytestream2).
Paras July 16, 2017, 12:30 p.m. UTC | #2 
On Sat, Jul 15, 2017 at 12:13 PM, Reimar Döffinger <Reimar.Doeffinger@gmx.de
> wrote:

> On 14.07.2017, at 19:04, Paras Chadha <paraschadha18@gmail.com> wrote:
>
> > +            if (bitpix == 16) {
> > +                for (j = 0; j < avctx->width; j++) {
> > +                    bytestream2_put_be16(&pbc, AV_RB16(ptr) - bzero);
> > +                    ptr += 2;
> > +                }
>
> Can't bzero be chosen as 0? Because then this is just a normal memcpy...
>
>
No, actually FITS does not support unsigned 16-bit integers as data. So,
unsigned 16 bit integers are represented as signed integers by setting
BZERO = 32768 and subtracting the same from each data element. Any FITS
reader will add the BZERO value to each data elements before processing.

Rest all changes have been done. I will post a patch soon.


> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
diff mbox

Patch

diff --git a/doc/general.texi b/doc/general.texi
index 8f582d5..7f389ce 100644
--- a/doc/general.texi
+++ b/doc/general.texi
@@ -591,6 +591,8 @@  following image formats are supported:
     @tab Digital Picture Exchange
 @item EXR          @tab   @tab X
     @tab OpenEXR
+@item FITS         @tab X @tab X
+    @tab Flexible Image Transport System
 @item JPEG         @tab X @tab X
     @tab Progressive JPEG is not supported.
 @item JPEG 2000    @tab X @tab X
diff --git a/libavcodec/Makefile b/libavcodec/Makefile
index de1187f..9ad44e0 100644
--- a/libavcodec/Makefile
+++ b/libavcodec/Makefile
@@ -291,6 +291,7 @@  OBJS-$(CONFIG_FFV1_ENCODER)            += ffv1enc.o ffv1.o
 OBJS-$(CONFIG_FFWAVESYNTH_DECODER)     += ffwavesynth.o
 OBJS-$(CONFIG_FIC_DECODER)             += fic.o
 OBJS-$(CONFIG_FITS_DECODER)            += fitsdec.o
+OBJS-$(CONFIG_FITS_ENCODER)            += fitsenc.o
 OBJS-$(CONFIG_FLAC_DECODER)            += flacdec.o flacdata.o flac.o
 OBJS-$(CONFIG_FLAC_ENCODER)            += flacenc.o flacdata.o flac.o vorbis_data.o
 OBJS-$(CONFIG_FLASHSV_DECODER)         += flashsv.o
diff --git a/libavcodec/allcodecs.c b/libavcodec/allcodecs.c
index 0243f47..d180dde 100644
--- a/libavcodec/allcodecs.c
+++ b/libavcodec/allcodecs.c
@@ -192,6 +192,7 @@  static void register_all(void)
     REGISTER_ENCDEC (FFV1,              ffv1);
     REGISTER_ENCDEC (FFVHUFF,           ffvhuff);
     REGISTER_DECODER(FIC,               fic);
+    REGISTER_ENCDEC (FITS,              fits);
     REGISTER_ENCDEC (FLASHSV,           flashsv);
     REGISTER_ENCDEC (FLASHSV2,          flashsv2);
     REGISTER_DECODER(FLIC,              flic);
diff --git a/libavcodec/fitsenc.c b/libavcodec/fitsenc.c
new file mode 100644
index 0000000..62d97fc
--- /dev/null
+++ b/libavcodec/fitsenc.c
@@ -0,0 +1,259 @@ 
+/*
+ * FITS image encoder
+ * Copyright (c) 2017 Paras Chadha
+ *
+ * This file is part of FFmpeg.
+ *
+ * FFmpeg is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * FFmpeg is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with FFmpeg; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+/**
+ * @file
+ * FITS image encoder
+ *
+ * Specification: https://fits.gsfc.nasa.gov/fits_standard.html Version 3.0
+ */
+
+#include "libavutil/intreadwrite.h"
+#include "avcodec.h"
+#include "bytestream.h"
+#include "internal.h"
+
+typedef struct FITSContext {
+    int first_image;
+} FITSContext;
+
+static av_cold int fits_encode_init(AVCodecContext *avctx)
+{
+    FITSContext * fitsctx = avctx->priv_data;
+    fitsctx->first_image = 1;
+    return 0;
+}
+
+static int write_keyword_value(char * header, const char * keyword, int value)
+{
+    int i, len, ret;
+    len = strlen(keyword);
+    for (i = 0; i < len; i++) {
+        header[i] = keyword[i];
+    }
+    for (; i < 8; i++) {
+        header[i] = ' ';
+    }
+    header[8] = '=';
+    header[9] = ' ';
+    ret = snprintf(header + 10, 70, "%d", value);
+    for (i = ret + 10; i < 80; i++) {
+        header[i] = ' ';
+    }
+    return 0;
+}
+
+static int fits_encode_frame(AVCodecContext *avctx, AVPacket *pkt,
+                            const AVFrame *pict, int *got_packet)
+{
+    AVFrame * const p = (AVFrame *)pict;
+    FITSContext * fitsctx = avctx->priv_data;
+    PutByteContext pbc;
+    uint8_t *ptr;
+    uint64_t header_size = 2880, data_size = 0, padded_data_size = 0, lines_written = 0;
+    int ret, bitpix, naxis, naxis3 = 1, bzero = 0, i, j, k, t, rgb = 0;
+    char header_line[80];
+
+    switch (avctx->pix_fmt) {
+        case AV_PIX_FMT_GRAY8:
+            bitpix = 8;
+            naxis = 2;
+            break;
+        case AV_PIX_FMT_GRAY16BE:
+            bitpix = 16;
+            naxis = 2;
+            bzero = 32768;
+            break;
+        case AV_PIX_FMT_RGB24:
+        case AV_PIX_FMT_RGBA:
+            bitpix = 8;
+            naxis = 3;
+            rgb = 1;
+            if (avctx->pix_fmt == AV_PIX_FMT_RGB24) {
+                naxis3 = 3;
+            } else {
+                naxis3 = 4;
+            }
+            break;
+        case AV_PIX_FMT_RGB48BE:
+        case AV_PIX_FMT_RGBA64BE:
+            bitpix = 16;
+            naxis = 3;
+            bzero = 32768;
+            rgb = 1;
+            if (avctx->pix_fmt == AV_PIX_FMT_RGB48BE) {
+                naxis3 = 3;
+            } else {
+                naxis3 = 4;
+            }
+            break;
+        default:
+            av_log(avctx, AV_LOG_ERROR, "unsupported pixel format\n");
+            return AVERROR(EINVAL);
+    }
+
+    data_size = (bitpix >> 3) * avctx->height * avctx->width * naxis3;
+    padded_data_size = ((data_size + 2879) / 2880 ) * 2880;
+
+    if ((ret = ff_alloc_packet2(avctx, pkt, header_size + padded_data_size, 0)) < 0)
+        return ret;
+
+    bytestream2_init_writer(&pbc, pkt->data, pkt->size);
+
+    if (fitsctx->first_image) {
+        strncpy(header_line, "SIMPLE  = ", 10);
+        for (i = 10; i < 80; i++) {
+            header_line[i] = ' ';
+        }
+        header_line[29] = 'T';
+    } else {
+        strncpy(header_line, "XTENSION= 'IMAGE   '", 20);
+        for (i = 20; i < 80; i++) {
+            header_line[i] = ' ';
+        }
+    }
+    bytestream2_put_buffer(&pbc, header_line, 80);
+    lines_written++;
+
+    write_keyword_value(header_line, "BITPIX", bitpix);
+    bytestream2_put_buffer(&pbc, header_line, 80);
+    lines_written++;
+
+    write_keyword_value(header_line, "NAXIS", naxis);
+    bytestream2_put_buffer(&pbc, header_line, 80);
+    lines_written++;
+
+    write_keyword_value(header_line, "NAXIS1", avctx->width);
+    bytestream2_put_buffer(&pbc, header_line, 80);
+    lines_written++;
+
+    write_keyword_value(header_line, "NAXIS2", avctx->height);
+    bytestream2_put_buffer(&pbc, header_line, 80);
+    lines_written++;
+
+    if (rgb) {
+        write_keyword_value(header_line, "NAXIS3", naxis3);
+        bytestream2_put_buffer(&pbc, header_line, 80);
+        lines_written++;
+    }
+
+    if (!fitsctx->first_image) {
+        write_keyword_value(header_line, "PCOUNT", 0);
+        bytestream2_put_buffer(&pbc, header_line, 80);
+        lines_written++;
+
+        write_keyword_value(header_line, "GCOUNT", 1);
+        bytestream2_put_buffer(&pbc, header_line, 80);
+        lines_written++;
+    } else {
+        fitsctx->first_image = 0;
+    }
+
+    if (bitpix == 16) {
+        write_keyword_value(header_line, "BZERO", bzero);
+        bytestream2_put_buffer(&pbc, header_line, 80);
+        lines_written++;
+    }
+
+    if (rgb) {
+        strncpy(header_line, "CTYPE3  = 'RGB     '", 20);
+        for (i = 20; i < 80; i++) {
+            header_line[i] = ' ';
+        }
+        bytestream2_put_buffer(&pbc, header_line, 80);
+        lines_written++;
+    }
+
+    strncpy(header_line, "END", 3);
+    for (i = 3; i < 80; i++) {
+        header_line[i] = ' ';
+    }
+    bytestream2_put_buffer(&pbc, header_line, 80);
+    lines_written++;
+
+    t = 36 - lines_written;
+
+    for (i = 0; i < 80; i++) {
+        header_line[i] = ' ';
+    }
+    while (t--) {
+        bytestream2_put_buffer(&pbc, header_line, 80);
+    }
+
+    if (rgb) {
+        switch (avctx->pix_fmt) {
+        #define case_n(cas, dref) \
+            case cas: \
+                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++) { \
+                            bytestream2_put_byte(&pbc, dref(ptr) - bzero); \
+                            ptr += naxis3; \
+                        } \
+                    } \
+                } \
+                break
+            case_n(AV_PIX_FMT_RGB24, *);
+            case_n(AV_PIX_FMT_RGBA, *);
+            case_n(AV_PIX_FMT_RGB48BE, AV_RB16);
+            case_n(AV_PIX_FMT_RGBA64BE, AV_RB16);
+        }
+    } else {
+        for (i = 0; i < avctx->height; i++) {
+            ptr = p->data[0] + (avctx->height - i - 1) * p->linesize[0];
+            if (bitpix == 16) {
+                for (j = 0; j < avctx->width; j++) {
+                    bytestream2_put_be16(&pbc, AV_RB16(ptr) - bzero);
+                    ptr += 2;
+                }
+            } else {
+                bytestream2_put_buffer(&pbc, ptr, avctx->width);
+            }
+        }
+    }
+
+    t = padded_data_size - data_size;
+    while (t--) {
+        bytestream2_put_byte(&pbc, 0);
+    }
+
+    pkt->flags |= AV_PKT_FLAG_KEY;
+    *got_packet = 1;
+    return 0;
+}
+
+AVCodec ff_fits_encoder = {
+    .name           = "fits",
+    .long_name      = NULL_IF_CONFIG_SMALL("Flexible Image Transport System"),
+    .type           = AVMEDIA_TYPE_VIDEO,
+    .id             = AV_CODEC_ID_FITS,
+    .priv_data_size = sizeof(FITSContext),
+    .init           = fits_encode_init,
+    .encode2        = fits_encode_frame,
+    .pix_fmts       = (const enum AVPixelFormat[]) { AV_PIX_FMT_RGBA64BE,
+                                                 AV_PIX_FMT_RGB48BE,
+                                                 AV_PIX_FMT_RGBA,
+                                                 AV_PIX_FMT_RGB24,
+                                                 AV_PIX_FMT_GRAY16BE,
+                                                 AV_PIX_FMT_GRAY8,
+                                                 AV_PIX_FMT_NONE },
+};
diff --git a/libavformat/img2enc.c b/libavformat/img2enc.c
index 1297b1a..25283cc 100644
--- a/libavformat/img2enc.c
+++ b/libavformat/img2enc.c
@@ -237,6 +237,7 @@  AVOutputFormat ff_image2_muxer = {
 AVOutputFormat ff_image2pipe_muxer = {
     .name           = "image2pipe",
     .long_name      = NULL_IF_CONFIG_SMALL("piped image2 sequence"),
+    .extensions     = "fits",
     .priv_data_size = sizeof(VideoMuxData),
     .video_codec    = AV_CODEC_ID_MJPEG,
     .write_header   = write_header,