diff mbox

[FFmpeg-devel] avcodec/fitsdec: change to le pixel formats

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

Commit Message

Paras Sept. 3, 2017, 3:05 p.m. UTC 
Signed-off-by: Paras Chadha <paraschadha18@gmail.com>
---
This fixes the failed fate tests on ppc64be:
http://fate.ffmpeg.org/report.cgi?time=20170903050743&slot=ppc64be-RHEL7.0-gcc-4.8.5-ibmcrl

 libavcodec/fitsdec.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

--
2.4.11

Comments

James Almer Sept. 13, 2017, 12:56 a.m. UTC | #1 
On 9/3/2017 12:05 PM, Paras Chadha wrote:
> Signed-off-by: Paras Chadha <paraschadha18@gmail.com>
> ---
> This fixes the failed fate tests on ppc64be:
> http://fate.ffmpeg.org/report.cgi?time=20170903050743&slot=ppc64be-RHEL7.0-gcc-4.8.5-ibmcrl
> 
>  libavcodec/fitsdec.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/libavcodec/fitsdec.c b/libavcodec/fitsdec.c
> index ec15248..97b3d13 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_GBRP16LE;

Just wondering, the encoder and muxer use the big endian pixel formats
variants, and you're using AV_RB* helper macros everywhere. It seems
unusual then having the decoder output frames in little endian formats.

Wouldn't it make more sense to set this to BE and update the fate tests
accordingly instead?

>              } else {
> -                avctx->pix_fmt = AV_PIX_FMT_GBRAP16;
> +                avctx->pix_fmt = AV_PIX_FMT_GBRAP16LE;
>              }
>          } 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_GRAY16LE;
>          }
>      }
> 
> --
> 2.4.11
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
James Almer Sept. 15, 2017, 5:29 p.m. UTC | #2 
On 9/12/2017 9:56 PM, James Almer wrote:
> On 9/3/2017 12:05 PM, Paras Chadha wrote:
>> Signed-off-by: Paras Chadha <paraschadha18@gmail.com>
>> ---
>> This fixes the failed fate tests on ppc64be:
>> http://fate.ffmpeg.org/report.cgi?time=20170903050743&slot=ppc64be-RHEL7.0-gcc-4.8.5-ibmcrl
>>
>>  libavcodec/fitsdec.c | 6 +++---
>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/libavcodec/fitsdec.c b/libavcodec/fitsdec.c
>> index ec15248..97b3d13 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_GBRP16LE;
> 
> Just wondering, the encoder and muxer use the big endian pixel formats
> variants, and you're using AV_RB* helper macros everywhere. It seems
> unusual then having the decoder output frames in little endian formats.
> 
> Wouldn't it make more sense to set this to BE and update the fate tests
> accordingly instead?

Pinging, and CCing in case you're not subscribed to the ml (Please
include the ml address in your reply if that's the case).

> 
>>              } else {
>> -                avctx->pix_fmt = AV_PIX_FMT_GBRAP16;
>> +                avctx->pix_fmt = AV_PIX_FMT_GBRAP16LE;
>>              }
>>          } 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_GRAY16LE;
>>          }
>>      }
>>
>> --
>> 2.4.11
>>
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel@ffmpeg.org
>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>
>
Paras Sept. 15, 2017, 7:12 p.m. UTC | #3 
On Fri, Sep 15, 2017 at 10:59 PM, James Almer <jamrial@gmail.com> wrote:

> On 9/12/2017 9:56 PM, James Almer wrote:
> > On 9/3/2017 12:05 PM, Paras Chadha wrote:
> >> Signed-off-by: Paras Chadha <paraschadha18@gmail.com>
> >> ---
> >> This fixes the failed fate tests on ppc64be:
> >> http://fate.ffmpeg.org/report.cgi?time=20170903050743&slot=p
> pc64be-RHEL7.0-gcc-4.8.5-ibmcrl
> >>
> >>  libavcodec/fitsdec.c | 6 +++---
> >>  1 file changed, 3 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/libavcodec/fitsdec.c b/libavcodec/fitsdec.c
> >> index ec15248..97b3d13 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_GBRP16LE;
> >
> > Just wondering, the encoder and muxer use the big endian pixel formats
> > variants, and you're using AV_RB* helper macros everywhere. It seems
> > unusual then having the decoder output frames in little endian formats.
> >
> > Wouldn't it make more sense to set this to BE and update the fate tests
> > accordingly instead?
>

yes, right. I will do this and send a patch.


>
> Pinging, and CCing in case you're not subscribed to the ml (Please
> include the ml address in your reply if that's the case).
>
> >
> >>              } else {
> >> -                avctx->pix_fmt = AV_PIX_FMT_GBRAP16;
> >> +                avctx->pix_fmt = AV_PIX_FMT_GBRAP16LE;
> >>              }
> >>          } 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_GRAY16LE;
> >>          }
> >>      }
> >>
> >> --
> >> 2.4.11
> >>
> >> _______________________________________________
> >> ffmpeg-devel mailing list
> >> ffmpeg-devel@ffmpeg.org
> >> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >>
> >
>
>
diff mbox

Patch

diff --git a/libavcodec/fitsdec.c b/libavcodec/fitsdec.c
index ec15248..97b3d13 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_GBRP16LE;
             } else {
-                avctx->pix_fmt = AV_PIX_FMT_GBRAP16;
+                avctx->pix_fmt = AV_PIX_FMT_GBRAP16LE;
             }
         } 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_GRAY16LE;
         }
     }