Michael Graczyk
2016-Apr-26 04:31 UTC
[opus] [opus-tools] [PATCH] Add channel-mapping argument to force channel mapping
This patch adds a new option "channel-mapping" to opusenc which sets the channel mapping family used by the multistream encoder. Please let me know whether adding this option is worthwhile and whether the help string is okay. I tried to keep it short but accurate. The error message for an unimplemented channel mapping is "Error cannot create encoder: request not implemented". I do not think that is descriptive enough, but I also do not know of a way to determine which channel mapping families are supported by the multistream encoder. If we hard-code valid channel mappings {0, 1, 255}, opusenc will need to be updated when channel mappings are added to Opus. With this patch, you can encode four streams independently with $ opusenc --channel-mapping 255 in.wav out.opus When channel mapping families are added to Opus in the future (ambisonics, for example), this option will make it possible to use them. --- src/opusenc.c | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/src/opusenc.c b/src/opusenc.c index 0f83194..f2c7c48 100644 --- a/src/opusenc.c +++ b/src/opusenc.c @@ -137,6 +137,8 @@ void usage(void) printf(" --framesize n Maximum frame size in milliseconds\n"); printf(" (2.5, 5, 10, 20, 40, 60, default: 20)\n"); printf(" --expect-loss Percentage packet loss to expect (default: 0)\n"); + printf(" --channel-mapping Channel Mapping Family\n"); + printf(" (default: {0, 1, 255} for {1-2, 3-8, 9+} channels)\n"); printf(" --downmix-mono Downmix to mono\n"); printf(" --downmix-stereo Downmix to stereo (if >2 channels)\n"); printf(" --max-delay n Maximum container delay in milliseconds\n"); @@ -260,6 +262,7 @@ int main(int argc, char **argv) {"complexity", required_argument, NULL, 0}, {"framesize", required_argument, NULL, 0}, {"expect-loss", required_argument, NULL, 0}, + {"channel-mapping", required_argument, NULL, 0}, {"downmix-mono",no_argument,NULL, 0}, {"downmix-stereo",no_argument,NULL, 0}, {"no-downmix",no_argument,NULL, 0}, @@ -337,6 +340,7 @@ int main(int argc, char **argv) opus_int32 coding_rate=48000; opus_int32 frame_size=960; int chan=2; + int channel_mapping=-1; int with_hard_cbr=0; int with_cvbr=0; int expect_loss=0; @@ -465,6 +469,12 @@ int main(int argc, char **argv) fprintf(stderr,"Expected loss is a percent and must be 0-100.\n"); exit(1); } + }else if(strcmp(long_options[option_index].name,"channel-mapping")==0){ + channel_mapping=atoi(optarg); + if(channel_mapping < 0 || channel_mapping > 255){ + fprintf(stderr,"Invalid channel-mapping: %s\n",optarg); + exit(1); + } }else if(strcmp(long_options[option_index].name,"comp")==0 || strcmp(long_options[option_index].name,"complexity")==0){ complexity=atoi(optarg); @@ -692,7 +702,11 @@ int main(int argc, char **argv) /*OggOpus headers*/ /*FIXME: broke forcemono*/ header.channels=chan; - header.channel_mapping=header.channels>8?255:chan>2; + if (channel_mapping >= 0) { + header.channel_mapping = channel_mapping; + } else { + header.channel_mapping = header.channels > 8 ? 255 : header.channels > 2; + } header.input_sample_rate=rate; header.gain=inopt.gain; -- Thanks, Michael Graczyk
Ulrich Windl
2016-Apr-26 06:45 UTC
[opus] Antw: [opus-tools] [PATCH] Add channel-mapping argument to force channel mapping
Hi! I haven't looked into the code yet, but the patch uses different coding conventions like "if(" and "if ("; like wise "){" and ") {". My personal taste is to have spaces after keywords, but that's just me. I'd prefer a consistent coding style. Regards, Ulrich>>> Michael Graczyk <mgraczyk at google.com> schrieb am 26.04.2016 um 06:31 inNachricht <CABcu6-gnkPHY0d8rqDie8z5OZ+WH6eb0P+k6bPit2H8i5RrS1w at mail.gmail.com>:> This patch adds a new option "channel-mapping" to opusenc which sets > the channel mapping family used by the multistream encoder. Please let > me know whether adding this option is worthwhile and whether the help > string is okay. I tried to keep it short but accurate. > > The error message for an unimplemented channel mapping is "Error > cannot create encoder: request not implemented". I do not think that > is descriptive enough, but I also do not know of a way to determine > which channel mapping families are supported by the multistream > encoder. If we hard-code valid channel mappings {0, 1, 255}, opusenc > will need to be updated when channel mappings are added to Opus. > > With this patch, you can encode four streams independently with > $ opusenc --channel-mapping 255 in.wav out.opus > > When channel mapping families are added to Opus in the future > (ambisonics, for example), this option will make it possible to use > them. > > --- > src/opusenc.c | 16 +++++++++++++++- > 1 file changed, 15 insertions(+), 1 deletion(-) > > diff --git a/src/opusenc.c b/src/opusenc.c > index 0f83194..f2c7c48 100644 > --- a/src/opusenc.c > +++ b/src/opusenc.c > @@ -137,6 +137,8 @@ void usage(void) > printf(" --framesize n Maximum frame size in milliseconds\n"); > printf(" (2.5, 5, 10, 20, 40, 60, default: 20)\n"); > printf(" --expect-loss Percentage packet loss to expect > (default: 0)\n"); > + printf(" --channel-mapping Channel Mapping Family\n"); > + printf(" (default: {0, 1, 255} for {1-2, 3-8, > 9+} channels)\n"); > printf(" --downmix-mono Downmix to mono\n"); > printf(" --downmix-stereo Downmix to stereo (if >2 channels)\n"); > printf(" --max-delay n Maximum container delay in milliseconds\n"); > @@ -260,6 +262,7 @@ int main(int argc, char **argv) > {"complexity", required_argument, NULL, 0}, > {"framesize", required_argument, NULL, 0}, > {"expect-loss", required_argument, NULL, 0}, > + {"channel-mapping", required_argument, NULL, 0}, > {"downmix-mono",no_argument,NULL, 0}, > {"downmix-stereo",no_argument,NULL, 0}, > {"no-downmix",no_argument,NULL, 0}, > @@ -337,6 +340,7 @@ int main(int argc, char **argv) > opus_int32 coding_rate=48000; > opus_int32 frame_size=960; > int chan=2; > + int channel_mapping=-1; > int with_hard_cbr=0; > int with_cvbr=0; > int expect_loss=0; > @@ -465,6 +469,12 @@ int main(int argc, char **argv) > fprintf(stderr,"Expected loss is a percent and must be > 0-100.\n"); > exit(1); > } > + }else > if(strcmp(long_options[option_index].name,"channel-mapping")==0){ > + channel_mapping=atoi(optarg); > + if(channel_mapping < 0 || channel_mapping > 255){ > + fprintf(stderr,"Invalid channel-mapping: %s\n",optarg); > + exit(1); > + } > }else if(strcmp(long_options[option_index].name,"comp")==0 || > strcmp(long_options[option_index].name,"complexity")==0){ > complexity=atoi(optarg); > @@ -692,7 +702,11 @@ int main(int argc, char **argv) > > /*OggOpus headers*/ /*FIXME: broke forcemono*/ > header.channels=chan; > - header.channel_mapping=header.channels>8?255:chan>2; > + if (channel_mapping >= 0) { > + header.channel_mapping = channel_mapping; > + } else { > + header.channel_mapping = header.channels > 8 ? 255 : header.channels > > 2; > + } > header.input_sample_rate=rate; > header.gain=inopt.gain; > > -- > > Thanks, > Michael Graczyk > _______________________________________________ > opus mailing list > opus at xiph.org > http://lists.xiph.org/mailman/listinfo/opus
Jean-Marc Valin
2016-Apr-26 06:55 UTC
[opus] Antw: [opus-tools] [PATCH] Add channel-mapping argument to force channel mapping
On 04/26/2016 02:45 AM, Ulrich Windl wrote:> I haven't looked into the code yet, but the patch uses different > coding conventions like "if(" and "if ("; like wise "){" and ") {". > My personal taste is to have spaces after keywords, but that's just > me. > > I'd prefer a consistent coding style.It's up to Greg who's maintaining opus-tools, but Opus has at least 3 different coding styles, so it's hard to be consistent with anything. Jean-Marc> Regards, Ulrich > >>>> Michael Graczyk <mgraczyk at google.com> schrieb am 26.04.2016 um >>>> 06:31 in > Nachricht > <CABcu6-gnkPHY0d8rqDie8z5OZ+WH6eb0P+k6bPit2H8i5RrS1w at mail.gmail.com>: >> >This patch adds a new option "channel-mapping" to opusenc which sets>> the channel mapping family used by the multistream encoder. Please >> let me know whether adding this option is worthwhile and whether >> the help string is okay. I tried to keep it short but accurate. >> >> The error message for an unimplemented channel mapping is "Error >> cannot create encoder: request not implemented". I do not think >> that is descriptive enough, but I also do not know of a way to >> determine which channel mapping families are supported by the >> multistream encoder. If we hard-code valid channel mappings {0, 1, >> 255}, opusenc will need to be updated when channel mappings are >> added to Opus. >> >> With this patch, you can encode four streams independently with $ >> opusenc --channel-mapping 255 in.wav out.opus >> >> When channel mapping families are added to Opus in the future >> (ambisonics, for example), this option will make it possible to >> use them. >> >> --- src/opusenc.c | 16 +++++++++++++++- 1 file changed, 15 >> insertions(+), 1 deletion(-) >> >> diff --git a/src/opusenc.c b/src/opusenc.c index 0f83194..f2c7c48 >> 100644 --- a/src/opusenc.c +++ b/src/opusenc.c @@ -137,6 +137,8 @@ >> void usage(void) printf(" --framesize n Maximum frame size in >> milliseconds\n"); printf(" (2.5, 5, 10, 20, >> 40, 60, default: 20)\n"); printf(" --expect-loss Percentage >> packet loss to expect (default: 0)\n"); + printf(" >> --channel-mapping Channel Mapping Family\n"); + printf(" >> (default: {0, 1, 255} for {1-2, 3-8, 9+} channels)\n"); printf(" >> --downmix-mono Downmix to mono\n"); printf(" --downmix-stereo >> Downmix to stereo (if >2 channels)\n"); printf(" --max-delay n >> Maximum container delay in milliseconds\n"); @@ -260,6 +262,7 @@ >> int main(int argc, char **argv) {"complexity", required_argument, >> NULL, 0}, {"framesize", required_argument, NULL, 0}, >> {"expect-loss", required_argument, NULL, 0}, + >> {"channel-mapping", required_argument, NULL, 0}, >> {"downmix-mono",no_argument,NULL, 0}, >> {"downmix-stereo",no_argument,NULL, 0}, >> {"no-downmix",no_argument,NULL, 0}, @@ -337,6 +340,7 @@ int >> main(int argc, char **argv) opus_int32 coding_rate=48000; >> opus_int32 frame_size=960; int chan=2; + >> int channel_mapping=-1; int >> with_hard_cbr=0; int with_cvbr=0; int >> expect_loss=0; @@ -465,6 +469,12 @@ int main(int argc, char >> **argv) fprintf(stderr,"Expected loss is a percent and must be >> 0-100.\n"); exit(1); } + }else >> if(strcmp(long_options[option_index].name,"channel-mapping")==0){ + >> channel_mapping=atoi(optarg); + if(channel_mapping < 0 || >> channel_mapping > 255){ + fprintf(stderr,"Invalid >> channel-mapping: %s\n",optarg); + exit(1); + } >> }else if(strcmp(long_options[option_index].name,"comp")==0 || >> strcmp(long_options[option_index].name,"complexity")==0){ >> complexity=atoi(optarg); @@ -692,7 +702,11 @@ int main(int argc, >> char **argv) >> >> /*OggOpus headers*/ /*FIXME: broke forcemono*/ >> header.channels=chan; - >> header.channel_mapping=header.channels>8?255:chan>2; + if >> (channel_mapping >= 0) { + header.channel_mapping >> channel_mapping; + } else { + header.channel_mapping >> header.channels > 8 ? 255 : header.channels > 2; + } >> header.input_sample_rate=rate; header.gain=inopt.gain; >> >> -- >> >> Thanks, Michael Graczyk >> _______________________________________________ opus mailing list >> opus at xiph.org http://lists.xiph.org/mailman/listinfo/opus > > > > _______________________________________________ opus mailing list > opus at xiph.org http://lists.xiph.org/mailman/listinfo/opus >
Michael Graczyk
2016-Apr-26 07:07 UTC
[opus] Antw: [opus-tools] [PATCH] Add channel-mapping argument to force channel mapping
Thanks Ulrich for pointing this out. I fixed the whitespace below. --- src/opusenc.c | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/src/opusenc.c b/src/opusenc.c index 0f83194..458a225 100644 --- a/src/opusenc.c +++ b/src/opusenc.c @@ -137,6 +137,8 @@ void usage(void) printf(" --framesize n Maximum frame size in milliseconds\n"); printf(" (2.5, 5, 10, 20, 40, 60, default: 20)\n"); printf(" --expect-loss Percentage packet loss to expect (default: 0)\n"); + printf(" --channel-mapping Channel Mapping Family\n"); + printf(" (default: {0, 1, 255} for {1-2, 3-8, 9+} channels)\n"); printf(" --downmix-mono Downmix to mono\n"); printf(" --downmix-stereo Downmix to stereo (if >2 channels)\n"); printf(" --max-delay n Maximum container delay in milliseconds\n"); @@ -260,6 +262,7 @@ int main(int argc, char **argv) {"complexity", required_argument, NULL, 0}, {"framesize", required_argument, NULL, 0}, {"expect-loss", required_argument, NULL, 0}, + {"channel-mapping", required_argument, NULL, 0}, {"downmix-mono",no_argument,NULL, 0}, {"downmix-stereo",no_argument,NULL, 0}, {"no-downmix",no_argument,NULL, 0}, @@ -337,6 +340,7 @@ int main(int argc, char **argv) opus_int32 coding_rate=48000; opus_int32 frame_size=960; int chan=2; + int channel_mapping=-1; int with_hard_cbr=0; int with_cvbr=0; int expect_loss=0; @@ -465,6 +469,12 @@ int main(int argc, char **argv) fprintf(stderr,"Expected loss is a percent and must be 0-100.\n"); exit(1); } + }else if(strcmp(long_options[option_index].name,"channel-mapping")==0){ + channel_mapping=atoi(optarg); + if(channel_mapping < 0 || channel_mapping > 255){ + fprintf(stderr,"Invalid channel-mapping: %s\n",optarg); + exit(1); + } }else if(strcmp(long_options[option_index].name,"comp")==0 || strcmp(long_options[option_index].name,"complexity")==0){ complexity=atoi(optarg); @@ -692,7 +702,11 @@ int main(int argc, char **argv) /*OggOpus headers*/ /*FIXME: broke forcemono*/ header.channels=chan; - header.channel_mapping=header.channels>8?255:chan>2; + if(channel_mapping>=0){ + header.channel_mapping=channel_mapping; + }else{ + header.channel_mapping=header.channels>8?255:header.channels>2; + } header.input_sample_rate=rate; header.gain=inopt.gain; --
Reasonably Related Threads
- Antw: [opus-tools] [PATCH] Add channel-mapping argument to force channel mapping
- [libnbd PATCH 0/4] copy: wrap source code at 80 characters
- [libnbd PATCH v2 0/3] copy: wrap source code at 80 characters
- [PATCH nbdkit v2 0/2] build: Replace ./nbdkit with a C program.
- [libnbd PATCH 2/4] copy: rename <purpose>_OPTION to OPT_<purpose>