From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from ffbox0-bg.ffmpeg.org (ffbox0-bg.ffmpeg.org [79.124.17.100]) by master.gitmailbox.com (Postfix) with ESMTPS id 1E2944D867 for ; Wed, 2 Jul 2025 12:05:19 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.ffmpeg.org (Postfix) with ESMTP id 6085A68D19A; Wed, 2 Jul 2025 15:05:16 +0300 (EEST) Received: from mail-ej1-f54.google.com (mail-ej1-f54.google.com [209.85.218.54]) by ffbox0-bg.ffmpeg.org (Postfix) with ESMTPS id 5D27268C8BC for ; Wed, 2 Jul 2025 15:05:10 +0300 (EEST) Received: by mail-ej1-f54.google.com with SMTP id a640c23a62f3a-ae36e88a5daso864022866b.1 for ; Wed, 02 Jul 2025 05:05:10 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=unified-streaming.com; s=google; t=1751457909; x=1752062709; darn=ffmpeg.org; h=message-id:in-reply-to:to:references:date:subject:mime-version :content-transfer-encoding:from:from:to:cc:subject:date:message-id :reply-to; bh=RGhnYxXXmAh7P0hdZcn5Ld5h2/b1Kj6lyv/7KK+/7Ns=; b=6CmOp5ZzvvkxzZTugdjd42hOtKzpXWeHZ/RBMTyW5reTrFwGN+9XUZWxh7Vp5/8+nC lzQb+k7Vl00i12mBIxLEZphX2rcssPOLpGCWUPNYaVNqSs4CahIrTYRDaS/Q1FMzyNj3 SpSJHWQQwFIpcii2nGJhfp/Js9rZ9xmAIEpUDtTAz/0LAx+6i7fv5rTSytHL5fiB0F8G bVYcBVSHHDmtFgAxlFbZguCTD/kvgsIY1oWriX7aJebqwuP68Jfb8Addhwr+yElgPXKc e7+Ppz7JFvL8wcajDsbBD0mPoDTvPatDVzNxpfOtCCgYCcZD0C+jyriGHwytt1K6aJZj 8D7w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1751457909; x=1752062709; h=message-id:in-reply-to:to:references:date:subject:mime-version :content-transfer-encoding:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=RGhnYxXXmAh7P0hdZcn5Ld5h2/b1Kj6lyv/7KK+/7Ns=; b=Fc0ZN5tptWAQttyrZehThZ+xLvJLkqhMHUlOZ5SHSD4vCGqf/Pyl6oMoC8ffLMDwJt +DAIRqjSMovFhot5tpG806NYhYIi1dyzWs0VlPzEu1Dr+Gr77QheSwHF8jHuXlbrOkX/ nRmHbgVXeRUIfY/m4JoLN6HRuRMUFOIByxq7IEoQst/oiIMUftPjGN8oIBsgJK6YrHSe sjLhUpe0nTMoZMlGSYbkm57yv8MxWktsguxYLKWNqaCnH/4bPTrgz1/ef1gwk0k9iHRT 1Ctbe+YxyhucdkHRHeHOIweSCbeeQbKGK+wenRKCA5zV02nqYm+WPVJAEogTg9hOUWL5 qC+g== X-Gm-Message-State: AOJu0YwbTzuDul+MtO/jr55uF5cm9e6xCimR7uVLEBnmPC5V/Xc8mXCy dJ9nWptU3IKu1bxbnux4B+JkQgwYo+YZQ87Ay3jlhlSiRigHPAk1y+RdqnsJQ4+JrD/omee6Uln w3Hee17XDW1DoHGG2oLfjIDg/8xp5xKxW1Wq0BfB714yXz9nUg/cS28Eyjc0ckFSKe9JfwrYaDR f+xVH+zXIetj3kZYj7svoGdjSduqgwr+0yo6t9oALvPs4Ux/pryV4PUS0= X-Gm-Gg: ASbGncuP1Mve1qf+i7icnb0h3Q0u8lsvzDdesZCkP/EJ8lWqKUYUMVF6MYNm0V1iOXy 9mTQYxdxF2paWEegZ8vhDEljSZdAG08nkDsn102Z7VftBGpFKpoGiseXWicWQYZ/o8tntWauRO8 +J/2jWc6nVl/9CFiW9BnmGxohnAbET1sQYurQvSJPjJecVVM8Z0nvzlrEE9FJZ7e8jpG0/1103v ZFUZflieHq9c3YS5Ij1EWF0C1P4e5nkx/UdN3UMpvJuTYAVOJXm4wTxj5NHJj9zSgQ9mCqbBLYT rjGHtSESc2KGE6YNFs4hHKKjGBeV/avwKPufYjkHXZEckt3nhIs4e23FCfEuHU0Rj6d1JmqvfS5 MrGCX39kQZE5pVLImmBUOnw1JicIj4A== X-Google-Smtp-Source: AGHT+IEILQSEEAUAn+wJoHgyijagKo9kU+TTcgFt5xYG3KbPdSE0yqqx9bfY5nWz4peu44VAF7Ep+g== X-Received: by 2002:a17:907:da5:b0:ae3:ab68:4d7a with SMTP id a640c23a62f3a-ae3c2c9d426mr291120966b.25.1751457907513; Wed, 02 Jul 2025 05:05:07 -0700 (PDT) Received: from smtpclient.apple (tensor.andric.com. [87.251.56.140]) by smtp.gmail.com with ESMTPSA id a640c23a62f3a-ae353c01636sm1070577366b.105.2025.07.02.05.05.06 for (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Wed, 02 Jul 2025 05:05:06 -0700 (PDT) From: Dimitry Andric Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3731.700.6.1.11\)) Date: Wed, 2 Jul 2025 14:05:05 +0200 References: <20250403200239.226898-1-dimitry@unified-streaming.com> <9ADA5613-0335-455A-B75C-8ADD42D860E6@unified-streaming.com> <039BFB64-4F2B-4644-BA12-AE7ECE262393@unified-streaming.com> <4ECBE413-6C97-473E-A61F-75DCD6281196@unified-streaming.com> <3198FB5F-EF3B-41B1-BB33-E3AC82A4161A@unified-streaming.com> To: FFmpeg development discussions and patches In-Reply-To: Message-Id: X-Mailer: Apple Mail (2.3731.700.6.1.11) Subject: Re: [FFmpeg-devel] [PATCH] avformat/mov: Fix decoding fragmented MP4 with multiple sample entries and empty stsc X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: FFmpeg development discussions and patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: FFmpeg development discussions and patches Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: On 31 May 2025, at 20:16, Dimitry Andric wrote: > > On 9 May 2025, at 00:15, James Almer wrote: >> >> On 5/8/2025 7:14 PM, Dimitry Andric wrote: >>> On 28 Apr 2025, at 13:00, Dimitry Andric wrote: >>>> >>>> On 19 Apr 2025, at 16:27, Dimitry Andric wrote: >>>>> >>>>> On 10 Apr 2025, at 11:03, Dimitry Andric wrote: >>>>>> >>>>>> On 3 Apr 2025, at 22:02, Dimitry Andric wrote: >>>>>>> >>>>>>> When decoding fragmented MP4 files that have an empty stsc box, and >>>>>>> instead contain sample description indexes in their tfhd boxes, the mov >>>>>>> demuxer does not notify the decoder whenever the current sample >>>>>>> description index changes. If the SPS or PPS changed sufficiently, this >>>>>>> can lead to unexpected decoding errors. >>>>>>> >>>>>>> To fix this, in mov_finalize_packet(), when stsc_data is not available, >>>>>>> use get_frag_stream_info_from_pkt() to get at the current fragment >>>>>>> stream info, and retrieve the current sample description index from >>>>>>> there. Then use that index in a similar manner as the stsc case. >>>>>>> >>>>>>> Signed-off-by: Dimitry Andric >>>>>>> --- >>>>>>> libavformat/mov.c | 50 ++++++++++++++++++++++++++++------------------- >>>>>>> 1 file changed, 30 insertions(+), 20 deletions(-) >>>>>>> >>>>>>> diff --git a/libavformat/mov.c b/libavformat/mov.c >>>>>>> index 452690090c..ead89192f4 100644 >>>>>>> --- a/libavformat/mov.c >>>>>>> +++ b/libavformat/mov.c >>>>>>> @@ -10756,25 +10756,29 @@ static int mov_switch_root(AVFormatContext *s, int64_t target, int index) >>>>>>> return 1; >>>>>>> } >>>>>>> >>>>>>> -static int mov_change_extradata(AVStream *st, AVPacket *pkt) >>>>>>> +static int mov_change_extradata(AVStream *st, AVPacket *pkt, int stsd_id) >>>>>>> { >>>>>>> MOVStreamContext *sc = st->priv_data; >>>>>>> uint8_t *side, *extradata; >>>>>>> int extradata_size; >>>>>>> >>>>>>> - /* Save the current index. */ >>>>>>> - sc->last_stsd_index = sc->stsc_data[sc->stsc_index].id - 1; >>>>>>> + if (stsd_id > 0 && >>>>>>> + stsd_id - 1 < sc->stsd_count && >>>>>>> + stsd_id - 1 != sc->last_stsd_index) { >>>>>>> + /* Save the current index. */ >>>>>>> + sc->last_stsd_index = stsd_id - 1; >>>>>>> >>>>>>> - /* Notify the decoder that extradata changed. */ >>>>>>> - extradata_size = sc->extradata_size[sc->last_stsd_index]; >>>>>>> - extradata = sc->extradata[sc->last_stsd_index]; >>>>>>> - if (st->discard != AVDISCARD_ALL && extradata_size > 0 && extradata) { >>>>>>> - side = av_packet_new_side_data(pkt, >>>>>>> - AV_PKT_DATA_NEW_EXTRADATA, >>>>>>> - extradata_size); >>>>>>> - if (!side) >>>>>>> - return AVERROR(ENOMEM); >>>>>>> - memcpy(side, extradata, extradata_size); >>>>>>> + /* Notify the decoder that extradata changed. */ >>>>>>> + extradata_size = sc->extradata_size[sc->last_stsd_index]; >>>>>>> + extradata = sc->extradata[sc->last_stsd_index]; >>>>>>> + if (st->discard != AVDISCARD_ALL && extradata_size > 0 && extradata) { >>>>>>> + side = av_packet_new_side_data(pkt, >>>>>>> + AV_PKT_DATA_NEW_EXTRADATA, >>>>>>> + extradata_size); >>>>>>> + if (!side) >>>>>>> + return AVERROR(ENOMEM); >>>>>>> + memcpy(side, extradata, extradata_size); >>>>>>> + } >>>>>>> } >>>>>>> >>>>>>> return 0; >>>>>>> @@ -10893,13 +10897,10 @@ static int mov_finalize_packet(AVFormatContext *s, AVStream *st, AVIndexEntry *s >>>>>>> >>>>>>> /* Multiple stsd handling. */ >>>>>>> if (sc->stsc_data) { >>>>>>> - if (sc->stsc_data[sc->stsc_index].id > 0 && >>>>>>> - sc->stsc_data[sc->stsc_index].id - 1 < sc->stsd_count && >>>>>>> - sc->stsc_data[sc->stsc_index].id - 1 != sc->last_stsd_index) { >>>>>>> - int ret = mov_change_extradata(st, pkt); >>>>>>> - if (ret < 0) >>>>>>> - return ret; >>>>>>> - } >>>>>>> + int stsd_id = sc->stsc_data[sc->stsc_index].id; >>>>>>> + int ret = mov_change_extradata(st, pkt, stsd_id); >>>>>>> + if (ret < 0) >>>>>>> + return ret; >>>>>>> >>>>>>> /* Update the stsc index for the next sample */ >>>>>>> sc->stsc_sample++; >>>>>>> @@ -10908,6 +10909,15 @@ static int mov_finalize_packet(AVFormatContext *s, AVStream *st, AVIndexEntry *s >>>>>>> sc->stsc_index++; >>>>>>> sc->stsc_sample = 0; >>>>>>> } >>>>>>> + } else { >>>>>>> + MOVContext *mov = s->priv_data; >>>>>>> + MOVFragmentStreamInfo *frag_stream_info = get_frag_stream_info_from_pkt(&mov->frag_index, pkt, sc->id); >>>>>>> + if (frag_stream_info) { >>>>>>> + int stsd_id = frag_stream_info->stsd_id; >>>>>>> + int ret = mov_change_extradata(st, pkt, stsd_id); >>>>>>> + if (ret < 0) >>>>>>> + return ret; >>>>>>> + } >>>>>>> } >>>>>>> >>>>>>> return 0; >>>>>>> -- >>>>>>> 2.43.0 >>>>>>> >>>>>> >>>>>> Any comments on this patch? >>>>> >>>>> Ping :) >>>> >>>> Is there any particular group of persons that "own" the mov muxer? >>> Another ping. >> >> I'll have a look seeing no one else will. > > To provide some backstory here, I will attempt to explain further what > this patch is supposed to fix. It is specifically about AVC (or possibly > HEVC) video that has more than one referenced PPS in the elementary > stream. (One encoder that sometimes produces this kind of video is x264, > unless you use the --stitchable option). > > In a MP4 file this can be represented by multiple sample description > entries in the 'stsd' box, and in a progressive file there is a 'stsc' > box which defines which samples have which sample description indexes. > FFmpeg handles these just fine. > > However, in a fragmented MP4 file the 'stsc' box is usually empty, and > the fragments have a 'tfhd' box with a sample description index field > instead. Such files can sometimes not be decoded properly by FFmpeg, > since it does not call mov_change_extradata() whenever the sample > description index changes, somewhere in the middle of the video. In that > case, it will either complain about a bad PPS ID, or if the ID matches > but the PPS contents does not, lots of decoding errors will occur. > > This proposed patch makes it so mov_change_extradata() is called even if > MovStreamContext's sc_data field is empty, but > get_frag_stream_info_from_pkt() returns a valid stsd_id in its > MOVFragmentStreamInfo. For fragmented files, mov_read_tfhd() already > takes care of reading the stsd_id from the tfhd boxes. Another ping. -Dimitry _______________________________________________ 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".