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 8499D43A95 for ; Wed, 6 Jul 2022 21:50:55 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id A6F5168BA44; Thu, 7 Jul 2022 00:50:53 +0300 (EEST) Received: from w4.tutanota.de (w4.tutanota.de [81.3.6.165]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id EBDCB68B7D8 for ; Thu, 7 Jul 2022 00:50:46 +0300 (EEST) Received: from w3.tutanota.de (unknown [192.168.1.164]) by w4.tutanota.de (Postfix) with ESMTP id 6D908106015C for ; Wed, 6 Jul 2022 21:50:46 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; t=1657144246; s=s1; d=lynne.ee; h=From:From:To:To:Subject:Subject:Content-Description:Content-ID:Content-Type:Content-Type:Content-Transfer-Encoding:Content-Transfer-Encoding:Cc:Date:Date:In-Reply-To:In-Reply-To:MIME-Version:MIME-Version:Message-ID:Message-ID:Reply-To:References:References:Sender; bh=kQNaYkoV8RyGtWlmlJR20NByzpVQsUJBueQbFticFqA=; b=ffCUDr0qNGTBAjY1sAcKSsfDTkGBuSiB6SM9jmNMFD3a5gz6twlvW3R3EwdL9UCD mpLICCaNAaxJwsuYTrecZuTzVURMAujZnUKPoIVB+kpPOEH/fM/uoc04im66zJitYtB f0qfn+mYSpZxE+5DB2ejvbEoenBH7iZqqVDhS2QwERwnfmGf1SKCOt1Ox8uSuBiV2Mt FLxRmMD1kA+cFhW+yyvoDftbyxZuBz3QIqBYFroCNj156g5C7rPKTuRa2CDyNpW153O pbzOpMeEbl0FzVm/DcgZBZj284KBPX6T24kczdiVb4CutXq+7TP7nrP7wgxnAF3llty d9bFl4ERiw== Date: Wed, 6 Jul 2022 23:50:46 +0200 (CEST) From: Lynne To: FFmpeg development discussions and patches Message-ID: In-Reply-To: <20220704141340.493931-3-philip-dylan.gleonec@savoirfairelinux.com> References: <1179126120.329661.1613560269226.JavaMail.zimbra@savoirfairelinux.com> <780b5984-3673-4b74-8883-63b980a43cc1@gmail.com> <0f24eb6c-375e-fc19-dc03-8b759d601f7a@savoirfairelinux.com> <20210218163835.142726-1-philip-dylan.gleonec@savoirfairelinux.com> <20220316140045.369016-1-philip-dylan.gleonec@savoirfairelinux.com> <20220316140045.369016-3-philip-dylan.gleonec@savoirfairelinux.com> <20220617162150.GW396728@pb2> <20220704141340.493931-1-philip-dylan.gleonec@savoirfairelinux.com> <20220704141340.493931-3-philip-dylan.gleonec@savoirfairelinux.com> MIME-Version: 1.0 Subject: Re: [FFmpeg-devel] [PATCH v2 2/2] avcodec/libopusdec: Enable FEC/PLC 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: Jul 4, 2022, 16:13 by philip-dylan.gleonec@savoirfairelinux.com: > Adds FEC/PLC support to libopus. The lost packets are detected as a > discontinuity in the audio stream. When a discontinuity is used, this > patch tries to decode the FEC data. If FEC data is present in the > packet, it is decoded, otherwise audio is re-created through PLC. > > This patch is based on Steinar H. Gunderson contribution, and corrects > the pts computation: all pts are expressed in samples instead of time. > This patch also adds an option "decode_fec" which enables or disables > FEC decoding. This option is disabled by default to keep consistent > behaviour with former versions. > > A number of checks are made to ensure compatibility with different > containers. Indeed, video containers seem to have a pts expressed in ms > while it is expressed in samples for audio containers. It also manages > the cases where pkt->duration is 0, in some RTP streams. This patch > ignores data it can not reconstruct, i.e. packets received twice and > packets with a length that is not a multiple of 2.5ms. > > Signed-off-by: Philip-Dylan Gleonec > Co-developed-by: Steinar H. Gunderson > --- > libavcodec/libopusdec.c | 105 +++++++++++++++++++++++++++++++++++----- > 1 file changed, 94 insertions(+), 11 deletions(-) > > diff --git a/libavcodec/libopusdec.c b/libavcodec/libopusdec.c > index 316ab0f2a7..f5d0e95fc8 100644 > --- a/libavcodec/libopusdec.c > +++ b/libavcodec/libopusdec.c > @@ -44,10 +44,15 @@ struct libopus_context { > #ifdef OPUS_SET_PHASE_INVERSION_DISABLED_REQUEST > int apply_phase_inv; > #endif > + int decode_fec; > + int64_t expected_next_pts; > }; > > #define OPUS_HEAD_SIZE 19 > > +// Sample rate is constant as libopus always output at 48kHz > +static const AVRational opus_timebase = { 1, 48000 }; > + > static av_cold int libopus_decode_init(AVCodecContext *avc) > { > struct libopus_context *opus = avc->priv_data; > @@ -140,6 +145,8 @@ static av_cold int libopus_decode_init(AVCodecContext *avc) > /* Decoder delay (in samples) at 48kHz */ > avc->delay = avc->internal->skip_samples = opus->pre_skip; > > + opus->expected_next_pts = AV_NOPTS_VALUE; > + > return 0; > } > > @@ -160,25 +167,100 @@ static int libopus_decode(AVCodecContext *avc, AVFrame *frame, > int *got_frame_ptr, AVPacket *pkt) > { > struct libopus_context *opus = avc->priv_data; > - int ret, nb_samples; > + uint8_t *outptr; > + int ret, nb_samples = 0, nb_lost_samples = 0, nb_samples_left; > + > + // If FEC is enabled, calculate number of lost samples > + if (opus->decode_fec && > + opus->expected_next_pts != AV_NOPTS_VALUE && > + pkt->pts != AV_NOPTS_VALUE && > + pkt->pts != opus->expected_next_pts) { > + // Cap at recovering 120 ms of lost audio. > + nb_lost_samples = pkt->pts - opus->expected_next_pts; > + nb_lost_samples = FFMIN(nb_lost_samples, MAX_FRAME_SIZE); > + // pts is expressed in ms for some containers (e.g. mkv) > + // FEC only works for SILK frames (> 10ms) > + // Detect if nb_lost_samples is in ms, and convert in samples if it is > + if (nb_lost_samples > 0) { > + if (avc->pkt_timebase.den != 48000) { > + nb_lost_samples = av_rescale_q(nb_lost_samples, avc->pkt_timebase, opus_timebase); > + } > + // For FEC/PLC, frame_size has to be to have a multiple of 2.5 ms > + if (nb_lost_samples % (5LL * opus_timebase.den / 2000)) { > + nb_lost_samples -= nb_lost_samples % (5LL * opus_timebase.den / 2000); > I counted not two, but three different coding styles used in both patches. Fix it. The lost samples count is very wrong for Ogg Opus files, and in general it's simply incorrect. You *always* need to convert PTS into samples properly via the timebase, and you even hardcode a random timebase that happens to correspond to samples in some conditions. Corrupt ogg files have a duration that's 10x longer than the input, whilst disabling FEC or using our native decoder outputs a correct number of samples. You can use ffmpeg -i test.opus -c:a copy -bsf:a noise=amount=0:dropamount=2 -y test2.opus to corrupt Ogg Opus files and test yourself. I used 100% packet loss percentage for my tests. _______________________________________________ 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".