diff mbox series

[FFmpeg-devel,3/3] avcodec/exr: add option to output pixels in float

Message ID 20200429030235.37074-4-mindmark@gmail.com
State New
Headers show
Series libswscale: initial input/output support for AV_PIX_FMT_GBRAPF32 | expand

Checks

Context Check Description
andriy/default pending
andriy/make success Make finished
andriy/make_fate success Make fate finished

Commit Message

Mark Reid April 29, 2020, 3:02 a.m. UTC 
From: Mark Reid <mindmark@gmail.com>

---
 libavcodec/exr.c | 103 +++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 86 insertions(+), 17 deletions(-)

Comments

Anton Khirnov May 6, 2020, 8:07 p.m. UTC | #1 
Quoting mindmark@gmail.com (2020-04-29 05:02:35)
> From: Mark Reid <mindmark@gmail.com>
> 
> ---
>  libavcodec/exr.c | 103 +++++++++++++++++++++++++++++++++++++++--------
>  1 file changed, 86 insertions(+), 17 deletions(-)
> 
> diff --git a/libavcodec/exr.c b/libavcodec/exr.c
> @@ -1888,6 +1955,8 @@ static const AVOption options[] = {
>          AV_OPT_TYPE_STRING, { .str = "" }, 0, 0, VD },
>      { "gamma", "Set the float gamma value when decoding", OFFSET(gamma),
>          AV_OPT_TYPE_FLOAT, { .dbl = 1.0f }, 0.001, FLT_MAX, VD },
> +    { "float", "Output frame in floating point pixel format, apply_trc does not get applied", OFFSET(output_float),
> +        AV_OPT_TYPE_BOOL, {.i64 = 0}, 0, 1, VD},

Wouldn't it make more sense to export the coded format rather than have
an option like this?
Mark Reid May 7, 2020, 1:16 a.m. UTC | #2 
On Wed, May 6, 2020 at 1:07 PM Anton Khirnov <anton@khirnov.net> wrote:

> Quoting mindmark@gmail.com (2020-04-29 05:02:35)
> > From: Mark Reid <mindmark@gmail.com>
> >
> > ---
> >  libavcodec/exr.c | 103 +++++++++++++++++++++++++++++++++++++++--------
> >  1 file changed, 86 insertions(+), 17 deletions(-)
> >
> > diff --git a/libavcodec/exr.c b/libavcodec/exr.c
> > @@ -1888,6 +1955,8 @@ static const AVOption options[] = {
> >          AV_OPT_TYPE_STRING, { .str = "" }, 0, 0, VD },
> >      { "gamma", "Set the float gamma value when decoding", OFFSET(gamma),
> >          AV_OPT_TYPE_FLOAT, { .dbl = 1.0f }, 0.001, FLT_MAX, VD },
> > +    { "float", "Output frame in floating point pixel format, apply_trc
> does not get applied", OFFSET(output_float),
> > +        AV_OPT_TYPE_BOOL, {.i64 = 0}, 0, 1, VD},
>
> Wouldn't it make more sense to export the coded format rather than have
> an option like this?
>
>
I could do that, I just was worried about breaking any user's use case. But
now that I think about it, I don't think it would, and I think that would
be simpler.


> --
> Anton Khirnov
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
Anton Khirnov May 7, 2020, 10:23 a.m. UTC | #3 
Quoting Mark Reid (2020-05-07 03:16:17)
> On Wed, May 6, 2020 at 1:07 PM Anton Khirnov <anton@khirnov.net> wrote:
> 
> > Quoting mindmark@gmail.com (2020-04-29 05:02:35)
> > > From: Mark Reid <mindmark@gmail.com>
> > >
> > > ---
> > >  libavcodec/exr.c | 103 +++++++++++++++++++++++++++++++++++++++--------
> > >  1 file changed, 86 insertions(+), 17 deletions(-)
> > >
> > > diff --git a/libavcodec/exr.c b/libavcodec/exr.c
> > > @@ -1888,6 +1955,8 @@ static const AVOption options[] = {
> > >          AV_OPT_TYPE_STRING, { .str = "" }, 0, 0, VD },
> > >      { "gamma", "Set the float gamma value when decoding", OFFSET(gamma),
> > >          AV_OPT_TYPE_FLOAT, { .dbl = 1.0f }, 0.001, FLT_MAX, VD },
> > > +    { "float", "Output frame in floating point pixel format, apply_trc
> > does not get applied", OFFSET(output_float),
> > > +        AV_OPT_TYPE_BOOL, {.i64 = 0}, 0, 1, VD},
> >
> > Wouldn't it make more sense to export the coded format rather than have
> > an option like this?
> >
> >
> I could do that, I just was worried about breaking any user's use case. But
> now that I think about it, I don't think it would, and I think that would
> be simpler.

