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

[Andreas Rheinhardt]

James Almer:
> 
> 
> On 7/6/2022 4:09 PM, Andreas Rheinhardt wrote:
>> 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.
> 
> The idea is to keep the encoder working with user provided buffers,
> while not allocating worst case scenario packets.
> 

This could also be done generically:
https://github.com/mkver/FFmpeg/commit/4c06a1e457fd00f2ad14c1328587d29964c9fa11
You were against it because you wanted some kind of checks (I don't
remember the details); checks the current code does not have.

- Andreas