From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <ffmpeg-devel-bounces@ffmpeg.org>
Received: from ffbox0-bg.ffmpeg.org (ffbox0-bg.ffmpeg.org [79.124.17.100])
by master.gitmailbox.com (Postfix) with ESMTPS id F05184D374
for <ffmpegdev@gitmailbox.com>; Sat, 31 May 2025 18:16:57 +0000 (UTC)
Received: from [127.0.1.1] (localhost [127.0.0.1])
by ffbox0-bg.ffmpeg.org (Postfix) with ESMTP id 147ED68DE40;
Sat, 31 May 2025 21:16:55 +0300 (EEST)
Received: from mail-ed1-f43.google.com (mail-ed1-f43.google.com
[209.85.208.43])
by ffbox0-bg.ffmpeg.org (Postfix) with ESMTPS id D5AC768DDCA
for <ffmpeg-devel@ffmpeg.org>; Sat, 31 May 2025 21:16:48 +0300 (EEST)
Received: by mail-ed1-f43.google.com with SMTP id
4fb4d7f45d1cf-6045b95d1feso5742839a12.1
for <ffmpeg-devel@ffmpeg.org>; Sat, 31 May 2025 11:16:48 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
d=unified-streaming.com; s=google; t=1748715408; x=1749320208; 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=Ms8SViijB3HDC85z+DZSpKnPU3J5dmInkdhDBjYiaLQ=;
b=IXFJmQ+zjNFVOYfzrPz3WYIjOs4QIKwTTA5+d3/TXHg8GbzCcfPQwuBkuXY3OeYyg2
edeVLmRXVJmUSKPHYMKIU0HJGlYGM/4UGeo+yHYpEt28UvunIp2Nn2VdWjW5G1kmg61F
90tbqHiWiI/7jmh7xlGeJT9uuHDXpSt0/+nZrwocYcNMW30RytgSm4wU3bDf3/AhpkR/
+vygzo9llhRO1q9yw+GGj425Xs2GkPdDZXrQKenEMJlr33jZtrqChmRBy8dqFlqH4vz8
SHhhc6OnW3gb9t8EEjXUXEyLbkfpOCxiu6JyFRHMQWuxB4dAdQSH04sAtMD9FeHzNWtY
CwBA==
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
d=1e100.net; s=20230601; t=1748715408; x=1749320208;
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=Ms8SViijB3HDC85z+DZSpKnPU3J5dmInkdhDBjYiaLQ=;
b=Dv3lYJK7u0q9SlgdJPyALsY0g1FaJZTnHA6xpag0Trr9DCxyG8m34xrg7WUS2fXcmZ
ZMnsofE4FFVOccFwYA0MzHr70bDn/RQ33bF5CX+MLqfYIO7+dD8mrItdH69VBvmkc753
FzPouQFS/b+/l+ZRuj3nB13lHPIGzqjZXhX+XU5NuPeUTcj6MZKkpNGx6UjsVr/63gyd
hA3Md0K/YeA6LAZcwDuDrkF8n3ImUWf7GoNY9gfpu4g+fASnlEuVuvSY322JeZsc90CM
4A71D6sFvnQ/CzVWuqkojCe2tcIgr0ct4yWVnbj46jnCEN/TSmdQsv1e5gO3FdfWZMcG
dqGw==
X-Gm-Message-State: AOJu0Yw9PlVdTt0vzgciIhYy5teJRPKJ/+zBvQdJonsGiRorxYPDDUWQ
9HfjIo4muoucGVh7UKpQbiipSdZ5A0oy6wD4gUzX0a9hSROtIFV/2p1C+4TW/9sBwMcY4vjnqly
T63kaU/UdJ70oVvhznMag6J+UITZ36xulN+N9Ns5FIm96JqqDo/MY2kETNUkLrZ81KbBYWyk4sL
srbSTK54J8+B5IKByuTocyWCABbuvrS/630euwPbPU4V7m8jMZIc4E2b7vUQ==
X-Gm-Gg: ASbGncu+LANX4+hLbBW3GxT400T+isTKCwuWhYrFXRqQV1H+QJd8IEk4g9RIosxNTiz
zsXC3E1EmElodKR6MCRnKT3apK08amSZjqsNCG8zxftvDNPjDgpkhi2E2ulPTcolFifATHfxEb7
74VIf0kApPOhGIb2cEdBq2s5mzFzmnl4xq3erZV0ALnA8GZC8Zf7GG3JQBaSh4gPW7jwjzeXRBf
Ot/aSo4O0yqhDdyPJVsB7hr4sJ5kuvYxuwAxlKR/Y43dKuzqg5HSzhnfKuCX3uSCN+o0cyJrzCe
HrC1Q62CYnAzXhuEyz1lnLoABRd9PKAe3qKw8Z1cOT4H3IsPtjzAljZiijHjfNiLKwWRx/7TDAG
IgysdM5aOfFA3hTs5Las=
X-Google-Smtp-Source: AGHT+IG/I16q6UH5Wni1YbojUAYYgEjEP5tuAtdCBTIgfLqNUj+GlrdO1YRHWUf+0iqvMOD4El5QaQ==
X-Received: by 2002:a05:6402:280f:b0:602:3e6d:9334 with SMTP id
4fb4d7f45d1cf-6057c628879mr5180751a12.24.1748715407826;
Sat, 31 May 2025 11:16:47 -0700 (PDT)
Received: from smtpclient.apple (tensor.andric.com. [87.251.56.140])
by smtp.gmail.com with ESMTPSA id
4fb4d7f45d1cf-60566c7442csm3630294a12.40.2025.05.31.11.16.47
for <ffmpeg-devel@ffmpeg.org>
(version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128);
Sat, 31 May 2025 11:16:47 -0700 (PDT)
From: Dimitry Andric <dimitry@unified-streaming.com>
Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3731.700.6.1.11\))
Date: Sat, 31 May 2025 20:16:46 +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>
<c6797759-5df0-42f1-bde7-b47f3ba39fd4@gmail.com>
To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
In-Reply-To: <c6797759-5df0-42f1-bde7-b47f3ba39fd4@gmail.com>
Message-Id: <E4C5AB80-C449-4313-879B-89EF2EDA28E8@unified-streaming.com>
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 <ffmpeg-devel.ffmpeg.org>
List-Unsubscribe: <https://ffmpeg.org/mailman/options/ffmpeg-devel>,
<mailto:ffmpeg-devel-request@ffmpeg.org?subject=unsubscribe>
List-Archive: <https://ffmpeg.org/pipermail/ffmpeg-devel>
List-Post: <mailto:ffmpeg-devel@ffmpeg.org>
List-Help: <mailto:ffmpeg-devel-request@ffmpeg.org?subject=help>
List-Subscribe: <https://ffmpeg.org/mailman/listinfo/ffmpeg-devel>,
<mailto:ffmpeg-devel-request@ffmpeg.org?subject=subscribe>
Reply-To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Errors-To: ffmpeg-devel-bounces@ffmpeg.org
Sender: "ffmpeg-devel" <ffmpeg-devel-bounces@ffmpeg.org>
Archived-At: <https://master.gitmailbox.com/ffmpegdev/E4C5AB80-C449-4313-879B-89EF2EDA28E8@unified-streaming.com/>
List-Archive: <https://master.gitmailbox.com/ffmpegdev/>
List-Post: <mailto:ffmpegdev@gitmailbox.com>
On 9 May 2025, at 00:15, James Almer <jamrial@gmail.com> wrote:
>
> On 5/8/2025 7:14 PM, Dimitry Andric wrote:
>> On 28 Apr 2025, at 13:00, Dimitry Andric <dimitry@unified-streaming.com> wrote:
>>>
>>> On 19 Apr 2025, at 16:27, Dimitry Andric <dimitry@unified-streaming.com> wrote:
>>>>
>>>> On 10 Apr 2025, at 11:03, Dimitry Andric <dimitry@unified-streaming.com> wrote:
>>>>>
>>>>> On 3 Apr 2025, at 22:02, Dimitry Andric <dimitry@unified-streaming.com> 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 <dimitry@unified-streaming.com>
>>>>>> ---
>>>>>> 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.
-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".