reassign 772877 libvorbisenc2
retitle 772877 libvorbis: Segmentation fault when disabling channel coupling
thanks
Actually the oggenc code about setting the "disable_coupling" option
seems to be fine, but the relevant libvorbis encoding code not so much.
So I'm reassigning this to libvorbisenc2.
The code can be reproduced without oggenc in a simple example by calling
"vorbis_encode_ctl" with action "OV_ECTL_COUPLING_SET" and argument
being a pointer to an int variable with value 0.
I've found the following two problems with the encoding library.
First of all, the segmentation fault seems to occur due to poor return
code checking:
/
| new_template = get_setup_template(.....);
| if(!hi->setup)return OV_EIMPL;
| hi->setup=new_template;
| hi->base_setting=new_base;
| vorbis_encode_setup_setting(vi,vi->channels,vi->rate);
\
If "get_setup_template" returns NULL, which it is allowed to in general,
and which it does in this specific scenario, then "hi->setup" will be
NULL - which "vorbis_encode_setup_setting" cannot cope with. The NULL
check should either come *after* assigning the new value to "hi->setup"
(that's the way it is done in "vorbis_encode_setup_vbr" and
"vorbis_encode_setup_managed"), or the NULL check should refer to
"new_template".
The second question is, why "get_setup_template" actually returns NULL.
In this case, it should be able to find a matching setup template. The
answer is in the following line at the beginning of "get_setup_template":
/
| if(q_or_bitrate)req/=ch;
\
This line is supposed to divide the total desired bitrate up into
bitrates for each channel. However, in the specific case of disabling
the coupling, the parameter "ch" is set to -1. The desired effect of
this is to accept only setups that don't couple channels, which is
achieved. However, as a side effect this also screws up the mentioned
bitrate division. The bitrate is then divided by "-1" and we end up with
a negative desired bitrate, which can never be fulfilled. Fixing this
requires a signature change of "get_setup_template", otherwise the real
number of channels cannot be known in this case.
Since the function "get_setup_template" doesn't appear in any header
file at all, this signature change doesn't affect the ABI nor API and is
therefore safe.
I will provide a patch, when I find the time to do a thorough testing of
the fix.
Cheers,
Martin
