From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org [79.124.17.100]) by master.gitmailbox.com (Postfix) with ESMTP id 79EEF427F0 for ; Sun, 31 Jul 2022 00:36:54 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 77F3468BA53; Sun, 31 Jul 2022 03:36:51 +0300 (EEST) Received: from out162-62-57-210.mail.qq.com (out162-62-57-210.mail.qq.com [162.62.57.210]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 137E468B985 for ; Sun, 31 Jul 2022 03:36:43 +0300 (EEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=foxmail.com; s=s201512; t=1659227799; bh=O++pK1nhtgj4v3y+skQDG6ZCnZyAhRVtUutlnnk91Gs=; h=Date:From:To:Cc:Subject:References; b=sdvdq8CXJ4ELr+VNBNXTNBFXcz93wekgPdJofXvKEKXXeNn1EUlh0cL5wwdcxywmM p8pJWCwsIDvCzxZCQtVOQND18vKPUtcZqHD5T6kCGpi8SkAgeBvMxQ7rbV71vrZFh/ 1tFTURsGXtGoxoHJTtBPTpx9O0VnvxAxzXIScpIU= Received: from black ([121.35.185.46]) by newxmesmtplogicsvrsza7.qq.com (NewEsmtp) with SMTP id 92632C91; Sun, 31 Jul 2022 08:36:38 +0800 X-QQ-mid: xmsmtpt1659227798tv7fkvd0o Message-ID: X-QQ-XMAILINFO: N7h1OCCDntuj3NrasQqWK+LnB8VCwsZd7jUBxVniW4zY+7Eqys14bJCBo4a9Qd oFkpn9r+GA/zmMfeLL1tKzgORQTanEea1LWhDavmYC2ozASgvOk1Ofil0XvXrBSaLdtdHBxzeyfH ubzEC/LbdZf1w6zoJwhHOc26X/3wVeZkcBiYNBqTgGpFsq5jPXnNCH9ee03ZaC1e3NheofqotDoU +KdBPZuHewiDc6ejBnnp4lYrQOvZFktcEcQ+IujB7J0LF+rmyYPs3OhQ+e5UHzTcimfSdPJn2ASA 8uwJTB+tcCwGC+Jy8sk25qLcdok/+67AH3sMtZ7gezpNRIc5D8NmgH9plNER7ZQXbWM8le0CSUBo BqZQeXIzA6D9IKfCyd8+ujCJSFhUakuTrmIRU3XeqmR1AC6a44LazTPeAKEiDdNjXCadDzAqbDmB PhycSEe2LN6C2klWAJh2Odkb6rUJN9H1ga6/BI994307T4Mb9kUGoffPmWOu2qCGAfCZCERSVoJM Bk9uIokkw6ukIEC2uIn9J9pwl0VmELJQfTnW5bvIKbpJU/O/xHu3muylRK8YfvLbmhtRC69TPdqq 69xoRCC1lzfCrVjy9urZBgAWDj+OvVEultwxSWxci//SCutGWiLl5Lrzn72L/EH/wNbZHzSttOYx 8+OFR/TR2/Ah3mM2kNRP8bKwRZQURcGXt2t3Xxuq41wl5lnCirbzh/ccJaj9/11wBGxAhcv59W6Q ghlZ/71SSNRkj5v4yswcLtIoH8rtga86lw9KkTAMnjR990rmvGiJxSwJdjNbTrpcfPfarPCHf+os Xb10EJ1pltLGTdl/inb4fx3mBSmTjfeXudHEPfUb9uB0hj1fRq4+gyxhE47L6UOkNbwwNfmOSozg Ebce62Ns5CK19RPAlEuMJaKz2PWzOIWILagF/INb69 Date: Sun, 31 Jul 2022 08:36:38 +0800 From: "quinkblack@foxmail.com" To: ffmpeg-devel References: X-Priority: 3 X-GUID: 89761D87-F8D6-433D-AE67-F915D0D25965 X-Has-Attach: no X-Mailer: Foxmail 7.2.23.121[en] Mime-Version: 1.0 X-OQ-MSGID: <2022073108363775255419@foxmail.com> Subject: Re: [FFmpeg-devel] [PATCH] avformat/mov: get the correct fragment stream info when decrypting the sample 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 Cc: Wang Yaqiang 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: OK, I didn't see this one before working on ticket #9807. Comments inline. On 2022-07-22, 1035567130 wrote: >From: Wang Yaqiang > > When using mov_read_packet get a pkt, and the packet is encrypted, > it will invoke the cenc_filter func for process the decryption. > This function find the frag_stream_info according to frag_index.current, > but the value of frag_index.current is equal to the last read index of moof box, > this will cause pkt and frag_stream_info mismatch > > Signed-off-by: Wang Yaqiang > --- > libavformat/isom.h | 2 ++ > libavformat/mov.c | 33 ++++++++++++++++++++++++++------- > 2 files changed, 28 insertions(+), 7 deletions(-) > > diff --git a/libavformat/isom.h b/libavformat/isom.h > index f05c2d9c28..d90e16457b 100644 > --- a/libavformat/isom.h > +++ b/libavformat/isom.h > @@ -137,6 +137,8 @@ typedef struct MOVFragmentStreamInfo { > int64_t next_trun_dts; > int index_entry; > MOVEncryptionIndex *encryption_index; > + int stsd_id; // current fragment stsd_id > + int64_t max_pos; //current fragment max pos > } MOVFragmentStreamInfo; > typedef struct MOVFragmentIndexItem { > diff --git a/libavformat/mov.c b/libavformat/mov.c > index a09a762d91..c118072931 100644 > --- a/libavformat/mov.c > +++ b/libavformat/mov.c > @@ -1182,6 +1182,20 @@ static int mov_read_moov(MOVContext *c, AVIOContext *pb, MOVAtom atom) > return 0; /* now go for mdat */ > } > +static int get_frag_index_with_pkt(MOVFragmentIndex *frag_index, AVPacket *pkt) > +{ > + if (!pkt) return -1; > + for (int i = 0; i < frag_index->nb_items;i++) { > + MOVFragmentStreamInfo *frag_stream_info = frag_index->item[i].stream_info; > + if (frag_stream_info) { > + if (pkt->pos + pkt->size <= frag_stream_info->max_pos) { > + return i; > + } Coding style: no curly braces in this case. > + } > + } The search is inefficient. Use 'current' as a bookmark. > + return -1; > +} > + > static MOVFragmentStreamInfo * get_frag_stream_info( > MOVFragmentIndex *frag_index, > int index, > @@ -4868,9 +4882,10 @@ static int mov_read_tfhd(MOVContext *c, AVIOContext *pb, MOVAtom atom) > av_log(c->fc, AV_LOG_TRACE, "frag flags 0x%x\n", frag->flags); > frag_stream_info = get_current_frag_stream_info(&c->frag_index); > - if (frag_stream_info) > + if (frag_stream_info) { > frag_stream_info->next_trun_dts = AV_NOPTS_VALUE; > - > + frag_stream_info->stsd_id = frag->stsd_id; > + } > return 0; > } > @@ -5335,9 +5350,10 @@ static int mov_read_sidx(MOVContext *c, AVIOContext *pb, MOVAtom atom) > index = update_frag_index(c, offset); > frag_stream_info = get_frag_stream_info(&c->frag_index, index, track_id); > - if (frag_stream_info) > + if (frag_stream_info) { > frag_stream_info->sidx_pts = timestamp; > - > + frag_stream_info->max_pos = offset + size; Now it depends on sidx, which may not exist. So it doesn't work reliable. > + } > if (av_sat_add64(offset, size) != offset + (uint64_t)size || > av_sat_add64(pts, duration) != pts + (uint64_t)duration > ) > @@ -7101,13 +7117,16 @@ static int cenc_filter(MOVContext *mov, AVStream* st, MOVStreamContext *sc, AVPa > MOVEncryptionIndex *encryption_index; > AVEncryptionInfo *encrypted_sample; > int encrypted_index, ret; > - > - frag_stream_info = get_frag_stream_info(&mov->frag_index, mov->frag_index.current, st->id); > + int frag_index = get_frag_index_with_pkt(&mov->frag_index,pkt); > + if (frag_index < 0) { > + frag_index = mov->frag_index.current; > + } Search should be reliable. Use an unreliable search and fallback to another unreliable bookmark is still unrealiable. > + frag_stream_info = get_frag_stream_info(&mov->frag_index,frag_index, st->id); > encrypted_index = current_index; > encryption_index = NULL; > if (frag_stream_info) { > // Note this only supports encryption info in the first sample descriptor. > - if (mov->fragment.stsd_id == 1) { > + if (frag_stream_info->stsd_id == 1) { Good catch! It should go in a separate patch. > if (frag_stream_info->encryption_index) { > if (!current_index && frag_stream_info->index_entry) > sc->cenc.frag_index_entry_base = frag_stream_info->index_entry; > -- > 2.33.0 _______________________________________________ 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".