diff mbox series

[FFmpeg-devel,4/4] avformat/avio: Move internal AVIOContext field to the end

Message ID AM7PR03MB666023756B606F0CB77EA9068FF19@AM7PR03MB6660.eurprd03.prod.outlook.com
State Superseded
Headers show
Series [FFmpeg-devel,1/4] avformat/aviobuf: Avoid allocation when using dynamic buffer | expand

Checks

Context Check Description
andriy/x86_make success Make finished
andriy/x86_make_fate success Make fate finished
andriy/PPC64_make warning Make failed

Commit Message

Andreas Rheinhardt Aug. 4, 2021, 4:24 p.m. UTC
This gets them off the ABI altogether at the cost of breaking the ABI
once more now.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
Do this and the preceding patch need a version bump?

 libavformat/avio.h | 98 ++++++++++++++++++++++------------------------
 1 file changed, 47 insertions(+), 51 deletions(-)

Comments

James Almer Aug. 4, 2021, 4:37 p.m. UTC | #1
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@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).

> 
>   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.

> +     */
> +    /**
> +     * 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;
>   
>   /**
>
Andreas Rheinhardt Aug. 4, 2021, 4:48 p.m. UTC | #2
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@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.

(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;
>>     /**
>>
>
Nicolas George Aug. 4, 2021, 4:49 p.m. UTC | #3
Andreas Rheinhardt (12021-08-04):
> Good. Because I intend to send another patchset that breaks ABI (but
> only the avpriv-ABI, not the public ABI).

If we lock all libraries version together, we never have to worry about
avpriv ABI.

Regards,
James Almer Aug. 4, 2021, 5:01 p.m. UTC | #4
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@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@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>
Andreas Rheinhardt Aug. 4, 2021, 5:08 p.m. UTC | #5
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@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
diff mbox series

Patch

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.
+     *****************************************************************
+     */
+    /**
+     * 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;
 
 /**