| Message ID | 20160828105654.GZ5460@nb4 |
|---|---|
| State | Not Applicable |
| Headers | show |
Hi, On Sun, Aug 28, 2016 at 4:26 PM, Michael Niedermayer <michael@niedermayer.cc> wrote: > On Sun, Aug 28, 2016 at 01:34:46PM +0530, Umair Khan wrote: >> Hi, >> >> Patches attached. :) >> >> - Umair > >> Changelog | 1 + >> 1 file changed, 1 insertion(+) >> d3f30e62d803d967bd5c27dc5dfad278ce5c02e9 0001-Changelog-Add-entry-for-ALS-encoder.patch >> From 020370545a82c8c1304ec9c177a75e27e59b84e8 Mon Sep 17 00:00:00 2001 >> From: Umair Khan <omerjerk@gmail.com> >> Date: Sat, 27 Aug 2016 22:22:02 +0530 >> Subject: [PATCH 1/2] Changelog: Add entry for ALS encoder >> >> Signed-off-by: Umair Khan <omerjerk@gmail.com> >> --- >> Changelog | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/Changelog b/Changelog >> index b903e31..90c15ad 100644 >> --- a/Changelog >> +++ b/Changelog >> @@ -15,6 +15,7 @@ version <next>: >> - True Audio (TTA) muxer >> - crystalizer audio filter >> - acrusher audio filter >> +- ALS encoder >> >> >> version 3.1: >> -- >> 2.7.4 (Apple Git-66) >> > > [...] >> +static int calc_short_term_prediction(ALSEncContext *ctx, ALSBlock *block, >> + int order) >> +{ >> + ALSSpecificConfig *sconf = &ctx->sconf; >> + int i, j; >> + >> + int32_t *res_ptr = block->res_ptr; >> + int32_t *smp_ptr = block->cur_ptr; >> + >> + assert(order > 0); > > should be av_assertX (X=0/1/2) > > > [...] >> +int ff_window_init(WindowContext *wctx, enum WindowType type, int length, >> + double param) >> +{ >> + if (!length || length < -1) >> + return AVERROR(EINVAL); >> + >> + wctx->type = type; >> + wctx->length = length; >> + wctx->param = param; >> + >> + switch (type) { >> + case WINDOW_TYPE_RECTANGLE: >> + rectangle_init(wctx); >> + break; >> + case WINDOW_TYPE_WELCH: >> + WINDOW_INIT(welch) >> + break; >> + case WINDOW_TYPE_SINERECT: >> + WINDOW_INIT(sinerect) >> + break; >> + case WINDOW_TYPE_HANNRECT: >> + WINDOW_INIT(hannrect) >> + break; >> + default: >> + return AVERROR(EINVAL); >> + } >> + > >> + if (HAVE_MMX) >> + ff_window_init_mmx(wctx); > > breaks build on non x86 as the function declaration / prototype is > not there in that case What should I do with this then? I'm not too aware of how the whole code works because I didn't originally write it. So, I'll need some help here. :) > [...] >> +static int als_write_trailer(struct AVFormatContext *s) >> +{ >> + AVIOContext *pb = s->pb; >> + AlsEncContext *ctx = s->priv_data; > >> + int header_size; >> + >> + header_size = ctx->header_size; >> + >> + if (pb->seekable) { >> + /* rewrite the header */ >> + int64_t file_size = avio_tell(pb); >> + int header_size = ctx->header_size; > > there are 2 "int header_size" here > > > [,..] > > something in this patch seems to break fate > --- ./tests/ref/fate/mpeg4-als-conformance-00 2016-08-28 12:44:08.726700510 +0200 > +++ tests/data/fate/mpeg4-als-conformance-00 2016-08-28 12:46:01.142702882 +0200 > @@ -1 +1 @@ > -CRC=0x7e67db0b > +CRC=0x2f028a7d > Test mpeg4-als-conformance-00 failed. Look at tests/data/fate/mpeg4-als-conformance-00.err for details. > make: *** [fate-mpeg4-als-conformance-00] Error 1 > TEST mpeg4-als-conformance-05 > --- ./tests/ref/fate/mpeg4-als-conformance-02 2016-08-28 12:44:08.726700510 +0200 > +++ tests/data/fate/mpeg4-als-conformance-02 2016-08-28 12:46:01.166702883 +0200 > @@ -1 +1 @@ > -CRC=0x7e67db0b > +CRC=0xe1118061 > Test mpeg4-als-conformance-02 failed. Look at tests/data/fate/mpeg4-als-conformance-02.err for details. > make: *** [fate-mpeg4-als-conformance-02] Error 1 > TEST amrnb-4k75 > --- ./tests/ref/fate/mpeg4-als-conformance-03 2016-08-28 12:44:08.726700510 +0200 > +++ tests/data/fate/mpeg4-als-conformance-03 2016-08-28 12:46:01.214702884 +0200 > @@ -1 +1 @@ > -CRC=0x7e67db0b > +CRC=0xf6bddab8 > Test mpeg4-als-conformance-03 failed. Look at tests/data/fate/mpeg4-als-conformance-03.err for details. > make: *** [fate-mpeg4-als-conformance-03] Error 1 > TEST amrnb-5k15 > TEST amrnb-5k9 > --- ./tests/ref/fate/mpeg4-als-conformance-04 2016-08-28 12:44:08.726700510 +0200 > +++ tests/data/fate/mpeg4-als-conformance-04 2016-08-28 12:46:01.226702884 +0200 > @@ -1 +1 @@ > -CRC=0x7e67db0b > +CRC=0x0f585e71 > Test mpeg4-als-conformance-04 failed. Look at tests/data/fate/mpeg4-als-conformance-04.err for details. > make: *** [fate-mpeg4-als-conformance-04] Error 1 > TEST amrnb-6k7 > TEST amrnb-7k4 > --- ./tests/ref/fate/mpeg4-als-conformance-01 2016-08-28 12:44:08.726700510 +0200 > +++ tests/data/fate/mpeg4-als-conformance-01 2016-08-28 12:46:01.278702885 +0200 > @@ -1 +1 @@ > -CRC=0x7e67db0b > +CRC=0x84528af2 > TEST amrnb-10k2 > Test mpeg4-als-conformance-01 failed. Look at tests/data/fate/mpeg4-als-conformance-01.err for details. > make: *** [fate-mpeg4-als-conformance-01] Error 1 > TEST amrnb-12k2 > --- ./tests/ref/fate/mpeg4-als-conformance-05 2016-08-28 12:44:08.726700510 +0200 > +++ tests/data/fate/mpeg4-als-conformance-05 2016-08-28 12:46:01.282702885 +0200 > @@ -1 +1 @@ > -CRC=0x7e67db0b > +CRC=0xa1730577 > Test mpeg4-als-conformance-05 failed. Look at tests/data/fate/mpeg4-als-conformance-05.err for details. > make: *** [fate-mpeg4-als-conformance-05] Error 1 > - Umair
On 8/29/2016 2:17 PM, Umair Khan wrote: >>> + if (HAVE_MMX) >>> >> + ff_window_init_mmx(wctx); >> > >> > breaks build on non x86 as the function declaration / prototype is >> > not there in that case > What should I do with this then? I'm not too aware of how the whole > code works because I didn't originally write it. > So, I'll need some help here. :) Use ARCH_X86 instead of HAVE_MMX. Don't wrap the ff_window_init_mmx declaration in the header with any pre-processor check, and also rename it to ff_window_init_x86 since there's no mmx code whatsoever. This all of course after it's been ported to yasm. As i said earlier you can keep that for a latter patch and focus on the encoder without all the assembly optimization part. For that matter, shouldn't this code be added to lpc.c/h instead of a new file? Chances are you may be duplicating parts of it as well.
On Mon, Aug 29, 2016 at 10:47:59PM +0530, Umair Khan wrote: > Hi, > > On Sun, Aug 28, 2016 at 4:26 PM, Michael Niedermayer > <michael@niedermayer.cc> wrote: > > On Sun, Aug 28, 2016 at 01:34:46PM +0530, Umair Khan wrote: > >> Hi, > >> > >> Patches attached. :) > >> > >> - Umair > > > >> Changelog | 1 + > >> 1 file changed, 1 insertion(+) > >> d3f30e62d803d967bd5c27dc5dfad278ce5c02e9 0001-Changelog-Add-entry-for-ALS-encoder.patch > >> From 020370545a82c8c1304ec9c177a75e27e59b84e8 Mon Sep 17 00:00:00 2001 > >> From: Umair Khan <omerjerk@gmail.com> > >> Date: Sat, 27 Aug 2016 22:22:02 +0530 > >> Subject: [PATCH 1/2] Changelog: Add entry for ALS encoder > >> > >> Signed-off-by: Umair Khan <omerjerk@gmail.com> > >> --- > >> Changelog | 1 + > >> 1 file changed, 1 insertion(+) > >> > >> diff --git a/Changelog b/Changelog > >> index b903e31..90c15ad 100644 > >> --- a/Changelog > >> +++ b/Changelog > >> @@ -15,6 +15,7 @@ version <next>: > >> - True Audio (TTA) muxer > >> - crystalizer audio filter > >> - acrusher audio filter > >> +- ALS encoder > >> > >> > >> version 3.1: > >> -- > >> 2.7.4 (Apple Git-66) > >> > > > > [...] > >> +static int calc_short_term_prediction(ALSEncContext *ctx, ALSBlock *block, > >> + int order) > >> +{ > >> + ALSSpecificConfig *sconf = &ctx->sconf; > >> + int i, j; > >> + > >> + int32_t *res_ptr = block->res_ptr; > >> + int32_t *smp_ptr = block->cur_ptr; > >> + > >> + assert(order > 0); > > > > should be av_assertX (X=0/1/2) > > > > > > [...] > >> +int ff_window_init(WindowContext *wctx, enum WindowType type, int length, > >> + double param) > >> +{ > >> + if (!length || length < -1) > >> + return AVERROR(EINVAL); > >> + > >> + wctx->type = type; > >> + wctx->length = length; > >> + wctx->param = param; > >> + > >> + switch (type) { > >> + case WINDOW_TYPE_RECTANGLE: > >> + rectangle_init(wctx); > >> + break; > >> + case WINDOW_TYPE_WELCH: > >> + WINDOW_INIT(welch) > >> + break; > >> + case WINDOW_TYPE_SINERECT: > >> + WINDOW_INIT(sinerect) > >> + break; > >> + case WINDOW_TYPE_HANNRECT: > >> + WINDOW_INIT(hannrect) > >> + break; > >> + default: > >> + return AVERROR(EINVAL); > >> + } > >> + > > > >> + if (HAVE_MMX) > >> + ff_window_init_mmx(wctx); > > > > breaks build on non x86 as the function declaration / prototype is > > not there in that case > > What should I do with this then? I'm not too aware of how the whole > code works because I didn't originally write it. > So, I'll need some help here. :) IIRC the declaration / prototype is under #if but the call is not under #if thus if the condition on the #if is untrue this fails to build (this is not the same as the functions implementation being missing for if(0) code, that one works with all supported platforms) its probably best to split the whole *_mmx code out into a seperate patch and also either the call must be under #if or the declaration must be available independant of an #if [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Democracy is the form of government in which you can choose your dictator
Hi, Sorry for late reply. I was travelling a bit. Attached is the patch which includes the following changes than the previous one - 1. Removed changes of the file libavformat/movenc.c, as I had added this file in the initial commit by mistake. 2. Removed the assembly code. 3. Make changes as suggested by Michael (av_assertX and header_size). As far as fate tests are concerned, I haven't checked that yet. I'll see what's the issue there. On Mon, Aug 29, 2016 at 11:34 PM, Michael Niedermayer <michael@niedermayer.cc> wrote: > On Mon, Aug 29, 2016 at 10:47:59PM +0530, Umair Khan wrote: >> Hi, >> >> On Sun, Aug 28, 2016 at 4:26 PM, Michael Niedermayer >> <michael@niedermayer.cc> wrote: >> > On Sun, Aug 28, 2016 at 01:34:46PM +0530, Umair Khan wrote: >> >> Hi, >> >> >> >> Patches attached. :) >> >> >> >> - Umair >> > >> >> Changelog | 1 + >> >> 1 file changed, 1 insertion(+) >> >> d3f30e62d803d967bd5c27dc5dfad278ce5c02e9 0001-Changelog-Add-entry-for-ALS-encoder.patch >> >> From 020370545a82c8c1304ec9c177a75e27e59b84e8 Mon Sep 17 00:00:00 2001 >> >> From: Umair Khan <omerjerk@gmail.com> >> >> Date: Sat, 27 Aug 2016 22:22:02 +0530 >> >> Subject: [PATCH 1/2] Changelog: Add entry for ALS encoder >> >> >> >> Signed-off-by: Umair Khan <omerjerk@gmail.com> >> >> --- >> >> Changelog | 1 + >> >> 1 file changed, 1 insertion(+) >> >> >> >> diff --git a/Changelog b/Changelog >> >> index b903e31..90c15ad 100644 >> >> --- a/Changelog >> >> +++ b/Changelog >> >> @@ -15,6 +15,7 @@ version <next>: >> >> - True Audio (TTA) muxer >> >> - crystalizer audio filter >> >> - acrusher audio filter >> >> +- ALS encoder >> >> >> >> >> >> version 3.1: >> >> -- >> >> 2.7.4 (Apple Git-66) >> >> >> > >> > [...] >> >> +static int calc_short_term_prediction(ALSEncContext *ctx, ALSBlock *block, >> >> + int order) >> >> +{ >> >> + ALSSpecificConfig *sconf = &ctx->sconf; >> >> + int i, j; >> >> + >> >> + int32_t *res_ptr = block->res_ptr; >> >> + int32_t *smp_ptr = block->cur_ptr; >> >> + >> >> + assert(order > 0); >> > >> > should be av_assertX (X=0/1/2) >> > >> > >> > [...] >> >> +int ff_window_init(WindowContext *wctx, enum WindowType type, int length, >> >> + double param) >> >> +{ >> >> + if (!length || length < -1) >> >> + return AVERROR(EINVAL); >> >> + >> >> + wctx->type = type; >> >> + wctx->length = length; >> >> + wctx->param = param; >> >> + >> >> + switch (type) { >> >> + case WINDOW_TYPE_RECTANGLE: >> >> + rectangle_init(wctx); >> >> + break; >> >> + case WINDOW_TYPE_WELCH: >> >> + WINDOW_INIT(welch) >> >> + break; >> >> + case WINDOW_TYPE_SINERECT: >> >> + WINDOW_INIT(sinerect) >> >> + break; >> >> + case WINDOW_TYPE_HANNRECT: >> >> + WINDOW_INIT(hannrect) >> >> + break; >> >> + default: >> >> + return AVERROR(EINVAL); >> >> + } >> >> + >> > >> >> + if (HAVE_MMX) >> >> + ff_window_init_mmx(wctx); >> > >> > breaks build on non x86 as the function declaration / prototype is >> > not there in that case >> >> What should I do with this then? I'm not too aware of how the whole >> code works because I didn't originally write it. >> So, I'll need some help here. :) > > IIRC the declaration / prototype is under #if > but the call is not under #if > thus if the condition on the #if is untrue this fails to build > > (this is not the same as the functions implementation being missing > for if(0) code, that one works with all supported platforms) > > its probably best to split the whole *_mmx code out into a seperate > patch and also either the call must be under #if or the declaration > must be available independant of an #if > > > [...] > -- > Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB > > Democracy is the form of government in which you can choose your dictator > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel >
Hi, Patch attached. It fixes the fate tests. However, there's a slight bug in the encoder in handling the last frame. I'll definitely fix it later. I hope the patch can be merged in this state. - Umair On Mon, Sep 12, 2016 at 12:53 PM, Umair Khan <omerjerk@gmail.com> wrote: > Hi, > > Sorry for late reply. I was travelling a bit. > Attached is the patch which includes the following changes than the > previous one - > > 1. Removed changes of the file libavformat/movenc.c, as I had added > this file in the initial commit by mistake. > 2. Removed the assembly code. > 3. Make changes as suggested by Michael (av_assertX and header_size). > > As far as fate tests are concerned, I haven't checked that yet. I'll > see what's the issue there. > > On Mon, Aug 29, 2016 at 11:34 PM, Michael Niedermayer > <michael@niedermayer.cc> wrote: > > On Mon, Aug 29, 2016 at 10:47:59PM +0530, Umair Khan wrote: > >> Hi, > >> > >> On Sun, Aug 28, 2016 at 4:26 PM, Michael Niedermayer > >> <michael@niedermayer.cc> wrote: > >> > On Sun, Aug 28, 2016 at 01:34:46PM +0530, Umair Khan wrote: > >> >> Hi, > >> >> > >> >> Patches attached. :) > >> >> > >> >> - Umair > >> > > >> >> Changelog | 1 + > >> >> 1 file changed, 1 insertion(+) > >> >> d3f30e62d803d967bd5c27dc5dfad278ce5c02e9 > 0001-Changelog-Add-entry-for-ALS-encoder.patch > >> >> From 020370545a82c8c1304ec9c177a75e27e59b84e8 Mon Sep 17 00:00:00 > 2001 > >> >> From: Umair Khan <omerjerk@gmail.com> > >> >> Date: Sat, 27 Aug 2016 22:22:02 +0530 > >> >> Subject: [PATCH 1/2] Changelog: Add entry for ALS encoder > >> >> > >> >> Signed-off-by: Umair Khan <omerjerk@gmail.com> > >> >> --- > >> >> Changelog | 1 + > >> >> 1 file changed, 1 insertion(+) > >> >> > >> >> diff --git a/Changelog b/Changelog > >> >> index b903e31..90c15ad 100644 > >> >> --- a/Changelog > >> >> +++ b/Changelog > >> >> @@ -15,6 +15,7 @@ version <next>: > >> >> - True Audio (TTA) muxer > >> >> - crystalizer audio filter > >> >> - acrusher audio filter > >> >> +- ALS encoder > >> >> > >> >> > >> >> version 3.1: > >> >> -- > >> >> 2.7.4 (Apple Git-66) > >> >> > >> > > >> > [...] > >> >> +static int calc_short_term_prediction(ALSEncContext *ctx, ALSBlock > *block, > >> >> + int order) > >> >> +{ > >> >> + ALSSpecificConfig *sconf = &ctx->sconf; > >> >> + int i, j; > >> >> + > >> >> + int32_t *res_ptr = block->res_ptr; > >> >> + int32_t *smp_ptr = block->cur_ptr; > >> >> + > >> >> + assert(order > 0); > >> > > >> > should be av_assertX (X=0/1/2) > >> > > >> > > >> > [...] > >> >> +int ff_window_init(WindowContext *wctx, enum WindowType type, int > length, > >> >> + double param) > >> >> +{ > >> >> + if (!length || length < -1) > >> >> + return AVERROR(EINVAL); > >> >> + > >> >> + wctx->type = type; > >> >> + wctx->length = length; > >> >> + wctx->param = param; > >> >> + > >> >> + switch (type) { > >> >> + case WINDOW_TYPE_RECTANGLE: > >> >> + rectangle_init(wctx); > >> >> + break; > >> >> + case WINDOW_TYPE_WELCH: > >> >> + WINDOW_INIT(welch) > >> >> + break; > >> >> + case WINDOW_TYPE_SINERECT: > >> >> + WINDOW_INIT(sinerect) > >> >> + break; > >> >> + case WINDOW_TYPE_HANNRECT: > >> >> + WINDOW_INIT(hannrect) > >> >> + break; > >> >> + default: > >> >> + return AVERROR(EINVAL); > >> >> + } > >> >> + > >> > > >> >> + if (HAVE_MMX) > >> >> + ff_window_init_mmx(wctx); > >> > > >> > breaks build on non x86 as the function declaration / prototype is > >> > not there in that case > >> > >> What should I do with this then? I'm not too aware of how the whole > >> code works because I didn't originally write it. > >> So, I'll need some help here. :) > > > > IIRC the declaration / prototype is under #if > > but the call is not under #if > > thus if the condition on the #if is untrue this fails to build > > > > (this is not the same as the functions implementation being missing > > for if(0) code, that one works with all supported platforms) > > > > its probably best to split the whole *_mmx code out into a seperate > > patch and also either the call must be under #if or the declaration > > must be available independant of an #if > > > > > > [...] > > -- > > Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB > > > > Democracy is the form of government in which you can choose your dictator > > > > _______________________________________________ > > ffmpeg-devel mailing list > > ffmpeg-devel@ffmpeg.org > > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > >
Hi, > Patch attached. > > It fixes the fate tests. > However, there's a slight bug in the encoder in handling the last frame. > I'll definitely fix it later. > I hope the patch can be merged in this state. no. The last frame has to be handled properly before merging happens. I'm at LinuxCon & ELCE for the next two weeks, so don't expect too much input from me before Oct. 14th. Please fix whatever is possible until then. -Thilo > - Umair > > On Mon, Sep 12, 2016 at 12:53 PM, Umair Khan <omerjerk@gmail.com> wrote: > >> Hi, >> >> Sorry for late reply. I was travelling a bit. >> Attached is the patch which includes the following changes than the >> previous one - >> >> 1. Removed changes of the file libavformat/movenc.c, as I had added >> this file in the initial commit by mistake. >> 2. Removed the assembly code. >> 3. Make changes as suggested by Michael (av_assertX and header_size). >> >> As far as fate tests are concerned, I haven't checked that yet. I'll >> see what's the issue there. >> >> On Mon, Aug 29, 2016 at 11:34 PM, Michael Niedermayer >> <michael@niedermayer.cc> wrote: >>> On Mon, Aug 29, 2016 at 10:47:59PM +0530, Umair Khan wrote: >>>> Hi, >>>> >>>> On Sun, Aug 28, 2016 at 4:26 PM, Michael Niedermayer >>>> <michael@niedermayer.cc> wrote: >>>>> On Sun, Aug 28, 2016 at 01:34:46PM +0530, Umair Khan wrote: >>>>>> Hi, >>>>>> >>>>>> Patches attached. :) >>>>>> >>>>>> - Umair >>>>> >>>>>> Changelog | 1 + >>>>>> 1 file changed, 1 insertion(+) >>>>>> d3f30e62d803d967bd5c27dc5dfad278ce5c02e9 >> 0001-Changelog-Add-entry-for-ALS-encoder.patch >>>>>> From 020370545a82c8c1304ec9c177a75e27e59b84e8 Mon Sep 17 00:00:00 >> 2001 >>>>>> From: Umair Khan <omerjerk@gmail.com> >>>>>> Date: Sat, 27 Aug 2016 22:22:02 +0530 >>>>>> Subject: [PATCH 1/2] Changelog: Add entry for ALS encoder >>>>>> >>>>>> Signed-off-by: Umair Khan <omerjerk@gmail.com> >>>>>> --- >>>>>> Changelog | 1 + >>>>>> 1 file changed, 1 insertion(+) >>>>>> >>>>>> diff --git a/Changelog b/Changelog >>>>>> index b903e31..90c15ad 100644 >>>>>> --- a/Changelog >>>>>> +++ b/Changelog >>>>>> @@ -15,6 +15,7 @@ version <next>: >>>>>> - True Audio (TTA) muxer >>>>>> - crystalizer audio filter >>>>>> - acrusher audio filter >>>>>> +- ALS encoder >>>>>> >>>>>> >>>>>> version 3.1: >>>>>> -- >>>>>> 2.7.4 (Apple Git-66) >>>>>> >>>>> >>>>> [...] >>>>>> +static int calc_short_term_prediction(ALSEncContext *ctx, ALSBlock >> *block, >>>>>> + int order) >>>>>> +{ >>>>>> + ALSSpecificConfig *sconf = &ctx->sconf; >>>>>> + int i, j; >>>>>> + >>>>>> + int32_t *res_ptr = block->res_ptr; >>>>>> + int32_t *smp_ptr = block->cur_ptr; >>>>>> + >>>>>> + assert(order > 0); >>>>> >>>>> should be av_assertX (X=0/1/2) >>>>> >>>>> >>>>> [...] >>>>>> +int ff_window_init(WindowContext *wctx, enum WindowType type, int >> length, >>>>>> + double param) >>>>>> +{ >>>>>> + if (!length || length < -1) >>>>>> + return AVERROR(EINVAL); >>>>>> + >>>>>> + wctx->type = type; >>>>>> + wctx->length = length; >>>>>> + wctx->param = param; >>>>>> + >>>>>> + switch (type) { >>>>>> + case WINDOW_TYPE_RECTANGLE: >>>>>> + rectangle_init(wctx); >>>>>> + break; >>>>>> + case WINDOW_TYPE_WELCH: >>>>>> + WINDOW_INIT(welch) >>>>>> + break; >>>>>> + case WINDOW_TYPE_SINERECT: >>>>>> + WINDOW_INIT(sinerect) >>>>>> + break; >>>>>> + case WINDOW_TYPE_HANNRECT: >>>>>> + WINDOW_INIT(hannrect) >>>>>> + break; >>>>>> + default: >>>>>> + return AVERROR(EINVAL); >>>>>> + } >>>>>> + >>>>> >>>>>> + if (HAVE_MMX) >>>>>> + ff_window_init_mmx(wctx); >>>>> >>>>> breaks build on non x86 as the function declaration / prototype is >>>>> not there in that case >>>> >>>> What should I do with this then? I'm not too aware of how the whole >>>> code works because I didn't originally write it. >>>> So, I'll need some help here. :) >>> >>> IIRC the declaration / prototype is under #if >>> but the call is not under #if >>> thus if the condition on the #if is untrue this fails to build >>> >>> (this is not the same as the functions implementation being missing >>> for if(0) code, that one works with all supported platforms) >>> >>> its probably best to split the whole *_mmx code out into a seperate >>> patch and also either the call must be under #if or the declaration >>> must be available independant of an #if >>> >>> >>> [...] >>> -- >>> Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB >>> >>> Democracy is the form of government in which you can choose your dictator >>> >>> _______________________________________________ >>> ffmpeg-devel mailing list >>> ffmpeg-devel@ffmpeg.org >>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel >>> >> >> >> >> _______________________________________________ >> ffmpeg-devel mailing list >> ffmpeg-devel@ffmpeg.org >> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Hi, On Tue, Oct 4, 2016 at 1:53 AM, Thilo Borgmann <thilo.borgmann@mail.de> wrote: > Hi, > >> Patch attached. >> >> It fixes the fate tests. >> However, there's a slight bug in the encoder in handling the last frame. >> I'll definitely fix it later. >> I hope the patch can be merged in this state. > > no. The last frame has to be handled properly before merging happens. > I'm at LinuxCon & ELCE for the next two weeks, so don't expect too much input from me before Oct. 14th. Please fix whatever is possible until then. > > -Thilo Okay!! I'm on it. I should be able to fix it. - Umair >> On Mon, Sep 12, 2016 at 12:53 PM, Umair Khan <omerjerk@gmail.com> wrote: >> >>> Hi, >>> >>> Sorry for late reply. I was travelling a bit. >>> Attached is the patch which includes the following changes than the >>> previous one - >>> >>> 1. Removed changes of the file libavformat/movenc.c, as I had added >>> this file in the initial commit by mistake. >>> 2. Removed the assembly code. >>> 3. Make changes as suggested by Michael (av_assertX and header_size). >>> >>> As far as fate tests are concerned, I haven't checked that yet. I'll >>> see what's the issue there. >>> >>> On Mon, Aug 29, 2016 at 11:34 PM, Michael Niedermayer >>> <michael@niedermayer.cc> wrote: >>>> On Mon, Aug 29, 2016 at 10:47:59PM +0530, Umair Khan wrote: >>>>> Hi, >>>>> >>>>> On Sun, Aug 28, 2016 at 4:26 PM, Michael Niedermayer >>>>> <michael@niedermayer.cc> wrote: >>>>>> On Sun, Aug 28, 2016 at 01:34:46PM +0530, Umair Khan wrote: >>>>>>> Hi, >>>>>>> >>>>>>> Patches attached. :) >>>>>>> >>>>>>> - Umair >>>>>> >>>>>>> Changelog | 1 + >>>>>>> 1 file changed, 1 insertion(+) >>>>>>> d3f30e62d803d967bd5c27dc5dfad278ce5c02e9 >>> 0001-Changelog-Add-entry-for-ALS-encoder.patch >>>>>>> From 020370545a82c8c1304ec9c177a75e27e59b84e8 Mon Sep 17 00:00:00 >>> 2001 >>>>>>> From: Umair Khan <omerjerk@gmail.com> >>>>>>> Date: Sat, 27 Aug 2016 22:22:02 +0530 >>>>>>> Subject: [PATCH 1/2] Changelog: Add entry for ALS encoder >>>>>>> >>>>>>> Signed-off-by: Umair Khan <omerjerk@gmail.com> >>>>>>> --- >>>>>>> Changelog | 1 + >>>>>>> 1 file changed, 1 insertion(+) >>>>>>> >>>>>>> diff --git a/Changelog b/Changelog >>>>>>> index b903e31..90c15ad 100644 >>>>>>> --- a/Changelog >>>>>>> +++ b/Changelog >>>>>>> @@ -15,6 +15,7 @@ version <next>: >>>>>>> - True Audio (TTA) muxer >>>>>>> - crystalizer audio filter >>>>>>> - acrusher audio filter >>>>>>> +- ALS encoder >>>>>>> >>>>>>> >>>>>>> version 3.1: >>>>>>> -- >>>>>>> 2.7.4 (Apple Git-66) >>>>>>> >>>>>> >>>>>> [...] >>>>>>> +static int calc_short_term_prediction(ALSEncContext *ctx, ALSBlock >>> *block, >>>>>>> + int order) >>>>>>> +{ >>>>>>> + ALSSpecificConfig *sconf = &ctx->sconf; >>>>>>> + int i, j; >>>>>>> + >>>>>>> + int32_t *res_ptr = block->res_ptr; >>>>>>> + int32_t *smp_ptr = block->cur_ptr; >>>>>>> + >>>>>>> + assert(order > 0); >>>>>> >>>>>> should be av_assertX (X=0/1/2) >>>>>> >>>>>> >>>>>> [...] >>>>>>> +int ff_window_init(WindowContext *wctx, enum WindowType type, int >>> length, >>>>>>> + double param) >>>>>>> +{ >>>>>>> + if (!length || length < -1) >>>>>>> + return AVERROR(EINVAL); >>>>>>> + >>>>>>> + wctx->type = type; >>>>>>> + wctx->length = length; >>>>>>> + wctx->param = param; >>>>>>> + >>>>>>> + switch (type) { >>>>>>> + case WINDOW_TYPE_RECTANGLE: >>>>>>> + rectangle_init(wctx); >>>>>>> + break; >>>>>>> + case WINDOW_TYPE_WELCH: >>>>>>> + WINDOW_INIT(welch) >>>>>>> + break; >>>>>>> + case WINDOW_TYPE_SINERECT: >>>>>>> + WINDOW_INIT(sinerect) >>>>>>> + break; >>>>>>> + case WINDOW_TYPE_HANNRECT: >>>>>>> + WINDOW_INIT(hannrect) >>>>>>> + break; >>>>>>> + default: >>>>>>> + return AVERROR(EINVAL); >>>>>>> + } >>>>>>> + >>>>>> >>>>>>> + if (HAVE_MMX) >>>>>>> + ff_window_init_mmx(wctx); >>>>>> >>>>>> breaks build on non x86 as the function declaration / prototype is >>>>>> not there in that case >>>>> >>>>> What should I do with this then? I'm not too aware of how the whole >>>>> code works because I didn't originally write it. >>>>> So, I'll need some help here. :) >>>> >>>> IIRC the declaration / prototype is under #if >>>> but the call is not under #if >>>> thus if the condition on the #if is untrue this fails to build >>>> >>>> (this is not the same as the functions implementation being missing >>>> for if(0) code, that one works with all supported platforms) >>>> >>>> its probably best to split the whole *_mmx code out into a seperate >>>> patch and also either the call must be under #if or the declaration >>>> must be available independant of an #if >>>> >>>> >>>> [...] >>>> -- >>>> Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB >>>> >>>> Democracy is the form of government in which you can choose your dictator >>>> >>>> _______________________________________________ >>>> ffmpeg-devel mailing list >>>> ffmpeg-devel@ffmpeg.org >>>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel >>>> >>> >>> >>> >>> _______________________________________________ >>> ffmpeg-devel mailing list >>> ffmpeg-devel@ffmpeg.org >>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
On Tue, Oct 04, 2016 at 12:22:38AM +0530, Umair Khan wrote: > Hi, > > Patch attached. > > It fixes the fate tests. > However, there's a slight bug in the encoder in handling the last frame. > I'll definitely fix it later. > I hope the patch can be merged in this state. [...] > +/** > + * Quantize and rescale a single PARCOR coefficient. > + * @param ctx Encoder context > + * @param parcor double-precision PARCOR coefficient > + * @param index coefficient index number > + * @param[out] q_parcor 7-bit quantized coefficient > + * @param[out] r_parcor 21-bit reconstructed coefficient > + * @return the number of bits used to encode the coefficient > + */ > +static int quantize_single_parcor_coeff(ALSEncContext *ctx, double parcor, > + int index, int32_t *q_parcor, > + int32_t *r_parcor) > +{ > + int rice_param, offset; > + int sign = !index - index; > + > + // compand coefficient for index 0 or 1 > + if (index < 2) > + parcor = sqrt(2.0 * (sign * parcor + 1.0)) - 1.0; > + > + // quantize to signed 7-bit > + *q_parcor = av_clip((int32_t)floor(64.0 * parcor), -64, 63); could use av_clip_intp2() [...] > +/** > + * Determine the number of samples in each frame, which is constant for all > + * frames in the stream except the very last one which may be smaller. > + */ > +static void frame_partitioning(ALSEncContext *ctx) > +{ > + AVCodecContext *avctx = ctx->avctx; > + ALSSpecificConfig *sconf = &ctx->sconf; > + > + // choose default frame size if not specified by the user > + if (avctx->frame_size <= 0) { > + if (avctx->sample_rate <= 24000) > + avctx->frame_size = 1024; > + else if(avctx->sample_rate <= 48000) > + avctx->frame_size = 2048; > + else if(avctx->sample_rate <= 96000) > + avctx->frame_size = 4096; > + else > + avctx->frame_size = 8192; > + > + // increase frame size if block switching is used > + if (sconf->block_switching) > + avctx->frame_size <<= sconf->block_switching >> 1; > + } > + > + // ensure a certain boundary for the frame size > + // frame length - 1 in ALSSpecificConfig is 16-bit, so max value is 65536 > + // frame size == 1 is not allowed because it is used in ffmpeg as a > + // special-case value to indicate PCM audio > + avctx->frame_size = av_clip(avctx->frame_size, 2, 65536); > + sconf->frame_length = avctx->frame_size; > + > + // determine distance between ra-frames. 0 = no ra, 1 = all ra > + // defaults to 10s intervals for random access > + sconf->ra_distance = avctx->gop_size; > + /* There is an API issue where the required output audio buffer size cannot > + be known to the user, and the default buffer size in ffmpeg.c is too > + small to consistently fit more than about 7 frames. Once this issue > + is resolved, the maximum value can be changed from 7 to 255. */ > + sconf->ra_distance = av_clip(sconf->ra_distance, 0, 7); av_clip_uintp2() [...]
--- ./tests/ref/fate/mpeg4-als-conformance-00 2016-08-28 12:44:08.726700510 +0200 +++ tests/data/fate/mpeg4-als-conformance-00 2016-08-28 12:46:01.142702882 +0200 @@ -1 +1 @@ -CRC=0x7e67db0b +CRC=0x2f028a7d Test mpeg4-als-conformance-00 failed. Look at tests/data/fate/mpeg4-als-conformance-00.err for details. make: *** [fate-mpeg4-als-conformance-00] Error 1 TEST mpeg4-als-conformance-05 --- ./tests/ref/fate/mpeg4-als-conformance-02 2016-08-28 12:44:08.726700510 +0200 +++ tests/data/fate/mpeg4-als-conformance-02 2016-08-28 12:46:01.166702883 +0200 @@ -1 +1 @@ -CRC=0x7e67db0b +CRC=0xe1118061 Test mpeg4-als-conformance-02 failed. Look at tests/data/fate/mpeg4-als-conformance-02.err for details. make: *** [fate-mpeg4-als-conformance-02] Error 1 TEST amrnb-4k75 --- ./tests/ref/fate/mpeg4-als-conformance-03 2016-08-28 12:44:08.726700510 +0200 +++ tests/data/fate/mpeg4-als-conformance-03 2016-08-28 12:46:01.214702884 +0200 @@ -1 +1 @@ -CRC=0x7e67db0b +CRC=0xf6bddab8 Test mpeg4-als-conformance-03 failed. Look at tests/data/fate/mpeg4-als-conformance-03.err for details. make: *** [fate-mpeg4-als-conformance-03] Error 1 TEST amrnb-5k15 TEST amrnb-5k9 --- ./tests/ref/fate/mpeg4-als-conformance-04 2016-08-28 12:44:08.726700510 +0200 +++ tests/data/fate/mpeg4-als-conformance-04 2016-08-28 12:46:01.226702884 +0200 @@ -1 +1 @@ -CRC=0x7e67db0b +CRC=0x0f585e71 Test mpeg4-als-conformance-04 failed. Look at tests/data/fate/mpeg4-als-conformance-04.err for details. make: *** [fate-mpeg4-als-conformance-04] Error 1 TEST amrnb-6k7 TEST amrnb-7k4 --- ./tests/ref/fate/mpeg4-als-conformance-01 2016-08-28 12:44:08.726700510 +0200 +++ tests/data/fate/mpeg4-als-conformance-01 2016-08-28 12:46:01.278702885 +0200 @@ -1 +1 @@ -CRC=0x7e67db0b +CRC=0x84528af2 TEST amrnb-10k2 Test mpeg4-als-conformance-01 failed. Look at tests/data/fate/mpeg4-als-conformance-01.err for details. make: *** [fate-mpeg4-als-conformance-01] Error 1 TEST amrnb-12k2 --- ./tests/ref/fate/mpeg4-als-conformance-05 2016-08-28 12:44:08.726700510 +0200 +++ tests/data/fate/mpeg4-als-conformance-05 2016-08-28 12:46:01.282702885 +0200 @@ -1 +1 @@ -CRC=0x7e67db0b +CRC=0xa1730577