[FFmpeg-devel] [PATCH] Added support for MB_INFO

[Andreas Rheinhardt]

Anton Khirnov:
> Quoting Andreas Rheinhardt (2022-06-23 16:21:18)
>> Anton Khirnov:
>>> Quoting Carotti, Elias (2022-06-21 10:48:07)
>>>> Hi,
>>>>
>>>> extending AVVideoEncParams was the first hypothesis I made but it
>>>> didn't seem it was the proper place to add the mb_info flags.
>>>>
>>>> I may be wrong but my impression is that AVVideoEncParams is used to
>>>> carry encoding parameters read from the bitstream at the decoder side
>>>> while here were going the other direction, i.e., were passing
>>>> information from the application to the encoder.
>>>>
>>>> Secondly, mb_info can't be strictly considered encoding parameters and
>>>> it's not present in the bitstream at all. 
>>>> It's just a way to give hints to the libx264 encoder on which
>>>> macroblock we know have not changed since the previous frame and could
>>>> be coded as P_SKIPs. Libx264 however, may or may not oblige according
>>>> to its logic, and this specific information is not transmitted in the
>>>> bitstream nor can be recovered at the decoder.
>>>
>>> Right, seems I was too hasty in reading your patch.
>>>
>>> But then I have to wonder whether this really needs a new installed
>>> header, with a struct and a destructor, given that it's specific to a
>>> single encoder for a single codec that is about 20 years old now.
>>>
>>> Wouldn't AV_FRAME_DATA_X264_MBINFO that would be just a raw array of
>>> uint8_t serve your needs just as fine? You could even get custom buffer
>>> management by using AVFrameSideData.buf.
>>>
>>
>> There is one problem though: libx264's free functions don't accept an
>> opaque parameter, so one can't easily create a reference for libx264 to
>> unref. I don't see a way around duplicating this buffer in the encoder.
>> (Or is there a way to know when libx264 is done with using this buffer?)
> 
> An ugly, but workable hack could be
> - user allocates the AVBuffer with extra space at the end
> - lavc/libx264.c checks that there is extra space AND the buffer is
>   writable (so the same side data wasn't passed to multiple encoders),
>   then stores its AVBufferRef* in there
> 

Ugly? Certainly. Workable? Questionable: Depending upon how the user
uses the API the frame might not be writable when it reaches the encode
callback of our libx264 wrapper even if the user actually supplies
writable frames to avcodec_send_frame(). The reason is that if said
callback is called in avcodec_send_frame(), both the user's AVFrame as
well as the newly created reference to it (which is the frame that the
callback receives) will each own a reference to each side data.
The callback is called in avcodec_send_frame() if there is currently no
buffered packet ready to be forwarded to the user. This is true if the
user drains the encoder every time a frame has been sent. And that is
how we ordinarily use the API (our examples use this pattern as well as
ffmpeg.c). So I guess that this is the common way our API is used by
others as well.
(I think I once proposed adding a flag to muxers/encoders to select
whether an ownership transfer of the packet/frame is intended or not.)

- Andreas