> On Sep 30, 2022, at 18:22, zhilizhao(赵志立) <quinkbl...@foxmail.com> wrote: > > > >> On Sep 30, 2022, at 17:50, wangyaqiang <1035567...@qq.com> wrote: >> >> >> >>> 2022年9月29日 16:29,zhilizhao(赵志立) <quinkbl...@foxmail.com >>> <mailto:quinkbl...@foxmail.com>> 写道: >>> >>> >>> >>>> On Sep 1, 2022, at 18:22, 1035567...@qq.com <mailto:1035567...@qq.com> >>>> wrote: >>>> >>>> From: Wang Yaqiang <wangyaqian...@kuaishou.com >>>> <mailto:wangyaqian...@kuaishou.com>> >>>> >>>> In the format of mp4 segment, the bitrate calculation of >>>> stream depends on the sample_size in moof->traf->trun box. >>>> In the original logic, when the last sidx box is read, >>>> it is not parsed backwards, and the total sample_size calculation is >>>> smaller. >>>> As a result, the bitrate displayed by ffprobe is also smaller than the >>>> actual. >>>> Increasing the moof_count variable ensures that the last moof is parsed. >>>> >>>> Test method: You can use -c copy remux a fmp4 file as mp4 >>>> and ffprobe the two files will find that the bitrate is inconsistent >>>> Befor patch: >>>> Stream #0:1[0x2](und): Audio: aac (HE-AAC) (mp4a / 0x6134706D), 44100 Hz, >>>> stereo, fltp, 59 kb/s >>>> After patch: >>>> Stream #0:1[0x2](und): Audio: aac (HE-AAC) (mp4a / 0x6134706D), 44100 Hz, >>>> stereo, fltp, 96 kb/s (default) >>>> >>>> Signed-off-by: Wang Yaqiang <wangyaqian...@kuaishou.com >>>> <mailto:wangyaqian...@kuaishou.com>> >>>> --- >>>> libavformat/isom.h | 1 + >>>> libavformat/mov.c | 8 +++++++- >>>> 2 files changed, 8 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/libavformat/isom.h b/libavformat/isom.h >>>> index f05c2d9c28..183a3c486b 100644 >>>> --- a/libavformat/isom.h >>>> +++ b/libavformat/isom.h >>>> @@ -296,6 +296,7 @@ typedef struct MOVContext { >>>> int has_looked_for_mfra; >>>> int use_tfdt; >>>> MOVFragmentIndex frag_index; >>>> + int moof_count; //ensure last fragment parse moof box >>>> int atom_depth; >>>> unsigned int aax_mode; ///< 'aax' file has been detected >>>> uint8_t file_key[20]; >>>> diff --git a/libavformat/mov.c b/libavformat/mov.c >>>> index 14550e6456..396658e342 100644 >>>> --- a/libavformat/mov.c >>>> +++ b/libavformat/mov.c >>>> @@ -7784,12 +7784,15 @@ static int mov_read_default(MOVContext *c, >>>> AVIOContext *pb, MOVAtom atom) >>>> int64_t start_pos = avio_tell(pb); >>>> int64_t left; >>>> int err = parse(c, pb, a); >>>> + if (a.type == MKTAG('m','o','o','f')) { >>>> + c->moof_count ++; >>>> + } >>>> if (err < 0) { >>>> c->atom_depth --; >>>> return err; >>>> } >>>> if (c->found_moov && c->found_mdat && a.size <= INT64_MAX - >>>> start_pos && >>>> - ((!(pb->seekable & AVIO_SEEKABLE_NORMAL) || c->fc->flags >>>> & AVFMT_FLAG_IGNIDX || c->frag_index.complete) || >>>> + ((!(pb->seekable & AVIO_SEEKABLE_NORMAL) || c->fc->flags >>>> & AVFMT_FLAG_IGNIDX || (c->frag_index.complete && c->moof_count >= >>>> c->frag_index.nb_items)) || >>>> start_pos + a.size == avio_size(pb))) { >>>> if (!(pb->seekable & AVIO_SEEKABLE_NORMAL) || c->fc->flags & >>>> AVFMT_FLAG_IGNIDX || c->frag_index.complete) >>>> c->next_root_atom = start_pos + a.size; >>> >>> No, it breaks the use case with global sidx. We can achieve fast >>> startup with global sidx. >> >> This patch indeed have an effect on startup, will add some option. >> >>> >>> The patch describes an issue with multiple sidx cases, actually >>> it’s more serious with global sidx. In the first case, the bitrate >>> is a little inaccurate. Bitrate is near zero for the second case. >>> >>> I have two ideas: >>> >>> 1. Just skip bitrate calculation from sc->data_size if sidx exist, >>> e.g., >>> >>> @@ -8500,7 +8500,7 @@ static int mov_read_header(AVFormatContext *s) >>> for (i = 0; i < s->nb_streams; i++) { >>> AVStream *st = s->streams[i]; >>> MOVStreamContext *sc = st->priv_data; >>> - if (st->duration > 0) { >>> + if (st->duration > 0 && !sc->has_sidx) { >>> /* Akin to sc->data_size * 8 * sc->time_scale / st->duration >>> but accounting for overflows. */ >>> st->codecpar->bit_rate = av_rescale(sc->data_size, ((int64_t) >>> sc->time_scale) * 8, st->duration); >>> if (st->codecpar->bit_rate == INT64_MIN) { >>> >>> It’s simple, and bitrate information has multiple sources. >>> >> >> In this way, will be no bitrate info in the stream, will affect the users? > > It depends. We have multiple sources to get bitrate. > >> >>> 2. Add a option to prevent mark complete when reading sidx >>> >>> @@ -5446,7 +5446,7 @@ static int mov_read_sidx(MOVContext *c, AVIOContext >>> *pb, MOVAtom atom) >>> // See if the remaining bytes are just an mfra which we can ignore. >>> is_complete = offset == stream_size; >>> - if (!is_complete && (pb->seekable & AVIO_SEEKABLE_NORMAL) && >>> stream_size > 0 ) { >>> + if (!is_complete && (pb->seekable & AVIO_SEEKABLE_NORMAL) && >>> stream_size > 0 && !parse_full) { >>> >>> Then >> >> Mfra box is not required in fmp4. for example, adding skip_trailer parameter >> when generate fm4, bitrate will still be incorrect. > > You missed the point. If mfra doesn’t exist in the file, > the issue doesn’t exist neither.
Sorry you are right. offset is not current file position but accumulated value from sidx boxes. c->frag_index.complete should be marked conditionally, if we can’t find a better solution. > >> >>> >>> @@ -8500,7 +8500,7 @@ static int mov_read_header(AVFormatContext *s) >>> for (i = 0; i < s->nb_streams; i++) { >>> AVStream *st = s->streams[i]; >>> MOVStreamContext *sc = st->priv_data; >>> - if (st->duration > 0) { >>> + if (st->duration > 0 && (!sc->has_sidx || parse_full)) { >>> >>> It’s such a corner case and I can’t find a suitable name for this option. >>> >>> Any ideas? >>> >>>> @@ -8361,6 +8364,9 @@ static int mov_read_header(AVFormatContext *s) >>>> av_log(s, AV_LOG_ERROR, "moov atom not found\n"); >>>> return AVERROR_INVALIDDATA; >>>> } >>>> + if (mov->frag_index.nb_items > mov->moof_count) { >>>> + av_log(s, AV_LOG_WARNING, "the number of moof is less then >>>> fragment count\n"); >>>> + } >>>> av_log(mov->fc, AV_LOG_TRACE, "on_parse_exit_offset=%"PRId64"\n", >>>> avio_tell(pb)); >>>> >>>> if (pb->seekable & AVIO_SEEKABLE_NORMAL) { >>>> -- >>>> 2.33.0 >>>> >>>> _______________________________________________ >>>> ffmpeg-devel mailing list >>>> ffmpeg-devel@ffmpeg.org <mailto:ffmpeg-devel@ffmpeg.org> >>>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel >>>> <https://ffmpeg.org/mailman/listinfo/ffmpeg-devel> >>>> >>>> To unsubscribe, visit link above, or email >>>> ffmpeg-devel-requ...@ffmpeg.org <mailto:ffmpeg-devel-requ...@ffmpeg.org> >>>> with subject "unsubscribe". >>> >>> _______________________________________________ >>> ffmpeg-devel mailing list >>> ffmpeg-devel@ffmpeg.org <mailto:ffmpeg-devel@ffmpeg.org> >>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel >>> <https://ffmpeg.org/mailman/listinfo/ffmpeg-devel> >>> >>> To unsubscribe, visit link above, or email >>> ffmpeg-devel-requ...@ffmpeg.org <mailto: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". > > _______________________________________________ > 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".