diff mbox

[FFmpeg-devel,1/2] avcodec/fitsdec: change to be pixel formats

Message ID 1505507428-14308-1-git-send-email-paraschadha18@gmail.com
State New
Headers show

Commit Message

Paras Sept. 15, 2017, 8:30 p.m. UTC
Signed-off-by: Paras Chadha <paraschadha18@gmail.com>
---
 libavcodec/fitsdec.c | 34 ++++++++++++++++++----------------
 1 file changed, 18 insertions(+), 16 deletions(-)

Comments

Carl Eugen Hoyos Sept. 15, 2017, 9:34 p.m. UTC | #1
2017-09-15 22:30 GMT+02:00 Paras Chadha <paraschadha18@gmail.com>:

> -                    *dst++ = (type) t; \
> +                    write(dst, (type) t); \

Is the cast still necessary?

Is the new code faster on any platform?

Carl Eugen
James Almer Sept. 15, 2017, 9:37 p.m. UTC | #2
On 9/15/2017 6:34 PM, Carl Eugen Hoyos wrote:
> 2017-09-15 22:30 GMT+02:00 Paras Chadha <paraschadha18@gmail.com>:
> 
>> -                    *dst++ = (type) t; \
>> +                    write(dst, (type) t); \
> 
> Is the cast still necessary?
> 
> Is the new code faster on any platform?

It fixes the fate tests on big endian systems, which is the purpose of
this change.

> 
> Carl Eugen
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
Carl Eugen Hoyos Sept. 15, 2017, 9:43 p.m. UTC | #3
2017-09-15 23:37 GMT+02:00 James Almer <jamrial@gmail.com>:
> On 9/15/2017 6:34 PM, Carl Eugen Hoyos wrote:
>> 2017-09-15 22:30 GMT+02:00 Paras Chadha <paraschadha18@gmail.com>:
>>
>>> -                    *dst++ = (type) t; \
>>> +                    write(dst, (type) t); \
>>
>> Is the cast still necessary?
>>
>> Is the new code faster on any platform?
>
> It fixes the fate tests on big endian systems, which is the
> purpose of this change.

I just wonder if the original patch wasn't cleaner.

Carl Eugen
diff mbox

Patch

diff --git a/libavcodec/fitsdec.c b/libavcodec/fitsdec.c
index b075381..c72415f 100644
--- a/libavcodec/fitsdec.c
+++ b/libavcodec/fitsdec.c
@@ -205,9 +205,9 @@  static int fits_decode_frame(AVCodecContext *avctx, void *data, int *got_frame,
             }
         } else if (header.bitpix == 16) {
             if (header.naxisn[2] == 3) {
-                avctx->pix_fmt = AV_PIX_FMT_GBRP16;
+                avctx->pix_fmt = AV_PIX_FMT_GBRP16BE;
             } else {
-                avctx->pix_fmt = AV_PIX_FMT_GBRAP16;
+                avctx->pix_fmt = AV_PIX_FMT_GBRAP16BE;
             }
         } else {
             av_log(avctx, AV_LOG_ERROR, "unsupported BITPIX = %d\n", header.bitpix);
@@ -217,7 +217,7 @@  static int fits_decode_frame(AVCodecContext *avctx, void *data, int *got_frame,
         if (header.bitpix == 8) {
             avctx->pix_fmt = AV_PIX_FMT_GRAY8;
         } else {
-            avctx->pix_fmt = AV_PIX_FMT_GRAY16;
+            avctx->pix_fmt = AV_PIX_FMT_GRAY16BE;
         }
     }
 
@@ -233,7 +233,7 @@  static int fits_decode_frame(AVCodecContext *avctx, void *data, int *got_frame,
      */
     if (header.rgb) {
         switch(header.bitpix) {
-#define CASE_RGB(cas, dst, type, dref) \
+#define CASE_RGB(cas, dst, type, dref, write) \
     case cas: \
         for (k = 0; k < header.naxisn[2]; k++) { \
             for (i = 0; i < avctx->height; i++) { \
@@ -245,40 +245,42 @@  static int fits_decode_frame(AVCodecContext *avctx, void *data, int *got_frame,
                     } else { \
                         t = fitsctx->blank_val; \
                     } \
-                    *dst++ = (type) t; \
+                    write(dst, (type) t); \
+                    dst++; \
                     ptr8 += cas >> 3; \
                 } \
             } \
         } \
         break
 
-            CASE_RGB(8, dst8, uint8_t, *);
-            CASE_RGB(16, dst16, uint16_t, AV_RB16);
+            CASE_RGB(8, dst8, uint8_t, *, AV_WB8);
+            CASE_RGB(16, dst16, uint16_t, AV_RB16, AV_WB16);
         }
     } else {
         switch (header.bitpix) {
-#define CASE_GRAY(cas, dst, type, t, rd) \
+#define CASE_GRAY(cas, dst, type, t, rd, write) \
     case cas: \
         for (i = 0; i < avctx->height; i++) { \
             dst = (type *) (p->data[0] + (avctx->height-i-1)* p->linesize[0]); \
             for (j = 0; j < avctx->width; j++) { \
                 t = rd; \
                 if (!header.blank_found || t != header.blank) { \
-                    *dst++ = ((t - header.data_min) * ((1 << (sizeof(type) * 8)) - 1)) / (header.data_max - header.data_min); \
+                    write(dst, ((t - header.data_min) * ((1 << (sizeof(type) * 8)) - 1)) / (header.data_max - header.data_min)); \
                 } else { \
-                    *dst++ = fitsctx->blank_val; \
+                    write(dst, fitsctx->blank_val); \
                 } \
+                dst++; \
                 ptr8 += abs(cas) >> 3; \
             } \
         } \
         break
 
-            CASE_GRAY(-64, dst16, uint16_t, tdbl, av_int2double(AV_RB64(ptr8)));
-            CASE_GRAY(-32, dst16, uint16_t, tflt, av_int2float(AV_RB32(ptr8)));
-            CASE_GRAY(8, dst8, uint8_t, t8, ptr8[0]);
-            CASE_GRAY(16, dst16, uint16_t, t16, AV_RB16(ptr8));
-            CASE_GRAY(32, dst16, uint16_t, t32, AV_RB32(ptr8));
-            CASE_GRAY(64, dst16, uint16_t, t64, AV_RB64(ptr8));
+            CASE_GRAY(-64, dst16, uint16_t, tdbl, av_int2double(AV_RB64(ptr8)), AV_WB16);
+            CASE_GRAY(-32, dst16, uint16_t, tflt, av_int2float(AV_RB32(ptr8)), AV_WB16);
+            CASE_GRAY(8, dst8, uint8_t, t8, ptr8[0], AV_WB8);
+            CASE_GRAY(16, dst16, uint16_t, t16, AV_RB16(ptr8), AV_WB16);
+            CASE_GRAY(32, dst16, uint16_t, t32, AV_RB32(ptr8), AV_WB16);
+            CASE_GRAY(64, dst16, uint16_t, t64, AV_RB64(ptr8), AV_WB16);
             default:
                 av_log(avctx, AV_LOG_ERROR, "invalid BITPIX, %d\n", header.bitpix);
                 return AVERROR_INVALIDDATA;