Re: [PATCH 3/3] ALSA: pcm: Add timestamp type to sw_params

[Clemens Ladisch <clemens@ladisch.de>]

Pierre-Louis Bossart wrote:
> On 7/10/14, 10:42 AM, Takashi Iwai wrote:
>> Pierre-Louis Bossart wrote:
>>> On 7/10/14, 8:04 AM, Takashi Iwai wrote:
>>>> For allowing adjusting the timestamp type on the fly, add it to
>>>> sw_params.  The existing ioctl is still kept for compatibility.
>>>
>>> I have strong objections to this extension. It will result in a
>>> discontinuity in the timestamps reported in pcm_status without a clear
>>> indication of what timestamp is reported (when does this change occur?),
>>> and it's completely unclear how userspace would handle a step (positive
>>> or negative) between ntp-adjusted and non-ntp-adjusted times.
>>
>> Well, I don't understand the logic; it's app itself who changes the
>> timestamp type.  It must know when it's changed (because app is
>> changing it), and how to handle (because app chooses the timestamp
>> type).  And the current type can be queried via sw_params_*_get()
>> thing.
>>
>> Of course, as default, there is no change from the current behavior --
>> it takes CLOCK_MONOTONIC.  So, there should be no breakage by this
>> change unless you change the application side to use the new call.
>>
>>> For apps
>>> that make use of the audio_time reported with a wall clock this could
>>> lead to complete nonsense.
>>> I would have no problems if this was a fixed parameter defined once
>>> before audio streaming starts.
>>
>> You don't have to change the setup after the stream starts.  It's
>> usually set before the stream starting.  The point of using sw_params
>> is that it's independent from the hardware driver, thus it fits better
>> than hw_params.  The switching after stream isn't the purpose.  It's
>> just a gratis bonus.
>
> So we agree that the dynamic switch isn't the intended usage but we
> allow for it anyway... I guess given that we can enable/disable
> timestamps dynamically this follows the same logic, it's just weird to
> have a sw_param whose intended use is really a hw_param, something you
> set once and don't modify later.

The difference between sw_params and hw_params is that the latter are
affected by device constraints (and might have interdependencies).
That sw_params can be changed at any time is just a consequence of that.

> If we want user-space to do the right thing we should at least
> document the consequences of a dynamic switch.

Changing the clock relative to which the timestamp is reported is not
only the desired but the _only_ consequence of this parameter.  (And
changing from/to GETTIMEOFDAY was already possible.)


Regards,
Clemens