| Message ID | 20210609180741.4621-1-jamrial@gmail.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [FFmpeg-devel] avformat: make AVStream.pts_wrap_bits public | expand |
| 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 |
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 [...]
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 [...]
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". >
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 [...]
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". >
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 --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, \
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(-)