> On April 5, 2020 5:59 PM Andreas Rheinhardt <andreas.rheinha...@gmail.com>
> wrote:
>
>
> AVChapters have an int as id field and therefore this value can appear
> <= 0. When remuxing from Matroska, this value actually contains
> the lower 32 bits of the original ChapterUID (which can be 64 bits).
>
> In order to ensure that the ChapterUID is always > 0, they were offset
> as follows (since 07704c61): First max(0, 1LL - chapter[i].id) was computed
> and stored in an uint32_t. And then the IDs were offset using this value.
>
> This has two downsides:
> 1. It does not ensure that the UID is actually != 0: Namely if there is
> a chapter with id == INT_MIN, then the offset will be 2^31 + 1 and a
> chapter with id == INT_MAX will become 2^31 - 1 + 2^31 + 1 = 2^32 = 0,
> because the actual calculation was performed in 32 bits.
> 2. As soon as a chapter id appears to be negative, a nontrivial offset
> is used, so that not even a ChapterUID that only uses 32 bits is
> preserved.
>
> So change this by treating the id as an unsigned value internally and
> only offset (by 1) if an id vanishes. The actual offsetting then has to
> be performed in 64 bits in order to make sure that no UINT32_MAX wraps
> around.
That means you are changing the chapter UIDs of the source when remuxing (if
for some reason a chapter with no id was added in the process). If tags were
referencing the chapter UIDs (TagChapterUID) they don't match the chapter
anymore.
I think silently changing the IDs is wrong. But its was already like that
before, so that's not breaking your patch. If anything your patch is less
likely to break remuxing.
> Signed-off-by: Andreas Rheinhardt <andreas.rheinha...@gmail.com>
> ---
> libavformat/matroskaenc.c | 15 ++++++++++-----
> 1 file changed, 10 insertions(+), 5 deletions(-)
>
> diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
> index 060e8b7816..a377d092df 100644
> --- a/libavformat/matroskaenc.c
> +++ b/libavformat/matroskaenc.c
> @@ -1422,7 +1422,8 @@ static int mkv_write_chapters(AVFormatContext *s)
> }
>
> chapteratom = start_ebml_master(dyn_cp, MATROSKA_ID_CHAPTERATOM, 0);
> - put_ebml_uint(dyn_cp, MATROSKA_ID_CHAPTERUID, c->id +
> mkv->chapter_id_offset);
> + put_ebml_uint(dyn_cp, MATROSKA_ID_CHAPTERUID,
> + (uint32_t)c->id + (uint64_t)mkv->chapter_id_offset);
> put_ebml_uint(dyn_cp, MATROSKA_ID_CHAPTERTIMESTART, chapterstart);
> put_ebml_uint(dyn_cp, MATROSKA_ID_CHAPTERTIMEEND, chapterend);
> if (mkv->mode != MODE_WEBM) {
> @@ -1479,7 +1480,7 @@ static int mkv_write_simpletag(AVIOContext *pb,
> AVDictionaryEntry *t)
> }
>
> static int mkv_write_tag_targets(AVFormatContext *s, uint32_t elementid,
> - unsigned int uid, ebml_master *tag)
> + uint64_t uid, ebml_master *tag)
> {
> AVIOContext *pb;
> MatroskaMuxContext *mkv = s->priv_data;
> @@ -1518,7 +1519,7 @@ static int mkv_check_tag_name(const char *name,
> uint32_t elementid)
> }
>
> static int mkv_write_tag(AVFormatContext *s, AVDictionary *m, uint32_t
> elementid,
> - unsigned int uid)
> + uint64_t uid)
> {
> MatroskaMuxContext *mkv = s->priv_data;
> ebml_master tag;
> @@ -1612,7 +1613,8 @@ static int mkv_write_tags(AVFormatContext *s)
> if (!mkv_check_tag(ch->metadata,
> MATROSKA_ID_TAGTARGETS_CHAPTERUID))
> continue;
>
> - ret = mkv_write_tag(s, ch->metadata,
> MATROSKA_ID_TAGTARGETS_CHAPTERUID, ch->id + mkv->chapter_id_offset);
> + ret = mkv_write_tag(s, ch->metadata,
> MATROSKA_ID_TAGTARGETS_CHAPTERUID,
> + (uint32_t)ch->id +
> (uint64_t)mkv->chapter_id_offset);
> if (ret < 0)
> return ret;
> }
> @@ -1882,7 +1884,10 @@ static int mkv_write_header(AVFormatContext *s)
> return ret;
>
> for (i = 0; i < s->nb_chapters; i++)
> - mkv->chapter_id_offset = FFMAX(mkv->chapter_id_offset, 1LL -
> s->chapters[i]->id);
> + if (!s->chapters[i]->id) {
> + mkv->chapter_id_offset = 1;
> + break;
> + }
>
> ret = mkv_write_chapters(s);
> if (ret < 0)
> --
> 2.20.1
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
