From patchwork Wed Aug 8 22:41:21 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Felicia Lim X-Patchwork-Id: 9939 Delivered-To: ffmpegpatchwork@gmail.com Received: by 2002:a02:104:0:0:0:0:0 with SMTP id c4-v6csp1438590jad; Wed, 8 Aug 2018 15:41:45 -0700 (PDT) X-Google-Smtp-Source: AA+uWPyvcbp5DhiKgXGzSW4ckreNz4eRkkUkvQUAh1DV7KHV7kHpCAYoxWJckA1U0LzLvNy3BJrt X-Received: by 2002:adf:d1cf:: with SMTP id m15-v6mr3303301wri.138.1533768105848; Wed, 08 Aug 2018 15:41:45 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1533768105; cv=none; d=google.com; s=arc-20160816; b=o6CulsJC617eP9GCXmQHGRTVCbsktSb16oKfU5Blvykr16JPw2J6noHgo4TdHgvNhq F2yJ35ZufFNupY1WSCeyoecJzCETIwWPnXWEH1WLyWGYPEa6xBoLnRyf2OWtERJYKRhz krq59iHf2+YDysYgPmJlKmCIUf9BZDr8wAUPfcUADxNFMKb3yBN8ft+iu4HXVDwm7CMQ IgH+NA2SkTWFxW63OLoXgt9+QUkxCEUd24pttXTmiUoD0lJtnCHlWXQvinBDVGqsXGdo kYVhfQsqGDMr5z2htQofIRaHrnt1em4fXGveLO4W4yvyQAdmeWyq16Hn44aswOT0GD38 X/aw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=sender:errors-to:reply-to:list-subscribe:list-help:list-post :list-archive:list-unsubscribe:list-id:precedence:subject:to :message-id:date:from:in-reply-to:references:mime-version :dkim-signature:delivered-to:arc-authentication-results; bh=fWu9yhowy89xptZQ4vooPX9SEvs48ZWyX0Gyqzw/aoA=; b=CjYSOXP4t5yF6gebH4wOvSlUUCntO1zxz27vKGDSHHmYVCHHySQK7MmbileUplWK5M 90pzjygUOZ6piKzENiluno7UlHrfcncwAw4FngCngYhHsEetnEHy0+HRnAAqIM7qUMBG Y5gEsnWDIEKMkEQqi8ILz5504nYDWVWHG7Ba/uaPPxZwBmbnDg9an3DyQMyR7d7uPCZZ ccWE643KwoWGA54jmt897aaXkdA4621TOZeLONPyWmy/T2Z++xLMuttClAMEtUnP9gCq o6uRNeQEV/5If76nd8gOzbL8Hx4cpFU+dXWYYiXp13twlvIpOseLx8s1rDfa9tBIC7aN KsMQ== ARC-Authentication-Results: i=1; mx.google.com; dkim=neutral (body hash did not verify) header.i=@google.com header.s=20161025 header.b=VzRGLVin; spf=pass (google.com: domain of ffmpeg-devel-bounces@ffmpeg.org designates 79.124.17.100 as permitted sender) smtp.mailfrom=ffmpeg-devel-bounces@ffmpeg.org Return-Path: Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org. [79.124.17.100]) by mx.google.com with ESMTP id y20-v6si4292172wmc.29.2018.08.08.15.41.44; Wed, 08 Aug 2018 15:41:45 -0700 (PDT) Received-SPF: pass (google.com: domain of ffmpeg-devel-bounces@ffmpeg.org designates 79.124.17.100 as permitted sender) client-ip=79.124.17.100; Authentication-Results: mx.google.com; dkim=neutral (body hash did not verify) header.i=@google.com header.s=20161025 header.b=VzRGLVin; spf=pass (google.com: domain of ffmpeg-devel-bounces@ffmpeg.org designates 79.124.17.100 as permitted sender) smtp.mailfrom=ffmpeg-devel-bounces@ffmpeg.org Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 27D1968A4B6; Thu, 9 Aug 2018 01:41:22 +0300 (EEST) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mail-wm0-f50.google.com (mail-wm0-f50.google.com [74.125.82.50]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 89528689E8B for ; Thu, 9 Aug 2018 01:41:15 +0300 (EEST) Received: by mail-wm0-f50.google.com with SMTP id o11-v6so4321311wmh.2 for ; Wed, 08 Aug 2018 15:41:36 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to; bh=R3iJNV2p7iEz3BOnKZaugALWGXBtQEbB/+xjBDmC8bs=; b=VzRGLVin6w4Zx1RtPEjwMU75+A8wNFLf4xPK/2Syegh40Zy1Nk2Gq7ORQQ2Baf57I0 8PGACIBJUPPs4sPahtRLoRvbNTnBGDqR+2EpS33JIjC3Zh/VDldKITO8nQ6oMSt0wwpE LnlviXgMhzFVlz8cxqGt6w42yAN5cQuU36wa+yJxjyD3nB2aoW8WL/vRZRM9n6sB/Rua KEN6HQQTEdB8xJZSpbAkO2qkZP8yVGZg/y/uZDvx/r2T87UVpsRYlsvntXtzmBkoFf02 SEBcgDs/n8V0uwpXNjuaAxA0OiM1gTzFtgYyOA3G3aO08gLku5I3vMkqn+kFTLIR44ZB UBaA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to; bh=R3iJNV2p7iEz3BOnKZaugALWGXBtQEbB/+xjBDmC8bs=; b=j/yIfLqr8eAMbo5uCCKHITLMf0mxaCSga6NAkHQ6UwozB8DPFCd8x1+TYfxlkqTMYA StVU0SkKfSnO79CPg+VE/OCgu71jdmgJRjRK6a0+m8gXcMH7fGR0VvkGJwe1o94kKyWs 94gret6wywgxfoW6453pMGEnFp9KA9cBNkdZT0Amc/2tjFFoJbgLTysgZjVrhp4tgfFu xFegQVf9RdOi+cNyebSaFgsF52P1uNKlMOBexfCzhy2FjHpZ0e600IoQ4tYIl+Tf20qc NCysmopgxmIByuMHc+QZTD79d9VuPiO7AoHisoQSq3fVz8q06OXlVAQQfNRle6QCfZhk sl6Q== X-Gm-Message-State: AOUpUlE0fK53HESvYKpNWu7cqDq+NevgnojnhKmDc8GAKZyjZt4uE0Wa xRWXJIoN2pZTUjQn2UUHO4X8GqmTEF5bdKg5kXJdciD7+4U= X-Received: by 2002:a1c:1510:: with SMTP id 16-v6mr29391wmv.74.1533768095477; Wed, 08 Aug 2018 15:41:35 -0700 (PDT) MIME-Version: 1.0 References: <20180727194431.191229-1-flim@google.com> In-Reply-To: From: Felicia Lim Date: Wed, 8 Aug 2018 15:41:21 -0700 Message-ID: To: FFmpeg development discussions and patches X-Content-Filtered-By: Mailman/MimeDel 2.1.20 Subject: Re: [FFmpeg-devel] [PATCH] avcodec/libopusenc: allow encoding channel mapping 2 X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.20 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 Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" I've attached the patch with the updated description, please let me know if I should make any other changes. Thanks, Felicia On Mon, Jul 30, 2018 at 10:50 AM Felicia Lim wrote: > On Sat, Jul 28, 2018 at 10:46 AM Rostislav Pehlivanov > wrote: > >> On 27 July 2018 at 20:44, Felicia Lim >> wrote: >> >> > --- >> > libavcodec/libopusenc.c | 22 ++++++++++++++++++++++ >> > 1 file changed, 22 insertions(+) >> > >> > diff --git a/libavcodec/libopusenc.c b/libavcodec/libopusenc.c >> > index 4ae81b0bb2..6b450ec317 100644 >> > --- a/libavcodec/libopusenc.c >> > +++ b/libavcodec/libopusenc.c >> > @@ -27,6 +27,7 @@ >> > #include "bytestream.h" >> > #include "internal.h" >> > #include "libopus.h" >> > +#include "mathops.h" >> > #include "vorbis.h" >> > #include "audio_frame_queue.h" >> > >> > @@ -200,6 +201,21 @@ static int >> libopus_check_vorbis_layout(AVCodecContext >> > *avctx, int mapping_family >> > return 0; >> > } >> > >> > +static int libopus_check_ambisonics_channels(AVCodecContext *avctx) { >> > + int channels = avctx->channels; >> > + int ambisonic_order = ff_sqrt(channels) - 1; >> > + if (channels != ((ambisonic_order + 1) * (ambisonic_order + 1)) && >> > + channels != ((ambisonic_order + 1) * (ambisonic_order + 1) + >> 2)) { >> > + av_log(avctx, AV_LOG_ERROR, >> > + "Ambisonics coding is only specified for channel counts" >> > + " which can be written as (n + 1)^2 or (n + 1)^2 + 2" >> > + " for nonnegative integer n\n"); >> > + return AVERROR_INVALIDDATA; >> > + } >> > + >> > + return 0; >> > +} >> > + >> > static int libopus_validate_layout_and_get_channel_map( >> > AVCodecContext *avctx, >> > int mapping_family, >> > @@ -231,6 +247,12 @@ static int libopus_validate_layout_and_ >> > get_channel_map( >> > channel_map = >> ff_vorbis_channel_layout_offsets[avctx->channels >> > - 1]; >> > } >> > break; >> > + case 2: >> > + ret = libopus_check_max_channels(avctx, 227); >> > + if (ret == 0) { >> > + ret = libopus_check_ambisonics_channels(avctx); >> > + } >> > >> >> No brackets needed, also if (ret) looks better imo. >> > > Thanks for reviewing. Would this change would break with the style > elsewhere in this function? I'm happy to change if you still think I should > do it. > > I also just realized the patch description should be "Remove warning when > trying to encode channel mapping 2": it already encodes even without this > patch. Will send an update after the above comment is resolved. > > >> Does libopus understand channel mapping 2 as ambisonics? What about the >> libopus_generic_encoder_() API? Doesn't that need to be used to encode >> ambisonics, like the patch by Drew did? >> > > Yes, libopus also understands channel mapping 2 as ambisonics by default > now. > > Ch map 2 uses the normal opus_multistream_encode(), so no further changes > are needed. On the other hand, ch map 3 uses the new > opus_projection_encode(), hence Drew's introduction of > libopus_generic_encoder_() to handle the switch as needed. > > > _______________________________________________ >> ffmpeg-devel mailing list >> ffmpeg-devel@ffmpeg.org >> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel >> > From 75a2470f26ea0d4c5bf8482001ce7d32212ba06f Mon Sep 17 00:00:00 2001 From: Felicia Lim Date: Mon, 30 Jul 2018 12:59:44 -0700 Subject: [PATCH] avcodec/libopusenc: Fix warning when encoding ambisonics with channel mapping 2 Also adds checks on the number of channels. --- libavcodec/libopusenc.c | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/libavcodec/libopusenc.c b/libavcodec/libopusenc.c index 4ae81b0bb2..6b450ec317 100644 --- a/libavcodec/libopusenc.c +++ b/libavcodec/libopusenc.c @@ -27,6 +27,7 @@ #include "bytestream.h" #include "internal.h" #include "libopus.h" +#include "mathops.h" #include "vorbis.h" #include "audio_frame_queue.h" @@ -200,6 +201,21 @@ static int libopus_check_vorbis_layout(AVCodecContext *avctx, int mapping_family return 0; } +static int libopus_check_ambisonics_channels(AVCodecContext *avctx) { + int channels = avctx->channels; + int ambisonic_order = ff_sqrt(channels) - 1; + if (channels != ((ambisonic_order + 1) * (ambisonic_order + 1)) && + channels != ((ambisonic_order + 1) * (ambisonic_order + 1) + 2)) { + av_log(avctx, AV_LOG_ERROR, + "Ambisonics coding is only specified for channel counts" + " which can be written as (n + 1)^2 or (n + 1)^2 + 2" + " for nonnegative integer n\n"); + return AVERROR_INVALIDDATA; + } + + return 0; +} + static int libopus_validate_layout_and_get_channel_map( AVCodecContext *avctx, int mapping_family, @@ -231,6 +247,12 @@ static int libopus_validate_layout_and_get_channel_map( channel_map = ff_vorbis_channel_layout_offsets[avctx->channels - 1]; } break; + case 2: + ret = libopus_check_max_channels(avctx, 227); + if (ret == 0) { + ret = libopus_check_ambisonics_channels(avctx); + } + break; case 255: ret = libopus_check_max_channels(avctx, 254); break; -- 2.18.0.597.ga71716f1ad-goog