Re: [FFmpeg-devel] [PATCH] avcodec/aacenc: set pce value by options pce
2018-11-02 13:20 GMT+01:00, Hendrik Leppkes : > On Fri, Nov 2, 2018 at 1:13 PM Carl Eugen Hoyos wrote: >> Isn't the main issue that FFmpeg suddenly started to assume for >> some very common layouts that they are not supported while >> they were supported without PCE before and worked fine in every >> sensible use-case? > > How can a layout be supported without PCE if its not part of the > default channel layout map? IRL, there is no difference between 5.1 back and 5.1 side. Carl Eugen ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avcodec/aacenc: set pce value by options pce
On Fri, Nov 2, 2018 at 1:13 PM Carl Eugen Hoyos wrote: > > 2018-11-02 11:13 GMT+01:00, Hendrik Leppkes : > > On Fri, Nov 2, 2018 at 10:17 AM Steven Liu wrote: > >> > >> fix ticket: 7504 > >> > >> Signed-off-by: Steven Liu > >> --- > >> libavcodec/aacenc.c | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >> diff --git a/libavcodec/aacenc.c b/libavcodec/aacenc.c > >> index 4d0abb107f..26175bdb39 100644 > >> --- a/libavcodec/aacenc.c > >> +++ b/libavcodec/aacenc.c > >> @@ -973,7 +973,7 @@ static av_cold int aac_encode_init(AVCodecContext > >> *avctx) > >> /* Channel map and unspecified bitrate guessing */ > >> s->channels = avctx->channels; > >> > >> -s->needs_pce = 1; > >> +s->needs_pce = s->options.pce; > >> for (i = 0; i < FF_ARRAY_ELEMS(aac_normal_chan_layouts); i++) { > >> if (avctx->channel_layout == aac_normal_chan_layouts[i]) { > >> s->needs_pce = s->options.pce; > > > > This doesn't seem correct. PCE should be used if you feed the > > encode a channel layout thats not supported without PCE > > Isn't the main issue that FFmpeg suddenly started to assume for > some very common layouts that they are not supported while > they were supported without PCE before and worked fine in every > sensible use-case? How can a layout be supported without PCE if its not part of the default channel layout map? Does it give you a best guess match? That seems terrible. It should explicitly encode what you give it, and not "interpet" it, thats up to the user. - Hendrik ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avcodec/aacenc: set pce value by options pce
2018-11-02 11:13 GMT+01:00, Hendrik Leppkes : > On Fri, Nov 2, 2018 at 10:17 AM Steven Liu wrote: >> >> fix ticket: 7504 >> >> Signed-off-by: Steven Liu >> --- >> libavcodec/aacenc.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/libavcodec/aacenc.c b/libavcodec/aacenc.c >> index 4d0abb107f..26175bdb39 100644 >> --- a/libavcodec/aacenc.c >> +++ b/libavcodec/aacenc.c >> @@ -973,7 +973,7 @@ static av_cold int aac_encode_init(AVCodecContext >> *avctx) >> /* Channel map and unspecified bitrate guessing */ >> s->channels = avctx->channels; >> >> -s->needs_pce = 1; >> +s->needs_pce = s->options.pce; >> for (i = 0; i < FF_ARRAY_ELEMS(aac_normal_chan_layouts); i++) { >> if (avctx->channel_layout == aac_normal_chan_layouts[i]) { >> s->needs_pce = s->options.pce; > > This doesn't seem correct. PCE should be used if you feed the > encode a channel layout thats not supported without PCE Isn't the main issue that FFmpeg suddenly started to assume for some very common layouts that they are not supported while they were supported without PCE before and worked fine in every sensible use-case? (And that apparently typical decoders do not support these "new" layouts with PCE?) Carl Eugen ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avcodec/aacenc: set pce value by options pce
On Fri, Nov 2, 2018 at 10:17 AM Steven Liu wrote: > > fix ticket: 7504 > > Signed-off-by: Steven Liu > --- > libavcodec/aacenc.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/libavcodec/aacenc.c b/libavcodec/aacenc.c > index 4d0abb107f..26175bdb39 100644 > --- a/libavcodec/aacenc.c > +++ b/libavcodec/aacenc.c > @@ -973,7 +973,7 @@ static av_cold int aac_encode_init(AVCodecContext *avctx) > /* Channel map and unspecified bitrate guessing */ > s->channels = avctx->channels; > > -s->needs_pce = 1; > +s->needs_pce = s->options.pce; > for (i = 0; i < FF_ARRAY_ELEMS(aac_normal_chan_layouts); i++) { > if (avctx->channel_layout == aac_normal_chan_layouts[i]) { > s->needs_pce = s->options.pce; This doesn't seem correct. PCE should be used if you feed the encode a channel layout thats not supported without PCE - and the loop just below your changed line will turn PCE off (or rather defer to the user option) if a layout is used that doesn't need PCE. If you just blindly turn PCE off, you'll cause endless regressions for uncommon channel layouts. - Hendrik ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avcodec/aacenc: set pce value by options pce
On Fri, Nov 2, 2018 at 11:13 AM Hendrik Leppkes wrote: > > On Fri, Nov 2, 2018 at 10:17 AM Steven Liu wrote: > > > > fix ticket: 7504 > > > > Signed-off-by: Steven Liu > > --- > > libavcodec/aacenc.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/libavcodec/aacenc.c b/libavcodec/aacenc.c > > index 4d0abb107f..26175bdb39 100644 > > --- a/libavcodec/aacenc.c > > +++ b/libavcodec/aacenc.c > > @@ -973,7 +973,7 @@ static av_cold int aac_encode_init(AVCodecContext > > *avctx) > > /* Channel map and unspecified bitrate guessing */ > > s->channels = avctx->channels; > > > > -s->needs_pce = 1; > > +s->needs_pce = s->options.pce; > > for (i = 0; i < FF_ARRAY_ELEMS(aac_normal_chan_layouts); i++) { > > if (avctx->channel_layout == aac_normal_chan_layouts[i]) { > > s->needs_pce = s->options.pce; > > This doesn't seem correct. PCE should be used if you feed the encode a > channel layout thats not supported without PCE - and the loop just > below your changed line will turn PCE off (or rather defer to the user > option) if a layout is used that doesn't need PCE. > If you just blindly turn PCE off, you'll cause endless regressions > for uncommon channel layouts. > What I forgot to say, the actual problem with HLS is that it doesn't deal properly with PCEs. The solution is not to turn off PCEs, but to actually fix the ADTS muxer to properly handle PCE. - Hendrik ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] avcodec/aacenc: set pce value by options pce
fix ticket: 7504 Signed-off-by: Steven Liu --- libavcodec/aacenc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libavcodec/aacenc.c b/libavcodec/aacenc.c index 4d0abb107f..26175bdb39 100644 --- a/libavcodec/aacenc.c +++ b/libavcodec/aacenc.c @@ -973,7 +973,7 @@ static av_cold int aac_encode_init(AVCodecContext *avctx) /* Channel map and unspecified bitrate guessing */ s->channels = avctx->channels; -s->needs_pce = 1; +s->needs_pce = s->options.pce; for (i = 0; i < FF_ARRAY_ELEMS(aac_normal_chan_layouts); i++) { if (avctx->channel_layout == aac_normal_chan_layouts[i]) { s->needs_pce = s->options.pce; -- 2.15.2 (Apple Git-101.1) ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel