Bug#843173: libfaad-dev: Implicit SBR detection via AudioSpecificConfig fails when char is signed

[Stefan Pöschel]

Hi Fabian,

Am 05.11.2016 um 20:34 schrieb Fabian Greffrath:
> Did you mean to write "fails when char is unsigned" in the subject
> line?

This was indeed a typo - sorry!

> Am Freitag, den 04.11.2016, 16:18 +0100 schrieb Stefan Pöschel:
>> One way to fix this is to make the struct variable sbr_present_flag
>> signed.
>> Another way is to enforce signed char by compiling FAAD2 with
>> -fsigned-char.
>
> I see that the fix is rather trivial, but wouldn't it introduce an ABI
> (not API) break?

To be honest: I have no knowledge in terms of ABI and which changes
affect it. But I have a strong feeling that both of my proposals break
it :-/

Am 05.11.2016 um 21:13 schrieb Fabian Greffrath:
> will it help if we explicitely cast the (-1) to char in the two
> occasions where it is used for sbr_present_flag?

This seems to be an elegant way to me.

The first occasion also works without the cast as far as I can see. Or
do you think adding the cast there as well helps to clarify the issue?
The second cast should IMO be annoted with a respective comment, to
describe the issue.

BTW:
This problem of comparing to -1 also affects the API user:
The mentioned internal function AudioSpecificConfigFromBitfile is
exposed to the public API by the function NeAACDecAudioSpecificConfig
(not mentioned in the documentation but in the include header).
Furthermore the mentioned field sbr_present_flag of the
mp4AudioSpecificConfig struct could return the problematic value -1 to
the user (in case the _presence_ of SBR is not signalled; SBR itself may
nonetheless be present or not). So the user has to compare to "(char)
-1" as well for a proper result on all systems.

This behaviour is left unchanged by your proposed fix and IMO should be
left unchanged to not break the API (though the comparison the user has
to make of course remains to be more elaborated).

Regards,
	Stefan

P.S.
An interesting blog post about the three possible types of "char":
http://blog.cdleary.com/2012/11/arm-chars-are-unsigned-by-default/