diff mbox series

[FFmpeg-devel] avformat: make AVStream.pts_wrap_bits public

Message ID 20210609180741.4621-1-jamrial@gmail.com
State New
Headers show
Series [FFmpeg-devel] avformat: make AVStream.pts_wrap_bits public
Related show

Checks

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

Commit Message

James Almer June 9, 2021, 6:07 p.m. UTC
It can be useful to library users, and is currently being used by ffmpeg.c

Suggested-by: Hendrik Leppkes <h.leppkes@gmail.com>
Signed-off-by: James Almer <jamrial@gmail.com>
---
 doc/APIchanges         |  3 +++
 libavformat/avformat.h | 17 +++++++----------
 libavformat/version.h  |  4 ++--
 3 files changed, 12 insertions(+), 12 deletions(-)

Comments

Michael Niedermayer June 10, 2021, 6:15 p.m. UTC | #1
On Wed, Jun 09, 2021 at 03:07:41PM -0300, James Almer wrote:
> It can be useful to library users, and is currently being used by ffmpeg.c
> 
> Suggested-by: Hendrik Leppkes <h.leppkes@gmail.com>
> Signed-off-by: James Almer <jamrial@gmail.com>
> ---
>  doc/APIchanges         |  3 +++
>  libavformat/avformat.h | 17 +++++++----------
>  libavformat/version.h  |  4 ++--
>  3 files changed, 12 insertions(+), 12 deletions(-)
> 
> diff --git a/doc/APIchanges b/doc/APIchanges
> index c46f4d5304..1b25bddd43 100644
> --- a/doc/APIchanges
> +++ b/doc/APIchanges
> @@ -14,6 +14,9 @@ libavutil:     2021-04-27
>  
>  API changes, most recent first:
>  
> +2021-06-09 - xxxxxxxxxx - lavf 59.3.100 - avformat.h
> +  Add pts_wrap_bits to AVStream
> +
>  2021-04-27 - cb3ac722f4 - lavc 59.0.100 - avcodec.h
>    Constified AVCodecParserContext.parser.
>  

