[FFmpeg-devel] [PATCH 1/2] avcodec/tiff: Avoid forward declarations
Pavel Koshevoy
pkoshevoy at gmail.com
Tue Mar 30 18:06:22 EEST 2021
On Tue, Mar 30, 2021 at 2:11 AM Andreas Rheinhardt <
andreas.rheinhardt at outlook.com> wrote:
> In this case it also fixes a potential for compilation failures:
> Not all compilers can handle the case in which a function with
> a forward declaration declared with an attribute to always inline it
> is called before the function body appears.
>
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at outlook.com>
> ---
> This is of course designed as an alternative to Pavel's patch
> https://ffmpeg.org/pipermail/ffmpeg-devel/2021-March/278383.html
>
> libavcodec/tiff.c | 392 +++++++++++++++++++++++-----------------------
> 1 file changed, 193 insertions(+), 199 deletions(-)
>
> diff --git a/libavcodec/tiff.c b/libavcodec/tiff.c
> index 0878098b90..1d72fdc720 100644
> --- a/libavcodec/tiff.c
> +++ b/libavcodec/tiff.c
> @@ -275,9 +275,99 @@ static int add_metadata(int count, int type,
> };
> }
>
> +/**
> + * Map stored raw sensor values into linear reference values (see: DNG
> Specification - Chapter 5)
> + */
> +static uint16_t av_always_inline dng_process_color16(uint16_t value,
> + const uint16_t *lut,
> + uint16_t black_level,
> + float scale_factor) {
> + float value_norm;
> +
> + // Lookup table lookup
> + if (lut)
> + value = lut[value];
> +
> + // Black level subtraction
> + value = av_clip_uint16_c((unsigned)value - black_level);
> +
> + // Color scaling
> + value_norm = (float)value * scale_factor;
> +
> + value = av_clip_uint16_c(value_norm * 65535);
> +
> + return value;
> +}
> +
> +static uint16_t av_always_inline dng_process_color8(uint16_t value,
> + const uint16_t *lut,
> + uint16_t black_level,
> + float scale_factor) {
> + return dng_process_color16(value, lut, black_level, scale_factor) >>
> 8;
> +}
> +
> static void av_always_inline dng_blit(TiffContext *s, uint8_t *dst, int
> dst_stride,
> const uint8_t *src, int src_stride,
> int width, int height,
> - int is_single_comp, int is_u16);
> + int is_single_comp, int is_u16)
> +{
> + int line, col;
> + float scale_factor;
> +
> + scale_factor = 1.0f / (s->white_level - s->black_level);
> +
> + if (is_single_comp) {
> + if (!is_u16)
> + return; /* <= 8bpp unsupported */
> +
> + /* Image is double the width and half the height we need, each
> row comprises 2 rows of the output
> + (split vertically in the middle). */
> + for (line = 0; line < height / 2; line++) {
> + uint16_t *dst_u16 = (uint16_t *)dst;
> + uint16_t *src_u16 = (uint16_t *)src;
> +
> + /* Blit first half of input row row to initial row of output
> */
> + for (col = 0; col < width; col++)
> + *dst_u16++ = dng_process_color16(*src_u16++, s->dng_lut,
> s->black_level, scale_factor);
> +
> + /* Advance the destination pointer by a row (source pointer
> remains in the same place) */
> + dst += dst_stride * sizeof(uint16_t);
> + dst_u16 = (uint16_t *)dst;
> +
> + /* Blit second half of input row row to next row of output */
> + for (col = 0; col < width; col++)
> + *dst_u16++ = dng_process_color16(*src_u16++, s->dng_lut,
> s->black_level, scale_factor);
> +
> + dst += dst_stride * sizeof(uint16_t);
> + src += src_stride * sizeof(uint16_t);
> + }
> + } else {
> + /* Input and output image are the same size and the MJpeg decoder
> has done per-component
> + deinterleaving, so blitting here is straightforward. */
> + if (is_u16) {
> + for (line = 0; line < height; line++) {
> + uint16_t *dst_u16 = (uint16_t *)dst;
> + uint16_t *src_u16 = (uint16_t *)src;
> +
> + for (col = 0; col < width; col++)
> + *dst_u16++ = dng_process_color16(*src_u16++,
> s->dng_lut, s->black_level, scale_factor);
> +
> + dst += dst_stride * sizeof(uint16_t);
> + src += src_stride * sizeof(uint16_t);
> + }
> + } else {
> + for (line = 0; line < height; line++) {
> + uint8_t *dst_u8 = dst;
> + const uint8_t *src_u8 = src;
> +
> + for (col = 0; col < width; col++)
> + *dst_u8++ = dng_process_color8(*src_u8++, s->dng_lut,
> s->black_level, scale_factor);
> +
> + dst += dst_stride;
> + src += src_stride;
> + }
> + }
> + }
> +}
>
> static void av_always_inline horizontal_fill(TiffContext *s,
> unsigned int bpp, uint8_t*
> dst,
> @@ -553,7 +643,108 @@ static int tiff_unpack_fax(TiffContext *s, uint8_t
> *dst, int stride,
> return ret;
> }
>
> -static int dng_decode_strip(AVCodecContext *avctx, AVFrame *frame);
> +static int dng_decode_jpeg(AVCodecContext *avctx, AVFrame *frame,
> + int tile_byte_count, int dst_x, int dst_y, int
> w, int h)
> +{
> + TiffContext *s = avctx->priv_data;
> + uint8_t *dst_data, *src_data;
> + uint32_t dst_offset; /* offset from dst buffer in pixels */
> + int is_single_comp, is_u16, pixel_size;
> + int ret;
> +
> + if (tile_byte_count < 0 || tile_byte_count >
> bytestream2_get_bytes_left(&s->gb))
> + return AVERROR_INVALIDDATA;
> +
> + /* Prepare a packet and send to the MJPEG decoder */
> + av_packet_unref(s->jpkt);
> + s->jpkt->data = (uint8_t*)s->gb.buffer;
> + s->jpkt->size = tile_byte_count;
> +
> + if (s->is_bayer) {
> + MJpegDecodeContext *mjpegdecctx = s->avctx_mjpeg->priv_data;
> + /* We have to set this information here, there is no way to know
> if a given JPEG is a DNG-embedded
> + image or not from its own data (and we need that information
> when decoding it). */
> + mjpegdecctx->bayer = 1;
> + }
> +
> + ret = avcodec_send_packet(s->avctx_mjpeg, s->jpkt);
> + if (ret < 0) {
> + av_log(avctx, AV_LOG_ERROR, "Error submitting a packet for
> decoding\n");
> + return ret;
> + }
> +
> + ret = avcodec_receive_frame(s->avctx_mjpeg, s->jpgframe);
> + if (ret < 0) {
> + av_log(avctx, AV_LOG_ERROR, "JPEG decoding error: %s.\n",
> av_err2str(ret));
> +
> + /* Normally skip, error if explode */
> + if (avctx->err_recognition & AV_EF_EXPLODE)
> + return AVERROR_INVALIDDATA;
> + else
> + return 0;
> + }
> +
> + is_u16 = (s->bpp > 8);
> +
> + /* Copy the outputted tile's pixels from 'jpgframe' to 'frame' (final
> buffer) */
> +
> + if (s->jpgframe->width != s->avctx_mjpeg->width ||
> + s->jpgframe->height != s->avctx_mjpeg->height ||
> + s->jpgframe->format != s->avctx_mjpeg->pix_fmt)
> + return AVERROR_INVALIDDATA;
> +
> + /* See dng_blit for explanation */
> + if (s->avctx_mjpeg->width == w * 2 &&
> + s->avctx_mjpeg->height == h / 2 &&
> + s->avctx_mjpeg->pix_fmt == AV_PIX_FMT_GRAY16LE) {
> + is_single_comp = 1;
> + } else if (s->avctx_mjpeg->width >= w &&
> + s->avctx_mjpeg->height >= h &&
> + s->avctx_mjpeg->pix_fmt == (is_u16 ? AV_PIX_FMT_GRAY16 :
> AV_PIX_FMT_GRAY8)
> + ) {
> + is_single_comp = 0;
> + } else
> + return AVERROR_INVALIDDATA;
> +
> + pixel_size = (is_u16 ? sizeof(uint16_t) : sizeof(uint8_t));
> +
> + if (is_single_comp && !is_u16) {
> + av_log(s->avctx, AV_LOG_ERROR, "DNGs with bpp <= 8 and 1
> component are unsupported\n");
> + av_frame_unref(s->jpgframe);
> + return AVERROR_PATCHWELCOME;
> + }
> +
> + dst_offset = dst_x + frame->linesize[0] * dst_y / pixel_size;
> + dst_data = frame->data[0] + dst_offset * pixel_size;
> + src_data = s->jpgframe->data[0];
> +
> + dng_blit(s,
> + dst_data,
> + frame->linesize[0] / pixel_size,
> + src_data,
> + s->jpgframe->linesize[0] / pixel_size,
> + w,
> + h,
> + is_single_comp,
> + is_u16);
> +
> + av_frame_unref(s->jpgframe);
> +
> + return 0;
> +}
> +
> +static int dng_decode_strip(AVCodecContext *avctx, AVFrame *frame)
> +{
> + TiffContext *s = avctx->priv_data;
> +
> + s->jpgframe->width = s->width;
> + s->jpgframe->height = s->height;
> +
> + s->avctx_mjpeg->width = s->width;
> + s->avctx_mjpeg->height = s->height;
> +
> + return dng_decode_jpeg(avctx, frame, s->stripsize, 0, 0, s->width,
> s->height);
> +}
>
> static int tiff_unpack_strip(TiffContext *s, AVFrame *p, uint8_t *dst,
> int stride,
> const uint8_t *src, int size, int
> strip_start, int lines)
> @@ -780,190 +971,6 @@ static int tiff_unpack_strip(TiffContext *s, AVFrame
> *p, uint8_t *dst, int strid
> return 0;
> }
>
> -/**
> - * Map stored raw sensor values into linear reference values (see: DNG
> Specification - Chapter 5)
> - */
> -static uint16_t av_always_inline dng_process_color16(uint16_t value,
> - const uint16_t *lut,
> - uint16_t black_level,
> - float scale_factor) {
> - float value_norm;
> -
> - // Lookup table lookup
> - if (lut)
> - value = lut[value];
> -
> - // Black level subtraction
> - value = av_clip_uint16_c((unsigned)value - black_level);
> -
> - // Color scaling
> - value_norm = (float)value * scale_factor;
> -
> - value = av_clip_uint16_c(value_norm * 65535);
> -
> - return value;
> -}
> -
> -static uint16_t av_always_inline dng_process_color8(uint16_t value,
> - const uint16_t *lut,
> - uint16_t black_level,
> - float scale_factor) {
> - return dng_process_color16(value, lut, black_level, scale_factor) >>
> 8;
> -}
> -
> -static void dng_blit(TiffContext *s, uint8_t *dst, int dst_stride,
> - const uint8_t *src, int src_stride,
> - int width, int height, int is_single_comp, int
> is_u16)
> -{
> - int line, col;
> - float scale_factor;
> -
> - scale_factor = 1.0f / (s->white_level - s->black_level);
> -
> - if (is_single_comp) {
> - if (!is_u16)
> - return; /* <= 8bpp unsupported */
> -
> - /* Image is double the width and half the height we need, each
> row comprises 2 rows of the output
> - (split vertically in the middle). */
> - for (line = 0; line < height / 2; line++) {
> - uint16_t *dst_u16 = (uint16_t *)dst;
> - uint16_t *src_u16 = (uint16_t *)src;
> -
> - /* Blit first half of input row row to initial row of output
> */
> - for (col = 0; col < width; col++)
> - *dst_u16++ = dng_process_color16(*src_u16++, s->dng_lut,
> s->black_level, scale_factor);
> -
> - /* Advance the destination pointer by a row (source pointer
> remains in the same place) */
> - dst += dst_stride * sizeof(uint16_t);
> - dst_u16 = (uint16_t *)dst;
> -
> - /* Blit second half of input row row to next row of output */
> - for (col = 0; col < width; col++)
> - *dst_u16++ = dng_process_color16(*src_u16++, s->dng_lut,
> s->black_level, scale_factor);
> -
> - dst += dst_stride * sizeof(uint16_t);
> - src += src_stride * sizeof(uint16_t);
> - }
> - } else {
> - /* Input and output image are the same size and the MJpeg decoder
> has done per-component
> - deinterleaving, so blitting here is straightforward. */
> - if (is_u16) {
> - for (line = 0; line < height; line++) {
> - uint16_t *dst_u16 = (uint16_t *)dst;
> - uint16_t *src_u16 = (uint16_t *)src;
> -
> - for (col = 0; col < width; col++)
> - *dst_u16++ = dng_process_color16(*src_u16++,
> s->dng_lut, s->black_level, scale_factor);
> -
> - dst += dst_stride * sizeof(uint16_t);
> - src += src_stride * sizeof(uint16_t);
> - }
> - } else {
> - for (line = 0; line < height; line++) {
> - uint8_t *dst_u8 = dst;
> - const uint8_t *src_u8 = src;
> -
> - for (col = 0; col < width; col++)
> - *dst_u8++ = dng_process_color8(*src_u8++, s->dng_lut,
> s->black_level, scale_factor);
> -
> - dst += dst_stride;
> - src += src_stride;
> - }
> - }
> - }
> -}
> -
> -static int dng_decode_jpeg(AVCodecContext *avctx, AVFrame *frame,
> - int tile_byte_count, int dst_x, int dst_y, int
> w, int h)
> -{
> - TiffContext *s = avctx->priv_data;
> - uint8_t *dst_data, *src_data;
> - uint32_t dst_offset; /* offset from dst buffer in pixels */
> - int is_single_comp, is_u16, pixel_size;
> - int ret;
> -
> - if (tile_byte_count < 0 || tile_byte_count >
> bytestream2_get_bytes_left(&s->gb))
> - return AVERROR_INVALIDDATA;
> -
> - /* Prepare a packet and send to the MJPEG decoder */
> - av_packet_unref(s->jpkt);
> - s->jpkt->data = (uint8_t*)s->gb.buffer;
> - s->jpkt->size = tile_byte_count;
> -
> - if (s->is_bayer) {
> - MJpegDecodeContext *mjpegdecctx = s->avctx_mjpeg->priv_data;
> - /* We have to set this information here, there is no way to know
> if a given JPEG is a DNG-embedded
> - image or not from its own data (and we need that information
> when decoding it). */
> - mjpegdecctx->bayer = 1;
> - }
> -
> - ret = avcodec_send_packet(s->avctx_mjpeg, s->jpkt);
> - if (ret < 0) {
> - av_log(avctx, AV_LOG_ERROR, "Error submitting a packet for
> decoding\n");
> - return ret;
> - }
> -
> - ret = avcodec_receive_frame(s->avctx_mjpeg, s->jpgframe);
> - if (ret < 0) {
> - av_log(avctx, AV_LOG_ERROR, "JPEG decoding error: %s.\n",
> av_err2str(ret));
> -
> - /* Normally skip, error if explode */
> - if (avctx->err_recognition & AV_EF_EXPLODE)
> - return AVERROR_INVALIDDATA;
> - else
> - return 0;
> - }
> -
> - is_u16 = (s->bpp > 8);
> -
> - /* Copy the outputted tile's pixels from 'jpgframe' to 'frame' (final
> buffer) */
> -
> - if (s->jpgframe->width != s->avctx_mjpeg->width ||
> - s->jpgframe->height != s->avctx_mjpeg->height ||
> - s->jpgframe->format != s->avctx_mjpeg->pix_fmt)
> - return AVERROR_INVALIDDATA;
> -
> - /* See dng_blit for explanation */
> - if (s->avctx_mjpeg->width == w * 2 &&
> - s->avctx_mjpeg->height == h / 2 &&
> - s->avctx_mjpeg->pix_fmt == AV_PIX_FMT_GRAY16LE) {
> - is_single_comp = 1;
> - } else if (s->avctx_mjpeg->width >= w &&
> - s->avctx_mjpeg->height >= h &&
> - s->avctx_mjpeg->pix_fmt == (is_u16 ? AV_PIX_FMT_GRAY16 :
> AV_PIX_FMT_GRAY8)
> - ) {
> - is_single_comp = 0;
> - } else
> - return AVERROR_INVALIDDATA;
> -
> - pixel_size = (is_u16 ? sizeof(uint16_t) : sizeof(uint8_t));
> -
> - if (is_single_comp && !is_u16) {
> - av_log(s->avctx, AV_LOG_ERROR, "DNGs with bpp <= 8 and 1
> component are unsupported\n");
> - av_frame_unref(s->jpgframe);
> - return AVERROR_PATCHWELCOME;
> - }
> -
> - dst_offset = dst_x + frame->linesize[0] * dst_y / pixel_size;
> - dst_data = frame->data[0] + dst_offset * pixel_size;
> - src_data = s->jpgframe->data[0];
> -
> - dng_blit(s,
> - dst_data,
> - frame->linesize[0] / pixel_size,
> - src_data,
> - s->jpgframe->linesize[0] / pixel_size,
> - w,
> - h,
> - is_single_comp,
> - is_u16);
> -
> - av_frame_unref(s->jpgframe);
> -
> - return 0;
> -}
> -
> static int dng_decode_tiles(AVCodecContext *avctx, AVFrame *frame,
> const AVPacket *avpkt)
> {
> @@ -1040,19 +1047,6 @@ static int dng_decode_tiles(AVCodecContext *avctx,
> AVFrame *frame,
> return avpkt->size;
> }
>
> -static int dng_decode_strip(AVCodecContext *avctx, AVFrame *frame)
> -{
> - TiffContext *s = avctx->priv_data;
> -
> - s->jpgframe->width = s->width;
> - s->jpgframe->height = s->height;
> -
> - s->avctx_mjpeg->width = s->width;
> - s->avctx_mjpeg->height = s->height;
> -
> - return dng_decode_jpeg(avctx, frame, s->stripsize, 0, 0, s->width,
> s->height);
> -}
> -
> static int init_image(TiffContext *s, ThreadFrame *frame)
> {
> int ret;
> --
> 2.27.0
>
LGTM ... I wasn't sure about declaring a function more than a couple lines
long as inline.
More information about the ffmpeg-devel
mailing list