[Asterisk-code-review] audiohook.c: Difference in read/write rates caused continuou... (asterisk[11])

[Joshua Colp]

Joshua Colp has posted comments on this change.

Change subject: audiohook.c: Difference in read/write rates caused continuous buffer resets
......................................................................


Patch Set 4: Code-Review-1

(1 comment)

https://gerrit.asterisk.org/#/c/462/4/main/audiohook.c
File main/audiohook.c:

Line 767: 	 * An audiohook's internal rate is influenced by compatibility mode. If
        : 	 * compatibility is turned "on" then all audiohooks have their sample rate
        : 	 * set to that of the audiohook_list internal sample rate. If compatibility
        : 	 * is turned "off" then audiohooks are allowed to adjust their internal rate
        : 	 * if needed.
        : 	 *
        : 	 * Since writes can only adjust the rates while compatibility is "on" and
        : 	 * reads can only do it while "off" this keeps an audiohook's internal sample
        : 	 * rate from potentially being changed on each read/write if the read/write
        : 	 * sample rates differ. Swapping the sample rate on each read/write of course
        : 	 * causes problems since the factory buffers get reset each time.
        : 	 */
This just adds to my confusion. Write of what? Where? Why off/on?

Fundamentally I can't look at the code with or without this comment and know if this is right or not. That means that either the comment isn't clear enough, or the code is still not right/too convoluted.

I'd go so far as to suggest scrapping what exists now, going through the logic I stated, and implementing that. We can document that and the behavior is clear.


-- 
To view, visit https://gerrit.asterisk.org/462
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Idab4dfef068a7922c09cc631dda27bc920a6c76f
Gerrit-PatchSet: 4
Gerrit-Project: asterisk
Gerrit-Branch: 11
Gerrit-Owner: Kevin Harwell <kharwell at digium.com>
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
Gerrit-HasComments: Yes