> diff --git a/libavformat/avformat.h b/libavformat/avformat.h
> index 094683f12a..0d12d5b0d2 100644
> --- a/libavformat/avformat.h
> +++ b/libavformat/avformat.h
> @@ -980,17 +980,14 @@ typedef struct AVStream {
>       */
>      AVCodecParameters *codecpar;
>  
> -    /*****************************************************************
> -     * All fields below this line are not part of the public API. They
> -     * may not be used outside of libavformat and can be changed and
> -     * removed at will.
> -     * Internal note: be aware that physically removing these fields
> -     * will break ABI. Replace removed fields with dummy fields, and
> -     * add new fields to AVStreamInternal.
> -     *****************************************************************
> +    /**
> +     * Number of bits in pts. Used for wrapping control.

pts and dts or in timestamps, its not just pts (no need to fix it in this patch
as its just copied as i realize) but it should be fixed when its made public

patch LGTM

thx

[...]
Michael Niedermayer June 10, 2021, 6:18 p.m. UTC | #2
On Wed, Jun 09, 2021 at 03:07:41PM -0300, James Almer wrote:
> It can be useful to library users, and is currently being used by ffmpeg.c
> 
> Suggested-by: Hendrik Leppkes <h.leppkes@gmail.com>
> Signed-off-by: James Almer <jamrial@gmail.com>
> ---
>  doc/APIchanges         |  3 +++
>  libavformat/avformat.h | 17 +++++++----------
>  libavformat/version.h  |  4 ++--
>  3 files changed, 12 insertions(+), 12 deletions(-)
> 
> diff --git a/doc/APIchanges b/doc/APIchanges
> index c46f4d5304..1b25bddd43 100644
> --- a/doc/APIchanges
> +++ b/doc/APIchanges
> @@ -14,6 +14,9 @@ libavutil:     2021-04-27
>  
>  API changes, most recent first:
>  
> +2021-06-09 - xxxxxxxxxx - lavf 59.3.100 - avformat.h
> +  Add pts_wrap_bits to AVStream
> +
>  2021-04-27 - cb3ac722f4 - lavc 59.0.100 - avcodec.h
>    Constified AVCodecParserContext.parser.
>  
> diff --git a/libavformat/avformat.h b/libavformat/avformat.h
> index 094683f12a..0d12d5b0d2 100644
> --- a/libavformat/avformat.h
> +++ b/libavformat/avformat.h
> @@ -980,17 +980,14 @@ typedef struct AVStream {
>       */
>      AVCodecParameters *codecpar;
>  
> -    /*****************************************************************
> -     * All fields below this line are not part of the public API. They
> -     * may not be used outside of libavformat and can be changed and
> -     * removed at will.
> -     * Internal note: be aware that physically removing these fields
> -     * will break ABI. Replace removed fields with dummy fields, and
> -     * add new fields to AVStreamInternal.
> -     *****************************************************************
> +    /**
> +     * Number of bits in pts. Used for wrapping control.
> +     *
> +     * - demuxing: set by libavformat
> +     * - muxing: set by libavformat
> +     *
>       */
> -
> -    int pts_wrap_bits; /**< number of bits in pts (used for wrapping control) */
> +    int pts_wrap_bits;
>  
>      /**
>       * An opaque field for libavformat internal usage.

The "All fields below this line..." thing should be moved down instead of
removed as i realize that this was not the last field

thx

[...]
James Almer June 10, 2021, 6:27 p.m. UTC | #3
On 6/10/2021 3:18 PM, Michael Niedermayer wrote:
> On Wed, Jun 09, 2021 at 03:07:41PM -0300, James Almer wrote:
>> It can be useful to library users, and is currently being used by ffmpeg.c
>>
>> Suggested-by: Hendrik Leppkes <h.leppkes@gmail.com>
>> Signed-off-by: James Almer <jamrial@gmail.com>
>> ---
>>   doc/APIchanges         |  3 +++
>>   libavformat/avformat.h | 17 +++++++----------
>>   libavformat/version.h  |  4 ++--
>>   3 files changed, 12 insertions(+), 12 deletions(-)
>>
>> diff --git a/doc/APIchanges b/doc/APIchanges
>> index c46f4d5304..1b25bddd43 100644
>> --- a/doc/APIchanges
>> +++ b/doc/APIchanges
>> @@ -14,6 +14,9 @@ libavutil:     2021-04-27
>>   
>>   API changes, most recent first:
>>   
>> +2021-06-09 - xxxxxxxxxx - lavf 59.3.100 - avformat.h
>> +  Add pts_wrap_bits to AVStream
>> +
>>   2021-04-27 - cb3ac722f4 - lavc 59.0.100 - avcodec.h
>>     Constified AVCodecParserContext.parser.
>>   
>> diff --git a/libavformat/avformat.h b/libavformat/avformat.h
>> index 094683f12a..0d12d5b0d2 100644
>> --- a/libavformat/avformat.h
>> +++ b/libavformat/avformat.h
>> @@ -980,17 +980,14 @@ typedef struct AVStream {
>>        */
>>       AVCodecParameters *codecpar;
>>   
>> -    /*****************************************************************
>> -     * All fields below this line are not part of the public API. They
>> -     * may not be used outside of libavformat and can be changed and
>> -     * removed at will.
>> -     * Internal note: be aware that physically removing these fields
>> -     * will break ABI. Replace removed fields with dummy fields, and
>> -     * add new fields to AVStreamInternal.
>> -     *****************************************************************
>> +    /**
>> +     * Number of bits in pts. Used for wrapping control.
>> +     *
>> +     * - demuxing: set by libavformat
>> +     * - muxing: set by libavformat
>> +     *
>>        */
>> -
>> -    int pts_wrap_bits; /**< number of bits in pts (used for wrapping control) */
>> +    int pts_wrap_bits;
>>   
>>       /**
>>        * An opaque field for libavformat internal usage.
> 
> The "All fields below this line..." thing should be moved down instead of
> removed as i realize that this was not the last field

No, the remaining field is the AVStreamInternal opaque one. Like in 
other structs, it's public in the sense its offset is fixed, but should 
not be accessed by the user.

One of purposes of this patch is precisely get rid of that ugly notice, 
so new fields are added directly at the end, and existing offsets can 
remain fixed within a given soname.

> 
> thx
> 
> [...]
> 
> 
> _______________________________________________
> 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".
>
Michael Niedermayer June 11, 2021, 12:03 p.m. UTC | #4
On Thu, Jun 10, 2021 at 03:27:37PM -0300, James Almer wrote:
> On 6/10/2021 3:18 PM, Michael Niedermayer wrote:
> > On Wed, Jun 09, 2021 at 03:07:41PM -0300, James Almer wrote:
> > > It can be useful to library users, and is currently being used by ffmpeg.c
> > > 
> > > Suggested-by: Hendrik Leppkes <h.leppkes@gmail.com>
> > > Signed-off-by: James Almer <jamrial@gmail.com>
> > > ---
> > >   doc/APIchanges         |  3 +++
> > >   libavformat/avformat.h | 17 +++++++----------
> > >   libavformat/version.h  |  4 ++--
> > >   3 files changed, 12 insertions(+), 12 deletions(-)
> > > 
> > > diff --git a/doc/APIchanges b/doc/APIchanges
> > > index c46f4d5304..1b25bddd43 100644
> > > --- a/doc/APIchanges
> > > +++ b/doc/APIchanges
> > > @@ -14,6 +14,9 @@ libavutil:     2021-04-27
> > >   API changes, most recent first:
> > > +2021-06-09 - xxxxxxxxxx - lavf 59.3.100 - avformat.h
> > > +  Add pts_wrap_bits to AVStream
> > > +
> > >   2021-04-27 - cb3ac722f4 - lavc 59.0.100 - avcodec.h
> > >     Constified AVCodecParserContext.parser.
> > > diff --git a/libavformat/avformat.h b/libavformat/avformat.h
> > > index 094683f12a..0d12d5b0d2 100644
> > > --- a/libavformat/avformat.h
> > > +++ b/libavformat/avformat.h
> > > @@ -980,17 +980,14 @@ typedef struct AVStream {
> > >        */
> > >       AVCodecParameters *codecpar;
> > > -    /*****************************************************************
> > > -     * All fields below this line are not part of the public API. They
> > > -     * may not be used outside of libavformat and can be changed and
> > > -     * removed at will.
> > > -     * Internal note: be aware that physically removing these fields
> > > -     * will break ABI. Replace removed fields with dummy fields, and
> > > -     * add new fields to AVStreamInternal.
> > > -     *****************************************************************
> > > +    /**
> > > +     * Number of bits in pts. Used for wrapping control.
> > > +     *
> > > +     * - demuxing: set by libavformat
> > > +     * - muxing: set by libavformat
> > > +     *
> > >        */
> > > -
> > > -    int pts_wrap_bits; /**< number of bits in pts (used for wrapping control) */
> > > +    int pts_wrap_bits;
> > >       /**
> > >        * An opaque field for libavformat internal usage.
> > 
> > The "All fields below this line..." thing should be moved down instead of
> > removed as i realize that this was not the last field
> 
> No, the remaining field is the AVStreamInternal opaque one. Like in other
> structs, it's public in the sense its offset is fixed, but should not be
> accessed by the user.
> 
> One of purposes of this patch is precisely get rid of that ugly notice, so
> new fields are added directly at the end, and existing offsets can remain
> fixed within a given soname.

If you apply this patch there are 3 fields not 1 afterwards, the other 2 are
int64_t first_dts;
int64_t cur_dts;

cur_dts is certainly not supposed to be a public field

now i see, where my mistake probably was, this is supposed to be applied
with other patches removing these fields first. Though this patch does
apply on its own here.

thx

[...]
James Almer June 11, 2021, 12:34 p.m. UTC | #5
On 6/11/2021 9:03 AM, Michael Niedermayer wrote:
> On Thu, Jun 10, 2021 at 03:27:37PM -0300, James Almer wrote:
>> On 6/10/2021 3:18 PM, Michael Niedermayer wrote:
>>> On Wed, Jun 09, 2021 at 03:07:41PM -0300, James Almer wrote:
>>>> It can be useful to library users, and is currently being used by ffmpeg.c
>>>>
>>>> Suggested-by: Hendrik Leppkes <h.leppkes@gmail.com>
>>>> Signed-off-by: James Almer <jamrial@gmail.com>
>>>> ---
>>>>    doc/APIchanges         |  3 +++
>>>>    libavformat/avformat.h | 17 +++++++----------
>>>>    libavformat/version.h  |  4 ++--
>>>>    3 files changed, 12 insertions(+), 12 deletions(-)
>>>>
>>>> diff --git a/doc/APIchanges b/doc/APIchanges
>>>> index c46f4d5304..1b25bddd43 100644
>>>> --- a/doc/APIchanges
>>>> +++ b/doc/APIchanges
>>>> @@ -14,6 +14,9 @@ libavutil:     2021-04-27
>>>>    API changes, most recent first:
>>>> +2021-06-09 - xxxxxxxxxx - lavf 59.3.100 - avformat.h
>>>> +  Add pts_wrap_bits to AVStream
>>>> +
>>>>    2021-04-27 - cb3ac722f4 - lavc 59.0.100 - avcodec.h
>>>>      Constified AVCodecParserContext.parser.
>>>> diff --git a/libavformat/avformat.h b/libavformat/avformat.h
>>>> index 094683f12a..0d12d5b0d2 100644
>>>> --- a/libavformat/avformat.h
>>>> +++ b/libavformat/avformat.h
>>>> @@ -980,17 +980,14 @@ typedef struct AVStream {
>>>>         */
>>>>        AVCodecParameters *codecpar;
>>>> -    /*****************************************************************
>>>> -     * All fields below this line are not part of the public API. They
>>>> -     * may not be used outside of libavformat and can be changed and
>>>> -     * removed at will.
>>>> -     * Internal note: be aware that physically removing these fields
>>>> -     * will break ABI. Replace removed fields with dummy fields, and
>>>> -     * add new fields to AVStreamInternal.
>>>> -     *****************************************************************
>>>> +    /**
>>>> +     * Number of bits in pts. Used for wrapping control.
>>>> +     *
>>>> +     * - demuxing: set by libavformat
>>>> +     * - muxing: set by libavformat
>>>> +     *
>>>>         */
>>>> -
>>>> -    int pts_wrap_bits; /**< number of bits in pts (used for wrapping control) */
>>>> +    int pts_wrap_bits;
>>>>        /**
>>>>         * An opaque field for libavformat internal usage.
>>>
>>> The "All fields below this line..." thing should be moved down instead of
>>> removed as i realize that this was not the last field
>>
>> No, the remaining field is the AVStreamInternal opaque one. Like in other
>> structs, it's public in the sense its offset is fixed, but should not be
>> accessed by the user.
>>
>> One of purposes of this patch is precisely get rid of that ugly notice, so
>> new fields are added directly at the end, and existing offsets can remain
>> fixed within a given soname.
> 
> If you apply this patch there are 3 fields not 1 afterwards, the other 2 are
> int64_t first_dts;
> int64_t cur_dts;
> 
> cur_dts is certainly not supposed to be a public field
> 
> now i see, where my mistake probably was, this is supposed to be applied
> with other patches removing these fields first. Though this patch does
> apply on its own here.

I sent this patch after i had pushed the one removing the two fields you 
mention above.
Were you maybe looking at a tree without the latest commits? I guess 
this patch could still apply just fine without the other changes if they 
were not already in your tree.

> 
> thx
> 
> [...]
> 
> 
> _______________________________________________
> 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".
>
Michael Niedermayer June 11, 2021, 2:13 p.m. UTC | #6
On Fri, Jun 11, 2021 at 09:34:10AM -0300, James Almer wrote:
> On 6/11/2021 9:03 AM, Michael Niedermayer wrote:
> > On Thu, Jun 10, 2021 at 03:27:37PM -0300, James Almer wrote:
> > > On 6/10/2021 3:18 PM, Michael Niedermayer wrote:
> > > > On Wed, Jun 09, 2021 at 03:07:41PM -0300, James Almer wrote:
> > > > > It can be useful to library users, and is currently being used by ffmpeg.c
> > > > > 
> > > > > Suggested-by: Hendrik Leppkes <h.leppkes@gmail.com>
> > > > > Signed-off-by: James Almer <jamrial@gmail.com>
> > > > > ---
> > > > >    doc/APIchanges         |  3 +++
> > > > >    libavformat/avformat.h | 17 +++++++----------
> > > > >    libavformat/version.h  |  4 ++--
> > > > >    3 files changed, 12 insertions(+), 12 deletions(-)
> > > > > 
> > > > > diff --git a/doc/APIchanges b/doc/APIchanges
> > > > > index c46f4d5304..1b25bddd43 100644
> > > > > --- a/doc/APIchanges
> > > > > +++ b/doc/APIchanges
> > > > > @@ -14,6 +14,9 @@ libavutil:     2021-04-27
> > > > >    API changes, most recent first:
> > > > > +2021-06-09 - xxxxxxxxxx - lavf 59.3.100 - avformat.h
> > > > > +  Add pts_wrap_bits to AVStream
> > > > > +
> > > > >    2021-04-27 - cb3ac722f4 - lavc 59.0.100 - avcodec.h
> > > > >      Constified AVCodecParserContext.parser.
> > > > > diff --git a/libavformat/avformat.h b/libavformat/avformat.h
> > > > > index 094683f12a..0d12d5b0d2 100644
> > > > > --- a/libavformat/avformat.h
> > > > > +++ b/libavformat/avformat.h
> > > > > @@ -980,17 +980,14 @@ typedef struct AVStream {
> > > > >         */
> > > > >        AVCodecParameters *codecpar;
> > > > > -    /*****************************************************************
> > > > > -     * All fields below this line are not part of the public API. They
> > > > > -     * may not be used outside of libavformat and can be changed and
> > > > > -     * removed at will.
> > > > > -     * Internal note: be aware that physically removing these fields
> > > > > -     * will break ABI. Replace removed fields with dummy fields, and
> > > > > -     * add new fields to AVStreamInternal.
> > > > > -     *****************************************************************
> > > > > +    /**
> > > > > +     * Number of bits in pts. Used for wrapping control.
> > > > > +     *
> > > > > +     * - demuxing: set by libavformat
> > > > > +     * - muxing: set by libavformat
> > > > > +     *
> > > > >         */
> > > > > -
> > > > > -    int pts_wrap_bits; /**< number of bits in pts (used for wrapping control) */
> > > > > +    int pts_wrap_bits;
> > > > >        /**
> > > > >         * An opaque field for libavformat internal usage.
> > > > 
> > > > The "All fields below this line..." thing should be moved down instead of
> > > > removed as i realize that this was not the last field
> > > 
> > > No, the remaining field is the AVStreamInternal opaque one. Like in other
> > > structs, it's public in the sense its offset is fixed, but should not be
> > > accessed by the user.
> > > 
> > > One of purposes of this patch is precisely get rid of that ugly notice, so
> > > new fields are added directly at the end, and existing offsets can remain
> > > fixed within a given soname.
> > 
> > If you apply this patch there are 3 fields not 1 afterwards, the other 2 are
> > int64_t first_dts;
> > int64_t cur_dts;
> > 
> > cur_dts is certainly not supposed to be a public field
> > 
> > now i see, where my mistake probably was, this is supposed to be applied
> > with other patches removing these fields first. Though this patch does
> > apply on its own here.
> 
> I sent this patch after i had pushed the one removing the two fields you
> mention above.
> Were you maybe looking at a tree without the latest commits? I guess this
> patch could still apply just fine without the other changes if they were not
> already in your tree.

yes

[...]
diff mbox series

Patch

diff --git a/doc/APIchanges b/doc/APIchanges
index c46f4d5304..1b25bddd43 100644
--- a/doc/APIchanges
+++ b/doc/APIchanges
@@ -14,6 +14,9 @@  libavutil:     2021-04-27
 
 API changes, most recent first:
 
+2021-06-09 - xxxxxxxxxx - lavf 59.3.100 - avformat.h
+  Add pts_wrap_bits to AVStream
+
 2021-04-27 - cb3ac722f4 - lavc 59.0.100 - avcodec.h
   Constified AVCodecParserContext.parser.
 
diff --git a/libavformat/avformat.h b/libavformat/avformat.h
index 094683f12a..0d12d5b0d2 100644
--- a/libavformat/avformat.h
+++ b/libavformat/avformat.h
@@ -980,17 +980,14 @@  typedef struct AVStream {
      */
     AVCodecParameters *codecpar;
 
-    /*****************************************************************
-     * All fields below this line are not part of the public API. They
-     * may not be used outside of libavformat and can be changed and
-     * removed at will.
-     * Internal note: be aware that physically removing these fields
-     * will break ABI. Replace removed fields with dummy fields, and
-     * add new fields to AVStreamInternal.
-     *****************************************************************
+    /**
+     * Number of bits in pts. Used for wrapping control.
+     *
+     * - demuxing: set by libavformat
+     * - muxing: set by libavformat
+     *
      */
-
-    int pts_wrap_bits; /**< number of bits in pts (used for wrapping control) */
+    int pts_wrap_bits;
 
     /**
      * An opaque field for libavformat internal usage.
diff --git a/libavformat/version.h b/libavformat/version.h
index ecea39d59c..7f02e18f24 100644
--- a/libavformat/version.h
+++ b/libavformat/version.h
@@ -32,8 +32,8 @@ 
 // Major bumping may affect Ticket5467, 5421, 5451(compatibility with Chromium)
 // Also please add any ticket numbers that you believe might be affected here
 #define LIBAVFORMAT_VERSION_MAJOR  59
-#define LIBAVFORMAT_VERSION_MINOR   2
-#define LIBAVFORMAT_VERSION_MICRO 102
+#define LIBAVFORMAT_VERSION_MINOR   3
+#define LIBAVFORMAT_VERSION_MICRO 100
 
 #define LIBAVFORMAT_VERSION_INT AV_VERSION_INT(LIBAVFORMAT_VERSION_MAJOR, \
                                                LIBAVFORMAT_VERSION_MINOR, \