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 808C9438D7 for ; Tue, 2 Aug 2022 17:48:59 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 1B0F868BB0F; Tue, 2 Aug 2022 20:48:56 +0300 (EEST) Received: from mail-vs1-f48.google.com (mail-vs1-f48.google.com [209.85.217.48]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id E299A68BAFE for ; Tue, 2 Aug 2022 20:48:49 +0300 (EEST) Received: by mail-vs1-f48.google.com with SMTP id c3so15348988vsc.6 for ; Tue, 02 Aug 2022 10:48:49 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=content-transfer-encoding:in-reply-to:cc:references:to:from :content-language:subject:user-agent:mime-version:date:message-id :from:to:cc; bh=IgkahYk7KjzD+9c7ZluNrMRf8zwqyVUMl449eOv4AS4=; b=lM7FdRUPEr3dBdbdD13muZwwJ9iIRbI1ajyi2mO9ZDUCFsdLErFqkdlPAfJVHuWWUZ /zUReS8vQwYoXEywzATEZBVjA0kIYTsgfMeMyZpt9UyBgbW33Lp1sLSHMUhhzufg+KfB ZwlyFQApxD7sCvHxjB2JAhtCH9uPC2H7KqWHxr8MZaaHy/5mzwaqRCwlOrcNwxwRrHom 1tsB9IrJifPGU7nD3H8dHB9WFJwY1v7pi8O0jWCy6pTTI7krB1db87/gMMNWDbgZux4o cwGWTy8otlE/CL9Igt7ADLxmhbbrPj4urQOgq8fLxXGkWHhRv+2iDBSLbdDRwNAf4CKI 5H2w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:in-reply-to:cc:references:to:from :content-language:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc; bh=IgkahYk7KjzD+9c7ZluNrMRf8zwqyVUMl449eOv4AS4=; b=JBAsSucw0qlpTfxXnobAQwwFu8TUJ+7CmRoY41EZJFYhDatAda+2AeidunSi9KGg0m Lp+C38TKGFJ1+XCLfQVc/o6poVN/7hzOIpwXUmiYzC8PwsZCSgz581W/o80WPbE11GX+ g+gbtbncW+9VykIJsP7Q4rtj19iPLPaMPxIrCGNcdHhN3/IhbKJoy2LHdW1kF93pf1zW hq4V/DiW/18ahZNDuB5+O5pwYQ/BBXH9kL3pWnqy64lmRLu2CS32KwJYl28bpnrWy7xe NgRBp/w4Pq6nsrfMO65X0DAx2wyrMuQdocehEYUHNYOngBULbYDBamIxjAjVbUaJi9Mb 061A== X-Gm-Message-State: ACgBeo0V2k4SOdMNqOeCfb/LpF+AAFuylhgbf7BlRXJiCWcxw4Q5tCri SZEea5uC64YZUD4tDLakZTbP9bq4N3M= X-Google-Smtp-Source: AA6agR4rRvtgX8ZLsKUW3g3hBE+N4w9JYV1lQ2nevZN7oTTWJJLZ1GqJSoV80oMi2gjVcOIPZTohjw== X-Received: by 2002:a67:c01b:0:b0:386:a7cc:a153 with SMTP id v27-20020a67c01b000000b00386a7cca153mr3288957vsi.52.1659462527648; Tue, 02 Aug 2022 10:48:47 -0700 (PDT) Received: from [192.168.0.11] ([186.136.131.204]) by smtp.gmail.com with ESMTPSA id j20-20020ab064d4000000b00387499c5353sm6288163uaq.39.2022.08.02.10.48.46 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 02 Aug 2022 10:48:46 -0700 (PDT) Message-ID: Date: Tue, 2 Aug 2022 14:48:47 -0300 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101 Thunderbird/91.11.0 Content-Language: en-US From: James Almer To: ffmpeg-devel@ffmpeg.org References: <20220713175948.1955-1-jamrial@gmail.com> <8b484a4e-ba96-d54e-a4b5-84e1ee5e53f9@gmail.com> <3dfc3a5b-6b08-e36c-44e7-40cdf062cb3c@gmail.com> <970d0900-7e9c-5dcc-b306-aee15a84c2b6@gmail.com> <2d7edac4-e974-ddec-3f9e-9f131d56ee96@gmail.com> In-Reply-To: Subject: Re: [FFmpeg-devel] [PATCH] avcodec/aacdec: don't force HE-AACv2 profile if no PS info is present 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: Alex Converse Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: On 7/22/2022 8:10 PM, James Almer wrote: > On 7/22/2022 8:00 PM, Alex Converse wrote: >> On Fri, Jul 22, 2022 at 8:37 AM James Almer wrote: >>> On 7/22/2022 12:14 PM, Andreas Rheinhardt wrote: >>>> James Almer: >>>>> On 7/22/2022 11:56 AM, Andreas Rheinhardt wrote: >>>>>> James Almer: >>>>>>> On 7/22/2022 11:23 AM, Andreas Rheinhardt wrote: >>>>>>>> James Almer: >>>>>>>>> On 7/18/2022 10:57 AM, Andreas Rheinhardt wrote: >>>>>>>>>> James Almer: >>>>>>>>>>> On 7/14/2022 9:10 AM, Andreas Rheinhardt wrote: >>>>>>>>>>>> James Almer: >>>>>>>>>>>>> Should fix ticket #3361 >>>>>>>>>>>>> >>>>>>>>>>>>> Signed-off-by: James Almer >>>>>>>>>>>>> --- >>>>>>>>>>>>> This also needs an update to some fate ref samples i'll upload >>>>>>>>>>>>> before >>>>>>>>>>>>> pushing >>>>>>>>>>>>> (fate-aac-al_sbr_ps_04_ur and fate-aac-al_sbr_ps_06_ur which >>>>>>>>>>>>> are now >>>>>>>>>>>>> decoded >>>>>>>>>>>>> properly as he_aac mono, so the .s16 files need to be >>>>>>>>>>>>> replaced). >>>>>>>>>>>>> >>>>>>>>>>>> >> >> [snip] >> >>>>>>>>> >>>>>>>>> >>>>>>>>> it seems at least for these samples the fixed decoder does not >>>>>>>>> generate >>>>>>>>> a decoded stream comparable to the float one, so I'll just >>>>>>>>> upload a >>>>>>>>> new >>>>>>>>> raw pcm file. >>>>>>>> >>>>>>>> When I decode both of these streams with git master, the left >>>>>>>> channel is >>>>>>>> pretty much identical, yet the right channel of the fixed-point >>>>>>>> decoder >>>>>>>> is silent and the right channel of the floating point decoder is >>>>>>>> not. >>>>>>>> With this patch applied, the result are two mono streams that are >>>>>>>> pretty >>>>>>>> much identical: The test sample created by the floating-point >>>>>>>> decoder >>>>>>>> works with the fixed-point decoder test (if one uncomments and >>>>>>>> modifies >>>>>>>> the latter). So the issue with aac-al_sbr_ps_06_ur is not a >>>>>>>> reason to >>>>>>>> upload new samples. >>>>>>> >>>>>>> Ok, can you suggest how to add a test that decodes with the fixed >>>>>>> point >>>>>>> decoder then compares that with the output of the float decoder? Is >>>>>>> there a helper in fate.sh already for this? >>>>>>> >>>>>> >>>>>> There is currently no helper in fate-run.sh for this. >>>>> >>>>> Yeah, figures that's the case. Can you suggest how one would work >>>>> for this? >>>>> >>>> >>>> A new function in fate-run.sh seems to be necessary. Given that a >>>> similar situation exists for AC-3 we should not hardcode aac; >>>> instead it >>>> should have two parameters for the float and the fixed decoder. Then it >>>> should decode the input file twice and do the same comparison that the >>>> current tests use (they use the oneoff method, which results in >>>> do_tiny_psnr with MAXDIFF being called). >>>> I think the tests for the fixed-point decoder (with checksums) >>>> should be >>>> separate, so that one can test this without the floating-point decoder >>>> being present. >>>> >>>>>> >>>>>>>> >>>>>>>> - Andreas >>>>>>>> >>>>>>>> PS: libfdk-aac produces a file that looks pretty much like the >>>>>>>> floating >>>>>>>> point decoder from git master. Are you sure your patch is correct? >>>>>>> >>>>>>> Yes, they duplicate the single channel in the stream and output >>>>>>> it as >>>>>>> stereo, something that should be done by a filter if that's what the >>>>>>> user wants. Decoding a mono sample should generate a mono stream. >>>>>> >>>>>> Not really. The channels are different. >>>>> >>>>> ./ffmpeg -c:a libfdk_aac -i ../samples/aac/al_sbr_ps_04_new.mp4 -af >>>>> channelmap=channel_layout=mono:map=0 -f md5 - >>>>> >>>>> has the same result as >>>>> >>>>> ./ffmpeg -c:a libfdk_aac -i ../samples/aac/al_sbr_ps_04_new.mp4 -af >>>>> channelmap=channel_layout=mono:map=1 -f md5 - >>>>> >>>>> Same with the samples in the ticket. >>>>> >>>> >>>> This seems to be true for al_sbr_ps_04_new.mp4; but it is not true for >>>> al_sbr_ps_06_new.mp4. >>> >>> So looks like nearly a hundred samples into al_sbr_ps_06_new.mp4 frames >>> start containing PS info. With this patch the decoder properly decodes >>> the first hundred as mono, but since the decoder is locked, it will keep >>> decoding the stereo samples as mono. >>> >> >> Hey all, >> >> I thought I should share a little bit of context about this problem, >> but I don't mean to come back from nowhere and try to overrule you >> all. Do what you decide is best. >> >> An HE-AACv2 decoder treating unsignaled mono as stereo is an >> intentional design trade-off that the MPEG audio committee made. It is >> a tradeoff that the FFmpeg decoder has mimicked for a number of years. >> If you want to revisit the trade-off (and there may very well be good >> reasons to do that) that's fine, but I think treating the current >> behavior as a "bug" is the wrong approach. >> >> In fact, those fate tests are based on a Coding Technologies test >> suite designed to validate a decoder conformed to the MPEG behavior. >> >> Parametric Stereo is nested inside the SBR extension after the main >> SBR payload which itself is nested inside the AAC raw data block after >> the main channel elements. It takes a full bitstream parse of both the >> AAC and SBR layers and finding an SBR intra-frame to even see if PS is >> present. >> >> >> As for why I think MPEG made this trade-off (my opinion of why they >> did this) : >> - It enables cutting (or joining a stream of) audio at arbitrary >> frames without losing PS content or stereo detection. >> - Most devices with mono output can support downmixing from stereo. >> - Down mixing stereo to mono can be ugly with regard to phase but it >> doesn't have nearly the complexity of the taste/environment/judgment >> factor that surround sound to stereo downmix does. >> - In transcode scenarios, most output codecs can support encoding >> stereo formatted audio where both channels are identical quite >> efficiently (even in AAC-LC with mid-side coding the overhead for >> mono-coded as stereo is a single digit number of bits per frame). >> - On playback on a stereo device it doesn't matter if the decoded >> signal is one channel played out of both speakers or two channels that >> are identical. >> - This behavior can be overridden with more complicated signaling the >> extradata (but this requires a transport that supports such signaling >> and doesn't simply wrap ADTS). >> - The folks working on iterating "MPEG-2 NBC" into the "MPEG-4 Audio" >> monstrosity were primarily focused on getting their new features used. > > Thanks a lot for clarifying this. This "bug" has been pending for nearly > a decade now... > > So i withdraw this patch and I'll close the ticket as invalid. I'll then > see if adding a downmix input option is feasible, but the user could > just request to downmix to mono by a filter down the chain (which is > what the audiomatch tests do), so maybe it's not that useful. Do you know for that matter where in the spec is this defined? I see fdk-aac also does the same, so it's clearly intentional, but where is it specified? _______________________________________________ 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".