| Message ID | 20210316082953.273550-2-andreas.rheinhardt@gmail.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [FFmpeg-devel,1/2] avformat/matroskaenc: Check chapter ids for duplicates | 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 |
Quoting Andreas Rheinhardt (2021-03-16 09:29:53) > 64 bits are needed in order to retain the uid values of Matroska > chapters; the type is kept signed because the semantics of NUT chapters > depend upon whether the id is > 0 or < 0. > > Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com> > --- > Apologies for being so late. > > doc/APIchanges | 4 ++++ > libavformat/aadec.c | 2 +- > libavformat/avformat.h | 4 ++++ > libavformat/internal.h | 4 ++++ > libavformat/matroskaenc.c | 4 ++++ > libavformat/nutdec.c | 4 ++-- > libavformat/utils.c | 4 ++++ > libavformat/version.h | 5 ++++- > 8 files changed, 27 insertions(+), 4 deletions(-) > > diff --git a/doc/APIchanges b/doc/APIchanges > index c0d955b1fa..8b93adebe1 100644 > --- a/doc/APIchanges > +++ b/doc/APIchanges > @@ -15,6 +15,10 @@ libavutil: 2017-10-21 > > API changes, most recent first: > > +2021-03-16 - xxxxxxxxxx - lavf 58.75.100 - avformat.h > + AVChapter.id will be changed from int to int64_t > + on the next major version bump. > + > 2021-03-12 - xxxxxxxxxx - lavc 58.131.100 - avcodec.h codec.h > Add a get_encode_buffer callback to AVCodecContext, similar to > get_buffer2 but for encoders. > diff --git a/libavformat/aadec.c b/libavformat/aadec.c > index e88cdb89df..80ca2c12d7 100644 > --- a/libavformat/aadec.c > +++ b/libavformat/aadec.c > @@ -222,7 +222,7 @@ static int aa_read_header(AVFormatContext *s) > c->content_end = start + largest_size; > > while ((chapter_pos = avio_tell(pb)) >= 0 && chapter_pos < c->content_end) { > - int chapter_idx = s->nb_chapters; > + unsigned chapter_idx = s->nb_chapters; unrelated?
Anton Khirnov: > Quoting Andreas Rheinhardt (2021-03-16 09:29:53) >> 64 bits are needed in order to retain the uid values of Matroska >> chapters; the type is kept signed because the semantics of NUT chapters >> depend upon whether the id is > 0 or < 0. >> >> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com> >> --- >> Apologies for being so late. >> >> doc/APIchanges | 4 ++++ >> libavformat/aadec.c | 2 +- >> libavformat/avformat.h | 4 ++++ >> libavformat/internal.h | 4 ++++ >> libavformat/matroskaenc.c | 4 ++++ >> libavformat/nutdec.c | 4 ++-- >> libavformat/utils.c | 4 ++++ >> libavformat/version.h | 5 ++++- >> 8 files changed, 27 insertions(+), 4 deletions(-) >> >> diff --git a/doc/APIchanges b/doc/APIchanges >> index c0d955b1fa..8b93adebe1 100644 >> --- a/doc/APIchanges >> +++ b/doc/APIchanges >> @@ -15,6 +15,10 @@ libavutil: 2017-10-21 >> >> API changes, most recent first: >> >> +2021-03-16 - xxxxxxxxxx - lavf 58.75.100 - avformat.h >> + AVChapter.id will be changed from int to int64_t >> + on the next major version bump. >> + >> 2021-03-12 - xxxxxxxxxx - lavc 58.131.100 - avcodec.h codec.h >> Add a get_encode_buffer callback to AVCodecContext, similar to >> get_buffer2 but for encoders. >> diff --git a/libavformat/aadec.c b/libavformat/aadec.c >> index e88cdb89df..80ca2c12d7 100644 >> --- a/libavformat/aadec.c >> +++ b/libavformat/aadec.c >> @@ -222,7 +222,7 @@ static int aa_read_header(AVFormatContext *s) >> c->content_end = start + largest_size; >> >> while ((chapter_pos = avio_tell(pb)) >= 0 && chapter_pos < c->content_end) { >> - int chapter_idx = s->nb_chapters; >> + unsigned chapter_idx = s->nb_chapters; > > unrelated? > The chapter ids created by aadec are just 0,1,... And in the hypothetical scenario that there are more than INT_MAX of them the INT_MAX+1U chapter would have a negative id if I didn't change it to unsigned above. (I don't think it can happen, but nb_chapters is an unsigned, so it is better anyway.) - Andreas
Quoting Andreas Rheinhardt (2021-03-16 10:34:22) > Anton Khirnov: > > Quoting Andreas Rheinhardt (2021-03-16 09:29:53) > >> 64 bits are needed in order to retain the uid values of Matroska > >> chapters; the type is kept signed because the semantics of NUT chapters > >> depend upon whether the id is > 0 or < 0. > >> > >> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com> > >> --- > >> Apologies for being so late. > >> > >> doc/APIchanges | 4 ++++ > >> libavformat/aadec.c | 2 +- > >> libavformat/avformat.h | 4 ++++ > >> libavformat/internal.h | 4 ++++ > >> libavformat/matroskaenc.c | 4 ++++ > >> libavformat/nutdec.c | 4 ++-- > >> libavformat/utils.c | 4 ++++ > >> libavformat/version.h | 5 ++++- > >> 8 files changed, 27 insertions(+), 4 deletions(-) > >> > >> diff --git a/doc/APIchanges b/doc/APIchanges > >> index c0d955b1fa..8b93adebe1 100644 > >> --- a/doc/APIchanges > >> +++ b/doc/APIchanges > >> @@ -15,6 +15,10 @@ libavutil: 2017-10-21 > >> > >> API changes, most recent first: > >> > >> +2021-03-16 - xxxxxxxxxx - lavf 58.75.100 - avformat.h > >> + AVChapter.id will be changed from int to int64_t > >> + on the next major version bump. > >> + > >> 2021-03-12 - xxxxxxxxxx - lavc 58.131.100 - avcodec.h codec.h > >> Add a get_encode_buffer callback to AVCodecContext, similar to > >> get_buffer2 but for encoders. > >> diff --git a/libavformat/aadec.c b/libavformat/aadec.c > >> index e88cdb89df..80ca2c12d7 100644 > >> --- a/libavformat/aadec.c > >> +++ b/libavformat/aadec.c > >> @@ -222,7 +222,7 @@ static int aa_read_header(AVFormatContext *s) > >> c->content_end = start + largest_size; > >> > >> while ((chapter_pos = avio_tell(pb)) >= 0 && chapter_pos < c->content_end) { > >> - int chapter_idx = s->nb_chapters; > >> + unsigned chapter_idx = s->nb_chapters; > > > > unrelated? > > > The chapter ids created by aadec are just 0,1,... And in the > hypothetical scenario that there are more than INT_MAX of them the > INT_MAX+1U chapter would have a negative id if I didn't change it to > unsigned above. (I don't think it can happen, but nb_chapters is an > unsigned, so it is better anyway.) Right, ok then.
Andreas Rheinhardt: > 64 bits are needed in order to retain the uid values of Matroska > chapters; the type is kept signed because the semantics of NUT chapters > depend upon whether the id is > 0 or < 0. > > Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com> > --- > Apologies for being so late. > > doc/APIchanges | 4 ++++ > libavformat/aadec.c | 2 +- > libavformat/avformat.h | 4 ++++ > libavformat/internal.h | 4 ++++ > libavformat/matroskaenc.c | 4 ++++ > libavformat/nutdec.c | 4 ++-- > libavformat/utils.c | 4 ++++ > libavformat/version.h | 5 ++++- > 8 files changed, 27 insertions(+), 4 deletions(-) > > diff --git a/doc/APIchanges b/doc/APIchanges > index c0d955b1fa..8b93adebe1 100644 > --- a/doc/APIchanges > +++ b/doc/APIchanges > @@ -15,6 +15,10 @@ libavutil: 2017-10-21 > > API changes, most recent first: > > +2021-03-16 - xxxxxxxxxx - lavf 58.75.100 - avformat.h > + AVChapter.id will be changed from int to int64_t > + on the next major version bump. > + > 2021-03-12 - xxxxxxxxxx - lavc 58.131.100 - avcodec.h codec.h > Add a get_encode_buffer callback to AVCodecContext, similar to > get_buffer2 but for encoders. > diff --git a/libavformat/aadec.c b/libavformat/aadec.c > index e88cdb89df..80ca2c12d7 100644 > --- a/libavformat/aadec.c > +++ b/libavformat/aadec.c > @@ -222,7 +222,7 @@ static int aa_read_header(AVFormatContext *s) > c->content_end = start + largest_size; > > while ((chapter_pos = avio_tell(pb)) >= 0 && chapter_pos < c->content_end) { > - int chapter_idx = s->nb_chapters; > + unsigned chapter_idx = s->nb_chapters; > uint32_t chapter_size = avio_rb32(pb); > if (chapter_size == 0 || avio_feof(pb)) > break; > diff --git a/libavformat/avformat.h b/libavformat/avformat.h > index e3bd01ec7f..765bc3b6f5 100644 > --- a/libavformat/avformat.h > +++ b/libavformat/avformat.h > @@ -1184,7 +1184,11 @@ typedef struct AVProgram { > change dynamically at runtime. */ > > typedef struct AVChapter { > +#if FF_API_CHAPTER_ID_INT > int id; ///< unique ID to identify the chapter > +#else > + int64_t id; ///< unique ID to identify the chapter > +#endif > AVRational time_base; ///< time base in which the start/end timestamps are specified > int64_t start, end; ///< chapter start/end time in time_base units > AVDictionary *metadata; > diff --git a/libavformat/internal.h b/libavformat/internal.h > index 0ffdc87b6a..df4918e318 100644 > --- a/libavformat/internal.h > +++ b/libavformat/internal.h > @@ -554,7 +554,11 @@ void ff_configure_buffers_for_index(AVFormatContext *s, int64_t time_tolerance); > * > * @return AVChapter or NULL on error > */ > +#if FF_API_CHAPTER_ID_INT > AVChapter *avpriv_new_chapter(AVFormatContext *s, int id, AVRational time_base, > +#else > +AVChapter *avpriv_new_chapter(AVFormatContext *s, int64_t id, AVRational time_base, > +#endif > int64_t start, int64_t end, const char *title); > > /** > diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c > index 8f29d64e72..02e171593e 100644 > --- a/libavformat/matroskaenc.c > +++ b/libavformat/matroskaenc.c > @@ -1669,7 +1669,11 @@ static int mkv_write_chapters(AVFormatContext *s) > int64_t chapterstart = av_rescale_q(c->start, c->time_base, scale); > int64_t chapterend = av_rescale_q(c->end, c->time_base, scale); > const AVDictionaryEntry *t; > +#if FF_API_CHAPTER_ID_INT > uint64_t uid = create_new_ids ? i + 1ULL : (uint32_t)c->id; > +#else > + uint64_t uid = create_new_ids ? i + 1ULL : c->id; > +#endif > if (chapterstart < 0 || chapterstart > chapterend || chapterend < 0) { > av_log(s, AV_LOG_ERROR, > "Invalid chapter start (%"PRId64") or end (%"PRId64").\n", > diff --git a/libavformat/nutdec.c b/libavformat/nutdec.c > index ebb062377d..d1f3496990 100644 > --- a/libavformat/nutdec.c > +++ b/libavformat/nutdec.c > @@ -489,8 +489,8 @@ static int decode_info_header(NUTContext *nut) > AVIOContext *bc = s->pb; > uint64_t tmp, chapter_start, chapter_len; > unsigned int stream_id_plus1, count; > - int chapter_id, i, ret = 0; > - int64_t value, end; > + int i, ret = 0; > + int64_t chapter_id, value, end; > char name[256], str_value[1024], type_str[256]; > const char *type; > int *event_flags = NULL; > diff --git a/libavformat/utils.c b/libavformat/utils.c > index 8573117694..8fb5dbbf9d 100644 > --- a/libavformat/utils.c > +++ b/libavformat/utils.c > @@ -4611,7 +4611,11 @@ AVProgram *av_new_program(AVFormatContext *ac, int id) > return program; > } > > +#if FF_API_CHAPTER_ID_INT > AVChapter *avpriv_new_chapter(AVFormatContext *s, int id, AVRational time_base, > +#else > +AVChapter *avpriv_new_chapter(AVFormatContext *s, int64_t id, AVRational time_base, > +#endif > int64_t start, int64_t end, const char *title) > { > AVChapter *chapter = NULL; > diff --git a/libavformat/version.h b/libavformat/version.h > index e43754b069..ca1cd1a920 100644 > --- a/libavformat/version.h > +++ b/libavformat/version.h > @@ -32,7 +32,7 @@ > // 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 58 > -#define LIBAVFORMAT_VERSION_MINOR 74 > +#define LIBAVFORMAT_VERSION_MINOR 75 > #define LIBAVFORMAT_VERSION_MICRO 100 > > #define LIBAVFORMAT_VERSION_INT AV_VERSION_INT(LIBAVFORMAT_VERSION_MAJOR, \ > @@ -109,6 +109,9 @@ > #ifndef FF_API_DEMUXER_OPEN > #define FF_API_DEMUXER_OPEN (LIBAVFORMAT_VERSION_MAJOR < 59) > #endif > +#ifndef FF_API_CHAPTER_ID_INT > +#define FF_API_CHAPTER_ID_INT (LIBAVFORMAT_VERSION_MAJOR < 59) > +#endif > #ifndef FF_API_LAVF_PRIV_OPT > #define FF_API_LAVF_PRIV_OPT (LIBAVFORMAT_VERSION_MAJOR < 60) > #endif > I'll apply this tomorrow unless there are objections. - Andreas
diff --git a/doc/APIchanges b/doc/APIchanges index c0d955b1fa..8b93adebe1 100644 --- a/doc/APIchanges +++ b/doc/APIchanges @@ -15,6 +15,10 @@ libavutil: 2017-10-21 API changes, most recent first: +2021-03-16 - xxxxxxxxxx - lavf 58.75.100 - avformat.h + AVChapter.id will be changed from int to int64_t + on the next major version bump. + 2021-03-12 - xxxxxxxxxx - lavc 58.131.100 - avcodec.h codec.h Add a get_encode_buffer callback to AVCodecContext, similar to get_buffer2 but for encoders. diff --git a/libavformat/aadec.c b/libavformat/aadec.c index e88cdb89df..80ca2c12d7 100644 --- a/libavformat/aadec.c +++ b/libavformat/aadec.c @@ -222,7 +222,7 @@ static int aa_read_header(AVFormatContext *s) c->content_end = start + largest_size; while ((chapter_pos = avio_tell(pb)) >= 0 && chapter_pos < c->content_end) { - int chapter_idx = s->nb_chapters; + unsigned chapter_idx = s->nb_chapters; uint32_t chapter_size = avio_rb32(pb); if (chapter_size == 0 || avio_feof(pb)) break; diff --git a/libavformat/avformat.h b/libavformat/avformat.h index e3bd01ec7f..765bc3b6f5 100644 --- a/libavformat/avformat.h +++ b/libavformat/avformat.h @@ -1184,7 +1184,11 @@ typedef struct AVProgram { change dynamically at runtime. */ typedef struct AVChapter { +#if FF_API_CHAPTER_ID_INT int id; ///< unique ID to identify the chapter +#else + int64_t id; ///< unique ID to identify the chapter +#endif AVRational time_base; ///< time base in which the start/end timestamps are specified int64_t start, end; ///< chapter start/end time in time_base units AVDictionary *metadata; diff --git a/libavformat/internal.h b/libavformat/internal.h index 0ffdc87b6a..df4918e318 100644 --- a/libavformat/internal.h +++ b/libavformat/internal.h @@ -554,7 +554,11 @@ void ff_configure_buffers_for_index(AVFormatContext *s, int64_t time_tolerance); * * @return AVChapter or NULL on error */ +#if FF_API_CHAPTER_ID_INT AVChapter *avpriv_new_chapter(AVFormatContext *s, int id, AVRational time_base, +#else +AVChapter *avpriv_new_chapter(AVFormatContext *s, int64_t id, AVRational time_base, +#endif int64_t start, int64_t end, const char *title); /** diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c index 8f29d64e72..02e171593e 100644 --- a/libavformat/matroskaenc.c +++ b/libavformat/matroskaenc.c @@ -1669,7 +1669,11 @@ static int mkv_write_chapters(AVFormatContext *s) int64_t chapterstart = av_rescale_q(c->start, c->time_base, scale); int64_t chapterend = av_rescale_q(c->end, c->time_base, scale); const AVDictionaryEntry *t; +#if FF_API_CHAPTER_ID_INT uint64_t uid = create_new_ids ? i + 1ULL : (uint32_t)c->id; +#else + uint64_t uid = create_new_ids ? i + 1ULL : c->id; +#endif if (chapterstart < 0 || chapterstart > chapterend || chapterend < 0) { av_log(s, AV_LOG_ERROR, "Invalid chapter start (%"PRId64") or end (%"PRId64").\n", diff --git a/libavformat/nutdec.c b/libavformat/nutdec.c index ebb062377d..d1f3496990 100644 --- a/libavformat/nutdec.c +++ b/libavformat/nutdec.c @@ -489,8 +489,8 @@ static int decode_info_header(NUTContext *nut) AVIOContext *bc = s->pb; uint64_t tmp, chapter_start, chapter_len; unsigned int stream_id_plus1, count; - int chapter_id, i, ret = 0; - int64_t value, end; + int i, ret = 0; + int64_t chapter_id, value, end; char name[256], str_value[1024], type_str[256]; const char *type; int *event_flags = NULL; diff --git a/libavformat/utils.c b/libavformat/utils.c index 8573117694..8fb5dbbf9d 100644 --- a/libavformat/utils.c +++ b/libavformat/utils.c @@ -4611,7 +4611,11 @@ AVProgram *av_new_program(AVFormatContext *ac, int id) return program; } +#if FF_API_CHAPTER_ID_INT AVChapter *avpriv_new_chapter(AVFormatContext *s, int id, AVRational time_base, +#else +AVChapter *avpriv_new_chapter(AVFormatContext *s, int64_t id, AVRational time_base, +#endif int64_t start, int64_t end, const char *title) { AVChapter *chapter = NULL; diff --git a/libavformat/version.h b/libavformat/version.h index e43754b069..ca1cd1a920 100644 --- a/libavformat/version.h +++ b/libavformat/version.h @@ -32,7 +32,7 @@ // 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 58 -#define LIBAVFORMAT_VERSION_MINOR 74 +#define LIBAVFORMAT_VERSION_MINOR 75 #define LIBAVFORMAT_VERSION_MICRO 100 #define LIBAVFORMAT_VERSION_INT AV_VERSION_INT(LIBAVFORMAT_VERSION_MAJOR, \ @@ -109,6 +109,9 @@ #ifndef FF_API_DEMUXER_OPEN #define FF_API_DEMUXER_OPEN (LIBAVFORMAT_VERSION_MAJOR < 59) #endif +#ifndef FF_API_CHAPTER_ID_INT +#define FF_API_CHAPTER_ID_INT (LIBAVFORMAT_VERSION_MAJOR < 59) +#endif #ifndef FF_API_LAVF_PRIV_OPT #define FF_API_LAVF_PRIV_OPT (LIBAVFORMAT_VERSION_MAJOR < 60) #endif
64 bits are needed in order to retain the uid values of Matroska chapters; the type is kept signed because the semantics of NUT chapters depend upon whether the id is > 0 or < 0. Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com> --- Apologies for being so late. doc/APIchanges | 4 ++++ libavformat/aadec.c | 2 +- libavformat/avformat.h | 4 ++++ libavformat/internal.h | 4 ++++ libavformat/matroskaenc.c | 4 ++++ libavformat/nutdec.c | 4 ++-- libavformat/utils.c | 4 ++++ libavformat/version.h | 5 ++++- 8 files changed, 27 insertions(+), 4 deletions(-)