Re: [FFmpeg-devel] [PATCH] avcodec: add Radiance HDR image format support

[Andreas Rheinhardt <andreas.rheinhardt@outlook.com>]

James Almer:
> On 7/6/2022 3:47 PM, Paul B Mahol wrote:
>> Added parser.
>> The test will be added after this is merged.
> 
> [...]
> 
>> +static int hdr_parse(AVCodecParserContext *s, AVCodecContext *avctx,
>> +                     const uint8_t **poutbuf, int *poutbuf_size,
>> +                     const uint8_t *buf, int buf_size)
>> +{
>> +    HDRParseContext *ipc = s->priv_data;
>> +    uint64_t state = ipc->pc.state64;
>> +    int next = END_NOT_FOUND, i = 0;
>> +
>> +    s->pict_type = AV_PICTURE_TYPE_NONE;
> 
> pict_type I, and key frame?
> 
> [...]
> 
>> +static int hdr_encode_frame(AVCodecContext *avctx, AVPacket *pkt,
>> +                            const AVFrame *frame, int *got_packet)
>> +{
>> +    HDREncContext *s = avctx->priv_data;
>> +    int64_t packet_size;
>> +    uint8_t *buf;
>> +    int ret;
>> +
>> +    packet_size = avctx->width * avctx->height * 4LL + 1024LL;
>> +    if ((ret = ff_get_encode_buffer(avctx, pkt, packet_size, 0)) < 0)
> 
> Wont using worst case scenario like this result in massive packets?
> av_shrink_packet() only changes the size field, it doesn't realloc the
> buffer to shrink it.
> 
> You could allocate a buffer in HDREncContext with
> av_fast_padded_malloc() here, use the bytestream2 API below, then
> allocate the packet with ff_get_encode_buffer() using
> bytestream2_tell_p() as size and do a memcpy at the end.
> 

If that is the aim, then there is no reason to do the memcpy ourselves
(or do the av_fast_padded_malloc directly): Just use ff_alloc_packet()
and the generic code will ensure that the packet data will be made
refcounted. And the temp buffer will be freed generically, too.

>> +        return ret;
>> +
>> +    buf = pkt->data;
>> +    bytestream_put_str(&buf, "#?RADIANCE\n");
>> +    bytestream_put_str(&buf, "SOFTWARE=lavc\n");
>> +    ret = snprintf(buf, 32, "PIXASPECT=%f\n",
>> av_q2d(av_inv_q(avctx->sample_aspect_ratio)));
>> +    if (ret > 0)
>> +        buf += ret;
>> +    bytestream_put_str(&buf, "FORMAT=32-bit_rle_rgbe\n\n");
>> +    ret = snprintf(buf, 32, "-Y %d +X %d\n", avctx->height,
>> avctx->width);
>> +    if (ret > 0)
>> +        buf += ret;
>> +
>> +    for (int y = 0; y < avctx->height; y++) {
>> +        const float *red   = (const float *)(frame->data[2] + y *
>> frame->linesize[2]);
>> +        const float *green = (const float *)(frame->data[0] + y *
>> frame->linesize[0]);
>> +        const float *blue  = (const float *)(frame->data[1] + y *
>> frame->linesize[1]);
>> +
>> +        if (avctx->width < 8 || avctx->width > 0x7fff) {
>> +            for (int x = 0; x < avctx->width; x++) {
>> +                float2rgbe(buf, red[x], green[x], blue[x]);
>> +                buf += 4;
>> +            }
>> +        } else {
>> +            bytestream_put_byte(&buf, 2);
>> +            bytestream_put_byte(&buf, 2);
>> +            bytestream_put_byte(&buf, avctx->width >> 8);
>> +            bytestream_put_byte(&buf, avctx->width & 0xFF);
>> +
>> +            for (int x = 0; x < avctx->width; x++)
>> +                float2rgbe(s->scanline + 4 * x, red[x], green[x],
>> blue[x]);
>> +            for (int p = 0; p < 4; p++)
>> +                rle(&buf, s->scanline + p, avctx->width);
>> +        }
>> +    }
>> +
>> +    pkt->flags |= AV_PKT_FLAG_KEY;
>> +
>> +    av_shrink_packet(pkt, buf - pkt->data);
>> +
>> +    *got_packet = 1;
>> +
>> +    return 0;
>> +}
> 
> _______________________________________________
> 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".

_______________________________________________
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".