| Message ID | 20220318221907.44594-1-jamrial@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel,1/3] avformat/slndec: add support to set channel layout | expand |
| Context | Check | Description |
|---|---|---|
| yinshiyou/make_loongarch64 | success | Make finished |
| yinshiyou/make_fate_loongarch64 | success | Make fate finished |
James Almer: > Signed-off-by: James Almer <jamrial@gmail.com> > --- > libavformat/pcmdec.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/libavformat/pcmdec.c b/libavformat/pcmdec.c > index dcd13787d5..a2bcfc2814 100644 > --- a/libavformat/pcmdec.c > +++ b/libavformat/pcmdec.c > @@ -170,7 +170,8 @@ PCMDEF(vidc, "PCM Archimedes VIDC", NULL, VIDC) > #if CONFIG_SLN_DEMUXER > static const AVOption sln_options[] = { > { "sample_rate", "", offsetof(PCMAudioDemuxerContext, sample_rate), AV_OPT_TYPE_INT, {.i64 = 8000}, 0, INT_MAX, AV_OPT_FLAG_DECODING_PARAM }, > - { "channels", "", offsetof(PCMAudioDemuxerContext, channels), AV_OPT_TYPE_INT, {.i64 = 1}, 0, INT_MAX, AV_OPT_FLAG_DECODING_PARAM }, > + { "channels", "", offsetof(PCMAudioDemuxerContext, channels), AV_OPT_TYPE_INT, {.i64 = 0}, 0, INT_MAX, AV_OPT_FLAG_DECODING_PARAM }, > + { "ch_layout", "", offsetof(PCMAudioDemuxerContext, ch_layout), AV_OPT_TYPE_CHLAYOUT, {.str = "mono"}, 0, 0, AV_OPT_FLAG_DECODING_PARAM }, > { NULL }, > }; > There is no avformat/slndec. And I presume it is possible to set an unspecified channel layout via AV_OPT_TYPE_CHLAYOUT, so why don't you deprecate the now redundant channels option? - Andreas
On 3/18/2022 7:41 PM, Andreas Rheinhardt wrote: > James Almer: >> Signed-off-by: James Almer <jamrial@gmail.com> >> --- >> libavformat/pcmdec.c | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/libavformat/pcmdec.c b/libavformat/pcmdec.c >> index dcd13787d5..a2bcfc2814 100644 >> --- a/libavformat/pcmdec.c >> +++ b/libavformat/pcmdec.c >> @@ -170,7 +170,8 @@ PCMDEF(vidc, "PCM Archimedes VIDC", NULL, VIDC) >> #if CONFIG_SLN_DEMUXER >> static const AVOption sln_options[] = { >> { "sample_rate", "", offsetof(PCMAudioDemuxerContext, sample_rate), AV_OPT_TYPE_INT, {.i64 = 8000}, 0, INT_MAX, AV_OPT_FLAG_DECODING_PARAM }, >> - { "channels", "", offsetof(PCMAudioDemuxerContext, channels), AV_OPT_TYPE_INT, {.i64 = 1}, 0, INT_MAX, AV_OPT_FLAG_DECODING_PARAM }, >> + { "channels", "", offsetof(PCMAudioDemuxerContext, channels), AV_OPT_TYPE_INT, {.i64 = 0}, 0, INT_MAX, AV_OPT_FLAG_DECODING_PARAM }, >> + { "ch_layout", "", offsetof(PCMAudioDemuxerContext, ch_layout), AV_OPT_TYPE_CHLAYOUT, {.str = "mono"}, 0, 0, AV_OPT_FLAG_DECODING_PARAM }, >> { NULL }, >> }; >> > > There is no avformat/slndec. And I presume it is possible to set an > unspecified channel layout via AV_OPT_TYPE_CHLAYOUT, so why don't you > deprecate the now redundant channels option? There is no slndec, but it's the sln demuxer, so it seemed better than to write pcmdec. I can change it if you want. And yes, I was going to deprecate the channels option, but in a separate patch(set). > > - Andreas > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
James Almer: > On 3/18/2022 7:41 PM, Andreas Rheinhardt wrote: >> James Almer: >>> Signed-off-by: James Almer <jamrial@gmail.com> >>> --- >>> libavformat/pcmdec.c | 3 ++- >>> 1 file changed, 2 insertions(+), 1 deletion(-) >>> >>> diff --git a/libavformat/pcmdec.c b/libavformat/pcmdec.c >>> index dcd13787d5..a2bcfc2814 100644 >>> --- a/libavformat/pcmdec.c >>> +++ b/libavformat/pcmdec.c >>> @@ -170,7 +170,8 @@ PCMDEF(vidc, "PCM Archimedes >>> VIDC", NULL, VIDC) >>> #if CONFIG_SLN_DEMUXER >>> static const AVOption sln_options[] = { >>> { "sample_rate", "", offsetof(PCMAudioDemuxerContext, >>> sample_rate), AV_OPT_TYPE_INT, {.i64 = 8000}, 0, INT_MAX, >>> AV_OPT_FLAG_DECODING_PARAM }, >>> - { "channels", "", offsetof(PCMAudioDemuxerContext, >>> channels), AV_OPT_TYPE_INT, {.i64 = 1}, 0, INT_MAX, >>> AV_OPT_FLAG_DECODING_PARAM }, >>> + { "channels", "", offsetof(PCMAudioDemuxerContext, >>> channels), AV_OPT_TYPE_INT, {.i64 = 0}, 0, INT_MAX, >>> AV_OPT_FLAG_DECODING_PARAM }, >>> + { "ch_layout", "", offsetof(PCMAudioDemuxerContext, >>> ch_layout), AV_OPT_TYPE_CHLAYOUT, {.str = "mono"}, 0, 0, >>> AV_OPT_FLAG_DECODING_PARAM }, >>> { NULL }, >>> }; >>> >> >> There is no avformat/slndec. And I presume it is possible to set an >> unspecified channel layout via AV_OPT_TYPE_CHLAYOUT, so why don't you >> deprecate the now redundant channels option? > > There is no slndec, but it's the sln demuxer, so it seemed better than > to write pcmdec. I can change it if you want. > The naming convention contains the (main) file that is changed; the fact that it is about the sln demuxer can (and should) be mentioned in the commit title afterwards like: "avformat/pcmdec: Add support for channel layouts for sln" - Andreas
Quoting James Almer (2022-03-18 23:19:05) > Signed-off-by: James Almer <jamrial@gmail.com> > --- > libavformat/pcmdec.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/libavformat/pcmdec.c b/libavformat/pcmdec.c > index dcd13787d5..a2bcfc2814 100644 > --- a/libavformat/pcmdec.c > +++ b/libavformat/pcmdec.c > @@ -170,7 +170,8 @@ PCMDEF(vidc, "PCM Archimedes VIDC", NULL, VIDC) > #if CONFIG_SLN_DEMUXER > static const AVOption sln_options[] = { > { "sample_rate", "", offsetof(PCMAudioDemuxerContext, sample_rate), AV_OPT_TYPE_INT, {.i64 = 8000}, 0, INT_MAX, AV_OPT_FLAG_DECODING_PARAM }, > - { "channels", "", offsetof(PCMAudioDemuxerContext, channels), AV_OPT_TYPE_INT, {.i64 = 1}, 0, INT_MAX, AV_OPT_FLAG_DECODING_PARAM }, > + { "channels", "", offsetof(PCMAudioDemuxerContext, channels), AV_OPT_TYPE_INT, {.i64 = 0}, 0, INT_MAX, AV_OPT_FLAG_DECODING_PARAM }, > + { "ch_layout", "", offsetof(PCMAudioDemuxerContext, ch_layout), AV_OPT_TYPE_CHLAYOUT, {.str = "mono"}, 0, 0, AV_OPT_FLAG_DECODING_PARAM }, > { NULL }, > }; > > -- > 2.35.1 For what purpose? The demuxer doesn't do anything with the value.
On 3/19/2022 10:52 AM, Anton Khirnov wrote: > Quoting James Almer (2022-03-18 23:19:05) >> Signed-off-by: James Almer <jamrial@gmail.com> >> --- >> libavformat/pcmdec.c | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/libavformat/pcmdec.c b/libavformat/pcmdec.c >> index dcd13787d5..a2bcfc2814 100644 >> --- a/libavformat/pcmdec.c >> +++ b/libavformat/pcmdec.c >> @@ -170,7 +170,8 @@ PCMDEF(vidc, "PCM Archimedes VIDC", NULL, VIDC) >> #if CONFIG_SLN_DEMUXER >> static const AVOption sln_options[] = { >> { "sample_rate", "", offsetof(PCMAudioDemuxerContext, sample_rate), AV_OPT_TYPE_INT, {.i64 = 8000}, 0, INT_MAX, AV_OPT_FLAG_DECODING_PARAM }, >> - { "channels", "", offsetof(PCMAudioDemuxerContext, channels), AV_OPT_TYPE_INT, {.i64 = 1}, 0, INT_MAX, AV_OPT_FLAG_DECODING_PARAM }, >> + { "channels", "", offsetof(PCMAudioDemuxerContext, channels), AV_OPT_TYPE_INT, {.i64 = 0}, 0, INT_MAX, AV_OPT_FLAG_DECODING_PARAM }, >> + { "ch_layout", "", offsetof(PCMAudioDemuxerContext, ch_layout), AV_OPT_TYPE_CHLAYOUT, {.str = "mono"}, 0, 0, AV_OPT_FLAG_DECODING_PARAM }, >> { NULL }, >> }; >> >> -- >> 2.35.1 > > For what purpose? The demuxer doesn't do anything with the value. The demuxer calls pcm_read_header() as init, which uses it.
diff --git a/libavformat/pcmdec.c b/libavformat/pcmdec.c index dcd13787d5..a2bcfc2814 100644 --- a/libavformat/pcmdec.c +++ b/libavformat/pcmdec.c @@ -170,7 +170,8 @@ PCMDEF(vidc, "PCM Archimedes VIDC", NULL, VIDC) #if CONFIG_SLN_DEMUXER static const AVOption sln_options[] = { { "sample_rate", "", offsetof(PCMAudioDemuxerContext, sample_rate), AV_OPT_TYPE_INT, {.i64 = 8000}, 0, INT_MAX, AV_OPT_FLAG_DECODING_PARAM }, - { "channels", "", offsetof(PCMAudioDemuxerContext, channels), AV_OPT_TYPE_INT, {.i64 = 1}, 0, INT_MAX, AV_OPT_FLAG_DECODING_PARAM }, + { "channels", "", offsetof(PCMAudioDemuxerContext, channels), AV_OPT_TYPE_INT, {.i64 = 0}, 0, INT_MAX, AV_OPT_FLAG_DECODING_PARAM }, + { "ch_layout", "", offsetof(PCMAudioDemuxerContext, ch_layout), AV_OPT_TYPE_CHLAYOUT, {.str = "mono"}, 0, 0, AV_OPT_FLAG_DECODING_PARAM }, { NULL }, };
Signed-off-by: James Almer <jamrial@gmail.com> --- libavformat/pcmdec.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)