Sorry for long time no reply. :( 2012/3/16 Tanu Kaskinen <tanuk at iki.fi>: > On Fri, 2012-02-03 at 13:10 -0500, Wang Xingchao wrote: >> for pcm sink which doesnot supports requested formats,there should be >> proper return value. > > Sorry for the delay. Some comments after a quick glance (I haven't done > a thorough review yet): > > IMO pa_sink_input_new_data_set_sink() should return an int instead of > pa_bool_t. I believe this code was written before we had the discussion > about failure reporting style. IIRC, also Arun (who wrote this code) > agreed in the end that errors should be reported with negative integers, > not with booleans. So, I'd like to have two patches: first one that > changes the return type and second that adds checks for the return > value. > Agree. So i will make some changes according to your suggestion, is that okay? > Please configure your editor to replace tabs with spaces. > > It would be nice to print an error message instead of failing silently > when pa_sink_input_new_data_set_sink() fails. > > Arun, it seems that if the sink input new data doesn't contain > req_formats, pa_sink_input_new_data_set_sink() will always succeed. Is > that so because if there are no req_formats, the stream format will be > PCM, and we have no sinks that don't support PCM? I guess we might > someday want to have sinks that don't support PCM, so the return value > of pa_sink_input_new_data_set_sink() should be always checked (like is > done in this patch) to be future-proof? That's why i wrote this patch. i met assertion error and after some check, i found it's caused by no return value check. If sink-input-data has no req_formats and no sink, pa_sink_input_new_data_set_sink() will return FALSE, data->format will finally keep NULL. That will cause pa_return_val_if_fail(data->foramt, -PA_ERR_NOTSUPPORTED). --xingchao
