Michael Niedermayer:
> On Tue, May 05, 2020 at 04:16:56PM +0200, Andreas Rheinhardt wrote:
>> When writing the header, the NUT muxer allocates an array with as many
>> entries as there are chapters containing information about the used
>> timebase. This information is used when writing the headers and also
>> when resending the headers (as the NUT muxer does from time to time).
>>
>> When the NUT muxer writes or resends the headers, it simply presumes
>> that there are enough entries in its array for each chapter in the
>> AVFormatContext. Yet users are allowed to add chapters during the muxing
>> process, so this presumption is wrong and may lead to segfaults.
>>
>> So explicitly store the number of entries of the chapter array and refer
>> to this number whenever headers are written.
>>
>> Signed-off-by: Andreas Rheinhardt <andreas.rheinha...@gmail.com>
>> ---
>> This patch presumes that the user may not change or remove the chapters
>> available during writing the header (if there were chapters available
>> when writing the header at all). I hope this is ok.
>>
>> libavformat/nut.h | 1 +
>> libavformat/nutenc.c | 3 ++-
>> 2 files changed, 3 insertions(+), 1 deletion(-)
>
> how do i apply this (for testing) ?
When you test this, you should be aware that the first time the header
is repeated is after 16 TiB of output (see patch #12 for the reason), so
you should better change the following line in nut_write_packet() to
reduce said interval:
if (1LL << (20 + 3 * nut->header_count) <= avio_tell(bc))
> on its own it fails and it seems the previous patchset doesnt like applying
> anymore either
>
I have already applied patches 1-4 + 9 (that you said were ok) from this
patchset. Patches 5-8 and 10-11 apply cleanly on top of master. Did you
try to also apply the patches that have already been merged?
(Patch #12 does not apply cleanly any more, because of the changes to
the ref files in b4967fc71c63eae8cd96f9c46cd3e1fbd705bbf9. Furthermore
my patch as sent only updated a subset of fate-tests, because my build
was not configured with enable-gpl. I already fixed this. Normally I'd
resend the mail, but given its huge size I'd like to avoid that this
time. Should I send it?)
>
>>
>> diff --git a/libavformat/nut.h b/libavformat/nut.h
>> index a4409ee23d..52225fed93 100644
>> --- a/libavformat/nut.h
>> +++ b/libavformat/nut.h
>> @@ -115,6 +115,7 @@ typedef struct NUTContext {
>> int flags;
>> int version; // version currently in use
>> int minor_version;
>> + unsigned nb_chapters;
>> } NUTContext;
>>
>> extern const AVCodecTag ff_nut_subtitle_tags[];
>> diff --git a/libavformat/nutenc.c b/libavformat/nutenc.c
>> index 5071278835..2d35c44b79 100644
>> --- a/libavformat/nutenc.c
>> +++ b/libavformat/nutenc.c
>> @@ -675,7 +675,7 @@ static int write_headers(AVFormatContext *avctx,
>> AVIOContext *bc)
>> goto fail;
>> }
>>
>> - for (i = 0; i < nut->avf->nb_chapters; i++) {
>> + for (i = 0; i < nut->nb_chapters; i++) {
>> write_chapter(nut, dyn_bc, i, prelude, &prelude_size);
>
> also if i read this correctly, this would not write all chapters.
> That seems not ideal
It's indeed not ideal; it is designed to fix a potential segfault, not
to add functionality. (Without this patch patch #12 would run into this
bug more often.)
I don't know NUT very well, but I see two ways how this could be
implemented:
1. One adds the timebases of these chapters to the arrays of timebases
that already exist and writes the chapters with their timebases. This
would involve moving code from nut_write_header() to write_headers(). I
don't know whether adding a new timebase mid-stream is even allowed by
the spec.
2. One writes the chapters with the best timebase available for them,
even when it is not an exact match.
- Andreas
_______________________________________________
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".
