[FFmpeg-devel] [PATCH 2/2] avformat/wavenc: skip writing peak-of-peaks position when writing peaks only

[Tobias Rapp]

On 05.10.2017 00:05, Michael Niedermayer wrote:
> On Wed, Oct 04, 2017 at 10:33:11AM +0200, Tobias Rapp wrote:
>> 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 <t.rapp at noa-archive.com>
>>>> ---
>>>>   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.
> 
> have you confirmed that files not created by ffmpeg mismatch what we
> do ?
> 
> If so this is ok but
> please bump the micro version of libavformat every time
> this muxer behavior changes. So demuxers can know what they are dealing
> with

Actually our own software (NOA MediaButler) always sets this field to 
"-1" when writing BWF files exactly due to the hassle of calculating the 
value. And when reading a Wave file for normalization the peak-of-peaks 
is re-calculated within the target time-range anyway, so this field is 
ignored.

Unfortunately I don't have access to other software that writes "levl" 
chunks but am putting Georg Lippitsch on CC as he did the implementation 
of BWF features within wavenc.c . I don't care if this patch is dropped, 
just stumbled over it when comparing FFmpeg BWF output to files of our 
own software and to the BWF specs.

Regards,
Tobias