Yes, if swscale supports this then it shouldn't cause problems.

I believe the general rule is that decoders should export the native
format inasmuch as possible and avoid doing conversion internally.
diff mbox series

Patch

diff --git a/libavcodec/exr.c b/libavcodec/exr.c
index 73419eadb1..f86e97a433 100644
--- a/libavcodec/exr.c
+++ b/libavcodec/exr.c
@@ -161,6 +161,7 @@  typedef struct EXRContext {
     enum AVColorTransferCharacteristic apply_trc_type;
     float gamma;
     uint16_t gamma_table[65536];
+    int output_float;
 } EXRContext;
 
 /* -15 stored using a single precision bias of 127 */
@@ -1035,14 +1036,14 @@  static int decode_block(AVCodecContext *avctx, void *tdata,
     const uint8_t *channel_buffer[4] = { 0 };
     const uint8_t *buf = s->buf;
     uint64_t line_offset, uncompressed_size;
-    uint16_t *ptr_x;
     uint8_t *ptr;
     uint32_t data_size;
     uint64_t line, col = 0;
     uint64_t tile_x, tile_y, tile_level_x, tile_level_y;
     const uint8_t *src;
-    int axmax = (avctx->width - (s->xmax + 1)) * 2 * s->desc->nb_components; /* nb pixel to add at the right of the datawindow */
-    int bxmin = s->xmin * 2 * s->desc->nb_components; /* nb pixel to add at the left of the datawindow */
+    int step = s->desc->flags & AV_PIX_FMT_FLAG_FLOAT ? 4 : 2 * s->desc->nb_components;
+    int axmax = (avctx->width - (s->xmax + 1)) * step; /* nb pixel to add at the right of the datawindow */
+    int bxmin = s->xmin * step; /* nb pixel to add at the left of the datawindow */
     int i, x, buf_size = s->buf_size;
     int c, rgb_channel_count;
     float one_gamma = 1.0f / s->gamma;
@@ -1175,6 +1176,58 @@  static int decode_block(AVCodecContext *avctx, void *tdata,
     if (s->channel_offsets[3] >= 0)
         channel_buffer[3] = src + td->xsize * s->channel_offsets[3];
 
+    if (s->desc->flags & AV_PIX_FMT_FLAG_FLOAT) {
+
+        /* todo: change this when a floating point pixel format with luma with alpha is implemented */
+        int channel_count = s->channel_offsets[3] >= 0 ? 4 : rgb_channel_count;
+        if (s->is_luma) {
+            channel_buffer[1] = channel_buffer[0];
+            channel_buffer[2] = channel_buffer[0];
+        }
+
+        for (c = 0; c < channel_count; c++) {
+            int plane = s->desc->comp[c].plane;
+            ptr = p->data[plane] + line * p->linesize[plane] + (col * 4);
+
+            for (i = 0; i < td->ysize; i++, ptr += p->linesize[plane]) {
+                const uint8_t *src;
+                union av_intfloat32 *ptr_x;
+
+                src = channel_buffer[c];
+                ptr_x = (union av_intfloat32 *)ptr;
+
+                // Zero out the start if xmin is not 0
+                memset(ptr_x, 0, bxmin);
+                ptr_x += s->xmin;
+
+                if (s->pixel_type == EXR_FLOAT) {
+                    // 32-bit
+                    for (x = 0; x < td->xsize; x++) {
+                        ptr_x->i = bytestream_get_le32(&src);
+                        ptr_x++;
+                    }
+                } else if (s->pixel_type == EXR_HALF) {
+                    // 16-bit
+                    for (x = 0; x < td->xsize; x++) {
+                        *ptr_x++ = exr_half2float(bytestream_get_le16(&src));
+                    }
+                } else if (s->pixel_type == EXR_UINT) {
+                    const float float_mult = 1.0f / (float)UINT32_MAX;
+                    for (x = 0; x < td->xsize; x++) {
+                        ptr_x->f = float_mult * (float)bytestream_get_le32(&src);
+                        ptr_x++;
+                    }
+                }
+
+                // Zero out the end if xmax+1 is not w
+                memset(ptr_x, 0, axmax);
+                channel_buffer[c] += td->channel_line_size;
+            }
+        }
+
+        return 0;
+    }
+
     ptr = p->data[0] + line * p->linesize[0] + (col * s->desc->nb_components * 2);
 
     for (i = 0;
@@ -1182,6 +1235,7 @@  static int decode_block(AVCodecContext *avctx, void *tdata,
 
         const uint8_t * a;
         const uint8_t *rgb[3];
+        uint16_t *ptr_x;
 
         for (c = 0; c < rgb_channel_count; c++) {
             rgb[c] = channel_buffer[c];
@@ -1676,7 +1730,8 @@  static int decode_frame(AVCodecContext *avctx, void *data,
     AVFrame *picture = data;
     uint8_t *ptr;
 
-    int y, ret;
+    int i, y, ret;
+    int planes;
     int out_line_size;
     int nb_blocks;   /* nb scanline or nb tile */
     uint64_t start_offset_table;
@@ -1694,15 +1749,16 @@  static int decode_frame(AVCodecContext *avctx, void *data,
     case EXR_UINT:
         if (s->channel_offsets[3] >= 0) {
             if (!s->is_luma) {
-                avctx->pix_fmt = AV_PIX_FMT_RGBA64;
+                avctx->pix_fmt = s->output_float ? AV_PIX_FMT_GBRAPF32 : AV_PIX_FMT_RGBA64;
             } else {
-                avctx->pix_fmt = AV_PIX_FMT_YA16;
+                /* todo: change this when a floating point pixel format with luma with alpha is implemented */
+                avctx->pix_fmt = s->output_float ? AV_PIX_FMT_GBRAPF32 : AV_PIX_FMT_YA16;
             }
         } else {
             if (!s->is_luma) {
-                avctx->pix_fmt = AV_PIX_FMT_RGB48;
+                avctx->pix_fmt = s->output_float ? AV_PIX_FMT_GBRPF32 : AV_PIX_FMT_RGB48;
             } else {
-                avctx->pix_fmt = AV_PIX_FMT_GRAY16;
+                avctx->pix_fmt = s->output_float ? AV_PIX_FMT_GRAYF32 : AV_PIX_FMT_GRAY16;
             }
         }
         break;
@@ -1751,7 +1807,14 @@  static int decode_frame(AVCodecContext *avctx, void *data,
     s->desc          = av_pix_fmt_desc_get(avctx->pix_fmt);
     if (!s->desc)
         return AVERROR_INVALIDDATA;
-    out_line_size    = avctx->width * 2 * s->desc->nb_components;
+
+    if (s->desc->flags & AV_PIX_FMT_FLAG_FLOAT) {
+        planes           = s->desc->nb_components;
+        out_line_size    = avctx->width * 4;
+    } else {
+        planes           = 1;
+        out_line_size    = avctx->width * 2 * s->desc->nb_components;
+    }
 
     if (s->is_tile) {
         nb_blocks = ((s->xdelta + s->tile_attr.xSize - 1) / s->tile_attr.xSize) *
@@ -1789,12 +1852,14 @@  static int decode_frame(AVCodecContext *avctx, void *data,
     // save pointer we are going to use in decode_block
     s->buf      = avpkt->data;
     s->buf_size = avpkt->size;
-    ptr         = picture->data[0];
 
     // Zero out the start if ymin is not 0
-    for (y = 0; y < s->ymin; y++) {
-        memset(ptr, 0, out_line_size);
-        ptr += picture->linesize[0];
+    for (i = 0; i < planes; i++) {
+        ptr = picture->data[i];
+        for (y = 0; y < s->ymin; y++) {
+            memset(ptr, 0, out_line_size);
+            ptr += picture->linesize[i];
+        }
     }
 
     s->picture = picture;
@@ -1802,10 +1867,12 @@  static int decode_frame(AVCodecContext *avctx, void *data,
     avctx->execute2(avctx, decode_block, s->thread_data, NULL, nb_blocks);
 
     // Zero out the end if ymax+1 is not h
-    ptr = picture->data[0] + ((s->ymax+1) * picture->linesize[0]);
-    for (y = s->ymax + 1; y < avctx->height; y++) {
-        memset(ptr, 0, out_line_size);
-        ptr += picture->linesize[0];
+    for (i = 0; i < planes; i++) {
+        ptr = picture->data[i] + ((s->ymax+1) * picture->linesize[i]);
+        for (y = s->ymax + 1; y < avctx->height; y++) {
+            memset(ptr, 0, out_line_size);
+            ptr += picture->linesize[i];
+        }
     }
 
     picture->pict_type = AV_PICTURE_TYPE_I;
@@ -1888,6 +1955,8 @@  static const AVOption options[] = {
         AV_OPT_TYPE_STRING, { .str = "" }, 0, 0, VD },
     { "gamma", "Set the float gamma value when decoding", OFFSET(gamma),
         AV_OPT_TYPE_FLOAT, { .dbl = 1.0f }, 0.001, FLT_MAX, VD },
+    { "float", "Output frame in floating point pixel format, apply_trc does not get applied", OFFSET(output_float),
+        AV_OPT_TYPE_BOOL, {.i64 = 0}, 0, 1, VD},
 
     // XXX: Note the abuse of the enum using AVCOL_TRC_UNSPECIFIED to subsume the existing gamma option
     { "apply_trc", "color transfer characteristics to apply to EXR linear input", OFFSET(apply_trc_type),