On 11.08.2016 16:57, Tanu Kaskinen wrote:
> On Thu, 2016-08-11 at 16:17 +0200, Georg Chini wrote:
>> On 10.08.2016 23:41, Tanu Kaskinen wrote:
>>> On Sun, 2016-06-05 at 21:05 +0200, Georg Chini wrote:
>>>
>> Don't understand, isn't that already called from the main thread?
> Certainly not. Maybe you're confused because of the "Called from main
> thread" comment that is visible at the end of the previous patch hunk?
yes, I was.
>
>>>> @@ -611,7 +650,8 @@ static int sink_input_pop_cb(pa_sink_input
>>>> *i, size_t nbytes, pa_memchunk *chunk
>>>> u->in_pop = false;
>>>>
>>>> if (pa_memblockq_peek(u->memblockq, chunk) < 0) {
>>>> - pa_log_info("Could not peek into queue");
>>>> + if (!u->source_sink_changed)
>>>> + pa_log_info("Could not peek into queue");
>>> What's the purpose of this change?
>> This is to suppress unnecessary log messages after startup or when
>> source or sink changes. See also comment at the bottom.
> At what point does it become necessary to log the messages?
> source_sink_changed is set to false in adjust_rates(), and I don't
> think that's a relevant event in this case. The first call to
> source_output_push_cb() would seem like more relevant trigger.
In later patches I need to know in adjust_rates if adjust_rates was
already called after the source or sink changed. Originally the flag
was introduced for that reason and only abused for the logging
purpose. So adjust_rates is the correct place to set it to false.
You are right that from the perspective of logging it is not the
relevant event, but adjust_rates is called 1/3 of a second after
source or sink changed and there can't be too many relevant
underrun messages before that. If you are not OK with the
approach I need to introduce another flag for logging purposes
or else log all messages.
But as already said it may become irrelevant when the code waits
for the source to deliver data.
