Re: [FFmpeg-devel] [PATCH] fix bug in af_pan channel coefficient parser

2017-11-22 Thread Tobias Rapp

On 21.11.2017 20:01, Michael Niedermayer wrote:

On Tue, Nov 21, 2017 at 02:24:23PM +0100, Tobias Rapp wrote:

On 20.11.2017 22:15, Michael Niedermayer wrote:

On Mon, Nov 20, 2017 at 05:14:15PM +0100, Nicolas George wrote:

Tobias Rapp (2017-11-20):

Would it be OK to backport the fix into release/3.4? I can do the
cherry-picking but am unsure about the policies and what other older
releases should possibly be adapted, too.


I do not know how backporting to releases work either, but I have of
course no objection in principle.


simple cherry pick with the hex hash in the commit message (-x)
test if it still all works and push


Backported the fix to release/3.4

Not sure what other releases are still maintained as the list within
the MAINTAINERS file looks outdated (2.5 to 2.8).


The ones we list on the download page should be updated if they are
affected


Which would currently mean release branches 3.4 to 2.8. From testing it 
seems that releases 3.2 and earlier do not support a pan filter string 
like "stereo|c0=c0-c2|c1=c1-c2", error message is 'Syntax error near 
"-c2"'. So backported the fix to release/3.3 additionally only.


Regards,
Tobias

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


Re: [FFmpeg-devel] [PATCH] fix bug in af_pan channel coefficient parser

2017-11-21 Thread Michael Niedermayer
On Tue, Nov 21, 2017 at 02:24:23PM +0100, Tobias Rapp wrote:
> On 20.11.2017 22:15, Michael Niedermayer wrote:
> >On Mon, Nov 20, 2017 at 05:14:15PM +0100, Nicolas George wrote:
> >>Tobias Rapp (2017-11-20):
> >>>Would it be OK to backport the fix into release/3.4? I can do the
> >>>cherry-picking but am unsure about the policies and what other older
> >>>releases should possibly be adapted, too.
> >>
> >>I do not know how backporting to releases work either, but I have of
> >>course no objection in principle.
> >
> >simple cherry pick with the hex hash in the commit message (-x)
> >test if it still all works and push
> 
> Backported the fix to release/3.4
> 
> Not sure what other releases are still maintained as the list within
> the MAINTAINERS file looks outdated (2.5 to 2.8).

The ones we list on the download page should be updated if they are
affected


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

Into a blind darkness they enter who follow after the Ignorance,
they as if into a greater darkness enter who devote themselves
to the Knowledge alone. -- Isha Upanishad


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


Re: [FFmpeg-devel] [PATCH] fix bug in af_pan channel coefficient parser

2017-11-21 Thread Tobias Rapp

On 20.11.2017 22:15, Michael Niedermayer wrote:

On Mon, Nov 20, 2017 at 05:14:15PM +0100, Nicolas George wrote:

Tobias Rapp (2017-11-20):

Would it be OK to backport the fix into release/3.4? I can do the
cherry-picking but am unsure about the policies and what other older
releases should possibly be adapted, too.


I do not know how backporting to releases work either, but I have of
course no objection in principle.


simple cherry pick with the hex hash in the commit message (-x)
test if it still all works and push


Backported the fix to release/3.4

Not sure what other releases are still maintained as the list within the 
MAINTAINERS file looks outdated (2.5 to 2.8).


Regards,
Tobias

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


Re: [FFmpeg-devel] [PATCH] fix bug in af_pan channel coefficient parser

2017-11-20 Thread Michael Niedermayer
On Mon, Nov 20, 2017 at 05:14:15PM +0100, Nicolas George wrote:
> Tobias Rapp (2017-11-20):
> > Would it be OK to backport the fix into release/3.4? I can do the
> > cherry-picking but am unsure about the policies and what other older
> > releases should possibly be adapted, too.
> 
> I do not know how backporting to releases work either, but I have of
> course no objection in principle.

simple cherry pick with the hex hash in the commit message (-x)
test if it still all works and push

