On Wed, 30 Sep 2020, Andreas Rheinhardt wrote:

There are two possible kinds of timecode tracks (with tag "tmcd") in the
mov muxer: Tracks created internally by the muxer and timecode tracks
sent by the user. If any of the latter exists, the former are
deactivated. The former all belong to another track, the source
track; the latter don't have a source track set, but the index of the
source track is initially zeroed by av_mallocz_array().

Would it be worthwhile to explicitly initialize these to e.g. -1, to make src_track clearer?

This is a problem since 3d894db700cc1e360a7a75ab9ac8bf67ac6670a3: Said commit added a function that calculates the duration of tracks and the duration of timecode tracks is calculated by rescaling the duration (calculated by the very same function) of the source track. This gives an infinite recursion if the first track (the one that will be treated as source track for all timecode tracks) is a timecode track itself, leading to a stack overflow.

This commit fixes this by not using the nonexistent source track
when calculating the duration of timecode tracks not created internally
by the mov muxer.

Signed-off-by: Andreas Rheinhardt <andreas.rheinha...@gmail.com>
---
1. Remuxing timecode tracks is currently impossible for mp4 (the codec
tag validation fails); I wonder whether this is actually intended given
that there is a warning that timecode metadata is ignored if an explicit
timecode track is present.
2. There is another place where the src_track field is used even when a
timecode track doesn't have a src_track: When writing the moov tag
(lines 4156-4166). This will probably also need to be fixed, but it is
not dangerous.
3. There is btw no validation that a track with "tmcd" tag is a data
stream.

libavformat/movenc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavformat/movenc.c b/libavformat/movenc.c
index 2006fcee4b..265465f97b 100644
--- a/libavformat/movenc.c
+++ b/libavformat/movenc.c
@@ -2901,7 +2901,7 @@ static int mov_write_minf_tag(AVFormatContext *s, 
AVIOContext *pb, MOVMuxContext

static int64_t calc_pts_duration(MOVMuxContext *mov, MOVTrack *track)
{
-    if (track->tag == MKTAG('t','m','c','d')) {
+    if (track->tag == MKTAG('t','m','c','d') && mov->nb_meta_tmcd) {
        // tmcd tracks gets track_duration set in mov_write_moov_tag from
        // another track's duration, while the end_pts may be left at zero.

This fix in itself is probably good and safe, so LGTM.

// Martin

_______________________________________________
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".

Reply via email to