[FFmpeg-devel] [RFC][PATCH] avcodec: add a get_enc_buffer() callback to AVCodecContext

[James Almer]

On 2/20/2021 8:41 PM, Lynne wrote:
> Feb 20, 2021, 21:53 by jamrial at gmail.com:
> 
>> This callback is functionally the same as get_buffer2() is for decoders, and
>> implements for the new encode API the functionality of the old encode API had
>> where the user could provide their own buffers.
>>
>> Signed-off-by: James Almer <jamrial at gmail.com>
>> ---
>> As suggested by Anton, here's get_buffer() for encoders. This way, users of the
>> new encode API can still provide their own buffers, like they could with the
>> old API, now that it's being removed.
>>
>> The initial implementation of the default callback uses the existing method of
>> simply calling av_new_packet() for it. In the future, a buffer pool could be
>> used instead.
>> Is AV_GET_ENC_BUFFER_FLAG_REF useful here? Is there an encoder that keeps a
>> reference to a previous packet around?
>>
> 
> Could be useful, and keeps it symmetrical with decoding, so I'd keep it,
> just in case.
> 
> 
>> Alternative names for the callback field and public default callback function
>> are welcome, hence it being RFC.
>>
> 
> get_enc_buffer() -> get_encoder_buffer()
> avcodec_default_get_enc_buffer-> avcodec_default_get_encoder_buffer()
> That's all I'd suggest. It's just 4 more characters.

If others prefer that, I'll use it.

>   
> 
>> +    /**
>> +     * This callback is called at the beginning of each packet to get a data
>> +     * buffer for it.
>> +     *
>> +     * The following field will be set in the packet before this callback is
>> +     * called:
>> +     * - size (may by zero)
>> +     * This callback must use the above value to calculate the required buffer size,
>> +     * which must padded by at least AV_INPUT_BUFFER_PADDING_SIZE bytes.
>> +     *
>> +     * This callback must fill the following fields in the packet:
>> +     * - data
>> +     * - buf must contain a pointer to an AVBufferRef structure. The packet's
>> +     *   data pointer must be contained in it.
>> +     *   See: av_buffer_create(), av_buffer_alloc(), and av_buffer_ref().
>>
> 
> Does a size of zero mean you can have a NULL packet data and buffer
> or a AV_INPUT_BUFFER_PADDING_SIZE-sized buffer?

As i mentioned on IRC, calling ff_alloc_packet2() or av_new_packet() 
with size 0 is valid (it will indeed allocate an AVBufferRef of padding 
size bytes, with pkt->data pointing to it and pkt->size set to 0), so a 
function that replaces the former and essentially wraps the latter 
should probably also allow it.

I didn't want to keep ambiguous cases in the doxy, but i can remove that 
part. av_new_packet() doesn't mention it either after all.

> 
> 
>> +     *
>> +     * If AV_CODEC_CAP_DR1 is not set then get_enc_buffer() must call
>> +     * avcodec_default_get_enc_buffer() instead of providing a buffer allocated by
>> +     * some other means.
>> +     *
>> +     * If AV_GET_ENC_BUFFER_FLAG_REF is set in flags then the packet may be reused
>> +     * (read and/or written to if it is writable) later by libavcodec.
>> +     *
>> +     * @see avcodec_default_get_enc_buffer()
>> +     *
>> +     * - encoding: Set by libavcodec, user can override.
>> +     * - decoding: unused
>> +     */
>> +    int (*get_enc_buffer)(struct AVCodecContext *s, AVPacket *pkt, int flags);
>>
> 
> Maybe mention thread safety? Since in a frame-threaded encoder
> this may be called from different threads.

So copy paste the paragraph "If frame multithreading is used, this 
callback may be called from a different thread, but not from more than 
one at once. Does not need to be reentrant" from get_buffer2()?.

> 
> Rest looks good.
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
>