[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Dictatorship: All citizens are under surveillance, all their steps and
actions recorded, for the politicians to enforce control.
Democracy: All politicians are under surveillance, all their steps and
actions recorded, for the citizens to enforce control.


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


Re: [FFmpeg-devel] [PATCH] fix bug in af_pan channel coefficient parser

2017-11-20 Thread Nicolas George
Tobias Rapp (2017-11-20):
> Would it be OK to backport the fix into release/3.4? I can do the
> cherry-picking but am unsure about the policies and what other older
> releases should possibly be adapted, too.

I do not know how backporting to releases work either, but I have of
course no objection in principle.

Regards,

-- 
  Nicolas George


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


Re: [FFmpeg-devel] [PATCH] fix bug in af_pan channel coefficient parser

2017-11-19 Thread Tobias Rapp

On 19.11.2017 17:39, Nicolas George wrote:

L'octidi 28 brumaire, an CCXXVI, Michael Roitzsch a écrit :

Hi FFmpeg team,

I was using af_pan to subtract one audio file from another. I first used an 
amerge filter and then configured the pan filter with a channel formula like 
c0=c0-c2|c1=c1-c3.

However, this does not work as expected. In debug output, I can see that 
channel 0 correctly uses gain coefficients 1 and -1 for c0 and c2. But channel 
1 incorrectly uses -1 and -1 for c1 and c3.

I looked into the code and found that the sign handling is currently wrong. 
When the last contribution of a formula is subtracted, the sign variable will 
be correctly set to -1 in line 191. But when then leaving the gains loop in 
line 189, because we reached the end of the formula for an output channel, sign 
is not reset. We therefore re-enter the gains loop for the next output channel 
with sign still -1. The first gain in this new formula will therefore be 
negated.

The attached patch fixes the problem for me.


Thanks, applied.


Would it be OK to backport the fix into release/3.4? I can do the 
cherry-picking but am unsure about the policies and what other older 
releases should possibly be adapted, too.


Regards,
Tobias

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


Re: [FFmpeg-devel] [PATCH] fix bug in af_pan channel coefficient parser

2017-11-19 Thread Nicolas George
L'octidi 28 brumaire, an CCXXVI, Michael Roitzsch a écrit :
> Hi FFmpeg team,
> 
> I was using af_pan to subtract one audio file from another. I first used an 
> amerge filter and then configured the pan filter with a channel formula like 
> c0=c0-c2|c1=c1-c3.
> 
> However, this does not work as expected. In debug output, I can see that 
> channel 0 correctly uses gain coefficients 1 and -1 for c0 and c2. But 
> channel 1 incorrectly uses -1 and -1 for c1 and c3.
> 
> I looked into the code and found that the sign handling is currently wrong. 
> When the last contribution of a formula is subtracted, the sign variable will 
> be correctly set to -1 in line 191. But when then leaving the gains loop in 
> line 189, because we reached the end of the formula for an output channel, 
> sign is not reset. We therefore re-enter the gains loop for the next output 
> channel with sign still -1. The first gain in this new formula will therefore 
> be negated.
> 
> The attached patch fixes the problem for me.

Thanks, applied.

Regards,

-- 
  Nicolas George


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


[FFmpeg-devel] [PATCH] fix bug in af_pan channel coefficient parser

2017-11-18 Thread Michael Roitzsch
Hi FFmpeg team,

I was using af_pan to subtract one audio file from another. I first used an 
amerge filter and then configured the pan filter with a channel formula like 
c0=c0-c2|c1=c1-c3.

However, this does not work as expected. In debug output, I can see that 
channel 0 correctly uses gain coefficients 1 and -1 for c0 and c2. But channel 
1 incorrectly uses -1 and -1 for c1 and c3.

I looked into the code and found that the sign handling is currently wrong. 
When the last contribution of a formula is subtracted, the sign variable will 
be correctly set to -1 in line 191. But when then leaving the gains loop in 
line 189, because we reached the end of the formula for an output channel, sign 
is not reset. We therefore re-enter the gains loop for the next output channel 
with sign still -1. The first gain in this new formula will therefore be 
negated.

The attached patch fixes the problem for me.

Regards,
Michael



0001-fix-sign-handling-in-channel-coefficient-parser.patch
Description: Binary data
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel