[FFmpeg-devel] [PATCH 4/4] avformat/avio: Move internal AVIOContext field to the end
Andreas Rheinhardt
andreas.rheinhardt at outlook.com
Wed Aug 4 20:08:59 EEST 2021
James Almer:
> On 8/4/2021 1:48 PM, Andreas Rheinhardt wrote:
>> James Almer:
>>> On 8/4/2021 1:24 PM, Andreas Rheinhardt wrote:
>>>> This gets them off the ABI altogether at the cost of breaking the ABI
>>>> once more now.
>>>>
>>>> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at outlook.com>
>>>> ---
>>>> Do this and the preceding patch need a version bump?
>>>
>>> You're not removing anything that was "public", so IMO no.
>>>
>>> Changes in offsets are ok since the latest AVPacket addition was a few
>>> days ago, so the ABI is not frozen yet (And at this rate wont be until
>>> we branch out a 5.0 release).
>>>
>>
>> Good. Because I intend to send another patchset that breaks ABI (but
>> only the avpriv-ABI, not the public ABI).
>>
>>>>
>>>> libavformat/avio.h | 98
>>>> ++++++++++++++++++++++------------------------
>>>> 1 file changed, 47 insertions(+), 51 deletions(-)
>>>>
>>>> diff --git a/libavformat/avio.h b/libavformat/avio.h
>>>> index 0b35409787..ebf43bfe15 100644
>>>> --- a/libavformat/avio.h
>>>> +++ b/libavformat/avio.h
>>>> @@ -148,9 +148,9 @@ enum AVIODataMarkerType {
>>>> /**
>>>> * Bytestream IO Context.
>>>> - * New fields can be added to the end with minor version bumps.
>>>> - * Removal, reordering and changes to existing fields require a major
>>>> - * version bump.
>>>> + * New public fields can be added with minor version bumps.
>>>> + * Removal, reordering and changes to existing public fields require
>>>> + * a major version bump.
>>>> * sizeof(AVIOContext) must not be used outside libav*.
>>>> *
>>>> * @note None of the function pointers in AVIOContext should be
>>>> called
>>>> @@ -237,12 +237,14 @@ typedef struct AVIOContext {
>>>> int64_t (*seek)(void *opaque, int64_t offset, int whence);
>>>> int64_t pos; /**< position in the file of the current
>>>> buffer */
>>>> int eof_reached; /**< true if was unable to read due to
>>>> error or eof */
>>>> + int error; /**< contains the error code or 0 if no
>>>> error happened */
>>>> int write_flag; /**< true if open for writing */
>>>> int max_packet_size;
>>>> + int min_packet_size; /**< Try to buffer at least this amount
>>>> of data
>>>> + before flushing it. */
>>>> unsigned long checksum;
>>>> unsigned char *checksum_ptr;
>>>> unsigned long (*update_checksum)(unsigned long checksum, const
>>>> uint8_t *buf, unsigned int size);
>>>> - int error; /**< contains the error code or 0 if no
>>>> error happened */
>>>> /**
>>>> * Pause or resume playback for network streaming protocols -
>>>> e.g. MMS.
>>>> */
>>>> @@ -259,12 +261,6 @@ typedef struct AVIOContext {
>>>> */
>>>> int seekable;
>>>> - /**
>>>> - * max filesize, used to limit allocations
>>>> - * This field is internal to libavformat and access from outside
>>>> is not allowed.
>>>> - */
>>>> - int64_t maxsize;
>>>> -
>>>> /**
>>>> * avio_read and avio_write should if possible be satisfied
>>>> directly
>>>> * instead of going through a buffer, and avio_seek will always
>>>> @@ -272,37 +268,6 @@ typedef struct AVIOContext {
>>>> */
>>>> int direct;
>>>> - /**
>>>> - * Bytes read statistic
>>>> - * This field is internal to libavformat and access from outside
>>>> is not allowed.
>>>> - */
>>>> - int64_t bytes_read;
>>>> -
>>>> - /**
>>>> - * seek statistic
>>>> - * This field is internal to libavformat and access from outside
>>>> is not allowed.
>>>> - */
>>>> - int seek_count;
>>>> -
>>>> - /**
>>>> - * writeout statistic
>>>> - * This field is internal to libavformat and access from outside
>>>> is not allowed.
>>>> - */
>>>> - int writeout_count;
>>>> -
>>>> - /**
>>>> - * Original buffer size
>>>> - * used internally after probing and ensure seekback to reset the
>>>> buffer size
>>>> - * This field is internal to libavformat and access from outside
>>>> is not allowed.
>>>> - */
>>>> - int orig_buffer_size;
>>>> -
>>>> - /**
>>>> - * Threshold to favor readahead over seek.
>>>> - * This is current internal only, do not use from outside.
>>>> - */
>>>> - int short_seek_threshold;
>>>> -
>>>> /**
>>>> * ',' separated list of allowed protocols.
>>>> */
>>>> @@ -325,30 +290,61 @@ typedef struct AVIOContext {
>>>> */
>>>> int ignore_boundary_point;
>>>> + int64_t written;
>>>> +
>>>> + /**
>>>> + * Maximum reached position before a backward seek in the write
>>>> buffer,
>>>> + * used keeping track of already written data for a later flush.
>>>> + */
>>>> + unsigned char *buf_ptr_max;
>>>> +
>>>> + /*****************************************************************
>>>> + * No fields below this line are part of the public API. They
>>>> + * may not be used outside of libavformat and can be changed and
>>>> + * removed at will.
>>>> + * New public fields should be added right above.
>>>> + *****************************************************************
>>>
>>> Since we got rid of this kind of notice from AVStream not too long ago,
>>> wouldn't it be better if we prevent it from happening here too?
>>> You could introduce a new AVIOInternal opaque struct and field here,
>>> much like we did with AVStreamInternal and AVFormatInternal to solve the
>>> same problem.
>>>
>>
>> This would add another allocation, so no. Unless you would be ok with
>> allocating said internal together with AVIOContext.
>
> Something similar to what you did in patch 1/1 for AVIOContext +
> DynBuffer in a single allocation? It should be ok if not too ugly. I'd
> rather not have non public fields in public headers if possible.
>
Ok, then I'll work on this.
- Andreas
More information about the ffmpeg-devel
mailing list