[FFmpeg-devel] [PATCH 4/4] avformat/avio: Move internal AVIOContext field to the end
James Almer
jamrial at gmail.com
Wed Aug 4 20:01:52 EEST 2021
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.
>
> (Contrary to AVStream the internals of AVIOContext are not used that
> widely, so it would be way less code to adapt.)
>
>>> + */
>>> + /**
>>> + * A callback that is used instead of short_seek_threshold.
>>> + * This is currently internal only, do not use from outside.
>>> + */
>>> + int (*short_seek_get)(void *opaque);
>>> +
>>> /**
>>> - * Internal, not meant to be used from outside of AVIOContext.
>>> + * Threshold to favor readahead over seek.
>>> + * This is currently internal only, do not use from outside.
>>> */
>>> + int short_seek_threshold;
>>> +
>>> enum AVIODataMarkerType current_type;
>>> int64_t last_time;
>>> /**
>>> - * A callback that is used instead of short_seek_threshold.
>>> - * This is current internal only, do not use from outside.
>>> + * max filesize, used to limit allocations
>>> */
>>> - int (*short_seek_get)(void *opaque);
>>> + int64_t maxsize;
>>> - int64_t written;
>>> + /**
>>> + * Bytes read statistic
>>> + */
>>> + int64_t bytes_read;
>>> /**
>>> - * Maximum reached position before a backward seek in the write
>>> buffer,
>>> - * used keeping track of already written data for a later flush.
>>> + * seek statistic
>>> */
>>> - unsigned char *buf_ptr_max;
>>> + int seek_count;
>>> +
>>> + /**
>>> + * writeout statistic
>>> + */
>>> + int writeout_count;
>>> /**
>>> - * Try to buffer at least this amount of data before flushing it
>>> + * Original buffer size
>>> + * used after probing to ensure seekback and to reset the buffer
>>> size
>>> */
>>> - int min_packet_size;
>>> + int orig_buffer_size;
>>> } AVIOContext;
>>> /**
>>>
>>
>
> _______________________________________________
> 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".
>
More information about the ffmpeg-devel
mailing list