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 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
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;
--
Seemingly Similar 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>