On 28 June 2010 01:51, Martin Storsjö <martin@martin.st> wrote:
On Mon, 28 Jun 2010, Josh Allmann wrote:
Xiph and AMR cleanups attached, but I'd appreciate it if someone could test the AMR and Vorbis audio stuff for me.
The AMR cleanup isn't totally right. Btw - the sample_50kbit.3gp sample in DSS has AMR, you can test with that.
Seems OK.
I have no Vorbis testing setup at the moment, but isn't that quite easy to set up with feng?
Before/after CRCs are the same, so I suppose it works.
These variables, octet_align, crc, interleaving, channels, were local to the amr_parse_sdp_line function earlier - now they're local to amr_parse_fmtp. This means that they're reset to the default values after each invocation of amr_parse_fmtp, and we're unable to verify that the complete fmtp line had a valid configuration.
In practice, this would trigger a warning after parsing any other configuration item than octet-align, since octet_align would be left at 0 when parsing those other configuration items. The sample only has octet-align and no other configuration items, though, so you wouldn't hit this issue with that.
So those variables should be moved to the payload context in this case - even if they're not used by the packet parsing routine (yet at least).
10l to me. Fixed.
+ if (av_strstart(line, "fmtp:", &p)) { + return ff_parse_fmtp(s->streams[st_index], data, p, + &amr_parse_fmtp); }
Btw, the common coding style is to omit & for function pointers, I think.
Corrected in the Xiph and AMR patches. Josh