Re: [FFmpeg-devel] [PATCH v2] avformat/wavenc: skip writing incorrect peak-of-peaks position value

2017-10-26 Thread Tobias Rapp 

On 25.10.2017 20:01, Michael Niedermayer wrote:

On Wed, Oct 25, 2017 at 07:55:52PM +0200, Michael Niedermayer wrote:

On Tue, Oct 24, 2017 at 11:47:21AM +0200, Tobias Rapp wrote:

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) for now until the
feature is implemented correctly.

Previous version reviewed-by: Peter Bubestinger 
Signed-off-by: Tobias Rapp 
---
v2:
  - added version micro bump
  - more code clean-up

  libavformat/version.h|  2 +-
  libavformat/wavenc.c | 11 +--
  tests/ref/lavf/wav_peak  |  2 +-
  tests/ref/lavf/wav_peak_only |  2 +-
  4 files changed, 4 insertions(+), 13 deletions(-)


will apply, thanks


rather LGTM, feel free to apply


Applied, thanks for review.

Regards,
Tobias

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH v2] avformat/wavenc: skip writing incorrect peak-of-peaks position value

2017-10-25 Thread Michael Niedermayer 
On Wed, Oct 25, 2017 at 07:55:52PM +0200, Michael Niedermayer wrote:
> On Tue, Oct 24, 2017 at 11:47:21AM +0200, Tobias Rapp wrote:
> > 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) for now until the
> > feature is implemented correctly.
> > 
> > Previous version reviewed-by: Peter Bubestinger 
> > Signed-off-by: Tobias Rapp 
> > ---
> > v2:
> >  - added version micro bump
> >  - more code clean-up
> > 
> >  libavformat/version.h|  2 +-
> >  libavformat/wavenc.c | 11 +--
> >  tests/ref/lavf/wav_peak  |  2 +-
> >  tests/ref/lavf/wav_peak_only |  2 +-
> >  4 files changed, 4 insertions(+), 13 deletions(-)
> 
> will apply, thanks

rather LGTM, feel free to apply

thx

[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

I am the wisest man alive, for I know one thing, and that is that I know
nothing. -- Socrates


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH v2] avformat/wavenc: skip writing incorrect peak-of-peaks position value

2017-10-25 Thread Michael Niedermayer 
On Tue, Oct 24, 2017 at 11:47:21AM +0200, Tobias Rapp wrote:
> 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) for now until the
> feature is implemented correctly.
> 
> Previous version reviewed-by: Peter Bubestinger 
> Signed-off-by: Tobias Rapp 
> ---
> v2:
>  - added version micro bump
>  - more code clean-up
> 
>  libavformat/version.h|  2 +-
>  libavformat/wavenc.c | 11 +--
>  tests/ref/lavf/wav_peak  |  2 +-
>  tests/ref/lavf/wav_peak_only |  2 +-
>  4 files changed, 4 insertions(+), 13 deletions(-)

will apply, thanks

[...]
-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

No snowflake in an avalanche ever feels responsible. -- Voltaire


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [PATCH v2] avformat/wavenc: skip writing incorrect peak-of-peaks position value

2017-10-24 Thread Tobias Rapp 
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) for now until the
feature is implemented correctly.

Previous version reviewed-by: Peter Bubestinger 
Signed-off-by: Tobias Rapp 
---
v2:
 - added version micro bump
 - more code clean-up

 libavformat/version.h|  2 +-
 libavformat/wavenc.c | 11 +--
 tests/ref/lavf/wav_peak  |  2 +-
 tests/ref/lavf/wav_peak_only |  2 +-
 4 files changed, 4 insertions(+), 13 deletions(-)

diff --git a/libavformat/version.h b/libavformat/version.h
index 0feb788..8ae091f 100644
--- a/libavformat/version.h
+++ b/libavformat/version.h
@@ -33,7 +33,7 @@
 // Also please add any ticket numbers that you believe might be affected here
 #define LIBAVFORMAT_VERSION_MAJOR  58
 #define LIBAVFORMAT_VERSION_MINOR   0
-#define LIBAVFORMAT_VERSION_MICRO 100
+#define LIBAVFORMAT_VERSION_MICRO 101
 
 #define LIBAVFORMAT_VERSION_INT AV_VERSION_INT(LIBAVFORMAT_VERSION_MAJOR, \
LIBAVFORMAT_VERSION_MINOR, \
diff --git a/libavformat/wavenc.c b/libavformat/wavenc.c
index adb20cb..159119d 100644
--- a/libavformat/wavenc.c
+++ b/libavformat/wavenc.c
@@ -74,8 +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;
 int write_bext;
@@ -195,7 +193,6 @@ static void peak_write_frame(AVFormatContext *s)
 {
 WAVMuxContext *wav = s->priv_data;
 AVCodecParameters *par = s->streams[0]->codecpar;
-int peak_of_peaks;
 int c;
 
 if (!wav->peak_output)
@@ -213,12 +210,6 @@ static void peak_write_frame(AVFormatContext *s)
 wav->peak_maxpos[c] =
 FFMAX(wav->peak_maxpos[c], wav->peak_maxneg[c]);
 
-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 <
 wav->peak_format * wav->peak_ppv) {
 wav->peak_outbuf_size += PEAK_BUFFER_SIZE;
@@ -287,7 +278,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 
(not implemented) */
 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
-- 
2.7.4


___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel