From patchwork Wed Oct 4 08:33:11 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tobias Rapp X-Patchwork-Id: 5407 Delivered-To: ffmpegpatchwork@gmail.com Received: by 10.2.161.76 with SMTP id m12csp590673jah; Wed, 4 Oct 2017 01:33:36 -0700 (PDT) X-Google-Smtp-Source: AOwi7QDUnGYsRyclzOAgnXoZ7Wrqz9WhwqPSR69NqrYUBBjR+qN0dnSslatWZWLtyB2v1lhv/soI X-Received: by 10.28.217.130 with SMTP id q124mr3403070wmg.30.1507106016855; Wed, 04 Oct 2017 01:33:36 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1507106016; cv=none; d=google.com; s=arc-20160816; b=j+ukDBePGY1J/Z3TJWXBd+N70ktckw7JcNteiVwKH+5X8iBZmd1C3qfSXc72lzfPVF WPb/5oDNDSdqoflR0/Lz5wCU6Mg2SfiOiF8a6q1SeYQOWmZOrRKxFDVtMu6KCsTTq6DO ZEpUfrqObmSwOiAO8eGQwpA/y3fFlbrt9hYH9m7vsjt5T+HxmXhnk+KjTmfLZnWzQIKn jMJpx20/8VER38HCyQDoeC3aM1p/F7E69cyy2Llabm4tqjSCSOBGWT7IkM6cZzKZ33VZ EBeftz4/xwUkLbC+0gs9pCuURqEmHtYEbRLRitYI63pyilI34xf3UJRArmBnrMcS0JV/ Cs1g== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=sender:errors-to:cc:reply-to:list-subscribe:list-help:list-post :list-archive:list-unsubscribe:list-id:precedence:subject :content-language:in-reply-to:mime-version:user-agent:date :message-id:organization:from:references:to:delivered-to :arc-authentication-results; bh=76kLySysc45ErL5ApeDMNZj5ZJeMabS98PZ9RmAmjBs=; b=HsqJ0EEnKXlMbufQAjb5XZzVi5ioH3BxxgXFwvUitGQ0cX75ZDxw9qYpg4f6TVDg49 M8+z1DdNWlz/Z4JZGbY+0Ih/RiqItBr8nwKFNrHUtdZg/qtb1vLLgl9IbNlI1o6ch5XS h8C34dr6vM/BW/Z4X06r4R0UUM0THTCsQ5UqUIZ4XnoqDceGd7gnMwTnN9XrHzZQfK12 eO2QwDMCRMq33YkPpR2vdS6e5go9GiYFj11ltdstJte63Trz67pu+00VVYBaLikRaZXo Xv+kBEjf2/KtNADQy0MExEh/sb51PedBt4zceuNcRctGKqf1bp26FYtV4qBI9XwRzjwn j8yA== ARC-Authentication-Results: i=1; mx.google.com; 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 m63si6125824wmh.163.2017.10.04.01.33.35; Wed, 04 Oct 2017 01:33:36 -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; 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 7FD5768A20E; Wed, 4 Oct 2017 11:33:17 +0300 (EEST) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mx02.mail.netstorage.at (mx02.mail.netstorage.at [89.207.146.155]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 4449F68996C for ; Wed, 4 Oct 2017 11:33:10 +0300 (EEST) Received: from p1002.netstorage.at (p1002.netstorage.at [89.207.146.186]) by mx02.mail.netstorage.at (Postfix) with ESMTPS id A733DA42C3; Wed, 4 Oct 2017 10:33:12 +0200 (CEST) Received: from mailix (noaport.de [46.237.252.213]) by p1002.netstorage.at (Postfix) with ESMTPA id 4FAE18193E; Wed, 4 Oct 2017 10:33:12 +0200 (CEST) Received: from [127.0.0.1] (HSI-KBW-46-237-252-214.hsi.kabel-badenwuerttemberg.de [46.237.252.214]) by mailix with ESMTPSA (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128) ; Wed, 4 Oct 2017 10:33:12 +0200 To: ffmpeg-devel@ffmpeg.org References: <1506697696-21235-1-git-send-email-t.rapp@noa-archive.com> <1506697696-21235-2-git-send-email-t.rapp@noa-archive.com> <20170930004810.GK30840@nb4> From: Tobias Rapp Organization: NOA GmbH Message-ID: <9bc3ecc2-109e-3c8f-f5a7-e102a29e731d@noa-archive.com> Date: Wed, 4 Oct 2017 10:33:11 +0200 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.3.0 MIME-Version: 1.0 In-Reply-To: <20170930004810.GK30840@nb4> Content-Language: en-US X-PPP-Message-ID: <20171004083312.2519.43456@p1002.netstorage.at> X-PPP-Vhost: noa-archive.com X-NetStorage-MailScanner-Information: Please contact the ISP for more information X-NetStorage-MailScanner-ID: A733DA42C3.A6A43 X-NetStorage-MailScanner: Found to be clean X-NetStorage-MailScanner-SpamCheck: not spam (whitelisted), SpamAssassin (nicht zwischen gespeichert, Wertung=-0.5, benoetigt 6, autolearn=not spam, BAYES_00 -0.50) X-NetStorage-MailScanner-From: t.rapp@noa-archive.com X-Spam-Status: No Subject: Re: [FFmpeg-devel] [PATCH 2/2] avformat/wavenc: skip writing peak-of-peaks position when writing peaks only 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 Cc: Georg Lippitsch Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" On 30.09.2017 02:48, Michael Niedermayer wrote: > On Fri, Sep 29, 2017 at 05:08:16PM +0200, Tobias Rapp wrote: >> Signed-off-by: Tobias Rapp >> --- >> libavformat/wavenc.c | 5 ++++- >> tests/ref/lavf/wav_peak_only | 2 +- >> 2 files changed, 5 insertions(+), 2 deletions(-) > > The commit message doest say why this chnage is done As I understand the BWF peak envelope chunk definition in EBU Tech 3285 - Supplement 3 the "dwPosPeakOfPeaks" field contains the (absolute) byte position of the peak audio sample within the _data_ chunk: """ The peak-of-peaks is first audio sample whose absolute value is the maximum value of the entire audio file. Rather than storing the peak-of-peaks as a sample value, the position of the peak of the peaks is stored. In other words, an audio sample frame index is stored. An application then knows where to read the peak of the peaks in the audio file. It would be more difficult to store a value for peak since this is dependant on the binary format of the audio samples (integers, floats, double...). If the value is 0xFFFFFFFF, then that means that the peak of the peaks is unknown. """ As a peak-only file doesn't contain a data chunk it would make no sense to store the data position. But when looking at FFmpeg's implementation within wavenc.c I notice now, that the implementation is using a totally different interpretation of the spec and stores the peak frame index (position relative to peak chunk data) instead. In my opinion the current implementation of "dwPosPeakOfPeaks" is wrong and the patch should actually be changed to always write "-1" unconditionally until the peak-of-peaks feature is implemented correctly. See attached patch update. Regards, Tobias From 07a2414a1154dd51b9da09bbd2c877363860e825 Mon Sep 17 00:00:00 2001 From: Tobias Rapp Date: Fri, 29 Sep 2017 16:32:20 +0200 Subject: [PATCH v2 2/2] avformat/wavenc: skip writing incorrect peak-of-peaks position value According to EBU tech 3285 supplement 3 the dwPosPeakOfPeaks field should contain the absolute position to the maximum audio sample value, but the current implementation writes the relative peak frame index instead. Fix the issue by writing the "unknown" value (-1) instead for now until the feature is implemented correctly. Signed-off-by: Tobias Rapp --- libavformat/wavenc.c | 5 +---- tests/ref/lavf/wav_peak | 2 +- tests/ref/lavf/wav_peak_only | 2 +- 3 files changed, 3 insertions(+), 6 deletions(-) diff --git a/libavformat/wavenc.c b/libavformat/wavenc.c index adb20cb..a6060de 100644 --- a/libavformat/wavenc.c +++ b/libavformat/wavenc.c @@ -74,7 +74,6 @@ typedef struct WAVMuxContext { uint32_t peak_num_frames; uint32_t peak_outbuf_size; uint32_t peak_outbuf_bytes; - uint32_t peak_pos_pop; uint16_t peak_pop; uint8_t *peak_output; int last_duration; @@ -215,8 +214,6 @@ static void peak_write_frame(AVFormatContext *s) peak_of_peaks = FFMAX3(wav->peak_maxpos[c], wav->peak_maxneg[c], wav->peak_pop); - if (peak_of_peaks > wav->peak_pop) - wav->peak_pos_pop = wav->peak_num_frames; wav->peak_pop = peak_of_peaks; if (wav->peak_outbuf_size - wav->peak_outbuf_bytes < @@ -287,7 +284,7 @@ static int peak_write_chunk(AVFormatContext *s) avio_wl32(pb, wav->peak_block_size); /* frames per value */ avio_wl32(pb, par->channels); /* number of channels */ avio_wl32(pb, wav->peak_num_frames); /* number of peak frames */ - avio_wl32(pb, wav->peak_pos_pop); /* audio sample frame index */ + avio_wl32(pb, -1); /* audio sample frame position (TODO) */ avio_wl32(pb, 128); /* equal to size of header */ avio_write(pb, timestamp, 28); /* ASCII time stamp */ ffio_fill(pb, 0, 60); diff --git a/tests/ref/lavf/wav_peak b/tests/ref/lavf/wav_peak index aa7e5fc..861b246 100644 --- a/tests/ref/lavf/wav_peak +++ b/tests/ref/lavf/wav_peak @@ -1,3 +1,3 @@ -35148d1f6e66b0080893851d917ecbf4 *./tests/data/lavf/lavf.peak.wav +105805963fb767d00da056f42f32d9f3 *./tests/data/lavf/lavf.peak.wav 89094 ./tests/data/lavf/lavf.peak.wav ./tests/data/lavf/lavf.peak.wav CRC=0x3a1da17e diff --git a/tests/ref/lavf/wav_peak_only b/tests/ref/lavf/wav_peak_only index dccd0e7..b203d03 100644 --- a/tests/ref/lavf/wav_peak_only +++ b/tests/ref/lavf/wav_peak_only @@ -1,2 +1,2 @@ -b609a363e6d490710ed52231a8d09d3c *./tests/data/lavf/lavf.peak_only.wav +f1a8aeeae8069f3992c4d780436c3d23 *./tests/data/lavf/lavf.peak_only.wav 832 ./tests/data/lavf/lavf.peak_only.wav