Renaming Source/Video:seek/tell

Issue #642 open
hahawoo
created an issue

There are few issues with seek and tell I think.

They set and get a property of the Source, rather than do an "action" on it, so I'd expect a "set/get" prefix. Also, even though the names are good for what they are, they have an obvious setter/getter relationship, and the name "tell" isn't super obvious, at least to me.

How about set/getLocation? I know it's kinda similar to set/getPosition though, but here's hoping that wouldn't be too confusing.

Comments (30)

  1. Bart van Strien

    These are well-known names, consistent with Files, and used in just about every other piece of software. (See fseek, ftell.) I'm not sure this is more obvious, and it feels a bit like being different for the sake of being different.

  2. hahawoo reporter
    • changed status to open

    All else being equal it would nice if Sources and Files had the same names, but I think the consistency between Sources and Files isn't as important as the consistency between Sources and LÖVE's naming convention for functions which set/get state.

    I took a look at some other library-framework-engines to see what names they were using (accuracy not guaranteed).

    • SFML: get/setPlayingOffset
    • Allegro: al_get/set_voice_position
    • Slick2D: get/setPosition
    • FlashPunk: position property
    • Polycode: get/setOffset (samples), getPlaybackTime (time), seekTo (time)
    • openFrameworks: get/setPositionMS, get/setPosition (normalised)
    • jMonkeyEngine: get/setTimeOffset (but I think this might be to set the sample that playback starts from)
    • Moai: get/setPosition
    • Unity: time and timeSamples variables

    I'm now thinking that get/setPlaybackPosition may be best names for LÖVE.

  3. Pablo Mayobre

    That is not something I would like, I mean, I use one file with many sounds and use a combination of source:tell, source:seek, and love.update(dt) to see wich sound is playing.

    Writing tell/seek is easy and not confusing at all, even if they don't match the other functions.

    Also they are short, just 3 or 4 letters long, and you want to change it for a function with a name like get/setPlaybackPosition that has almost 20 letters... That definetly doesn't match love standards and I wouldn't write a function that long

  4. Alex Szpakowski

    Writing tell/seek is easy and not confusing at all, even if they don't match the other functions.

    If audio recording (issue #1) is added to LÖVE, what should a function which returns the current position/offset of the recording in samples or seconds be called?

  5. Pablo Mayobre

    I guess that recording wont be handled with sources, but instead return a source that you can save or play, so I guess that the API for that would be external to the source API.

    Was that what you mean? I don't really see a relationship with that sorry

  6. Alex Szpakowski

    It would return a SoundData.

    I mentioned it because it's functionality that achieves the exact same thing as Source:tell does right now, but for recording instead of playback. I don't really see any option other than naming it to something like love.audio.getRecordPosition or something similar, and renaming Source:tell and Source:seek to be consistent.

  7. Alex Szpakowski

    To clarify, an audio recording API will probably not return a Source. It will likely have a way to get all currently recorded audio as a SoundData, but it should also have a function for telling you how long you've been recording for (or how much time or samples will be in the SoundData if you wanted it now), hence love.audio.getRecordPosition or similar.

    Basically something like this: http://docs.unity3d.com/Documentation/ScriptReference/Microphone.GetPosition.html

  8. Pablo Mayobre

    Sorry I didn't wrote that right. What I mean is that you are getting the whole length of the data, it's quite different to seek or tell, so a source:length would match that function in the recorder.

    love.audio.recordedLength() would match source:length() and they do the same thing. But not the same thing as source:tell() (Althought source:tell() would return the same as source:length() after the source finished playing, and that would be useful to know if the source has already finished)

    Also I don't think that the recorder should be part of the audio module since they do different things but whatever

  9. Alex Szpakowski

    What I mean is that you are getting the whole length of the data

    But you aren't, you're getting the duration you've been recording for. The API will likely have a fixed maximum recording duration you set when you start recording, which would be the whole length (or use SoundData:getDuration).

    Also I don't think that the recorder should be part of the audio module since they do different things but whatever

    They don't. love.audio only deals with audio output right now because audio input hasn't been added to LÖVE at all yet, but it's all under the umbrella of interacting with audio hardware on the system (and the underlying libraries that LÖVE uses - OpenAL in this case - works that way too.)

  10. Pablo Mayobre

    I don't like that implementation, I should be able to record indefinetly until I say stop, And length would return the length of the SoundData at that time or if used with a preexisting source, the whole length of the source

    If i want to set the duration of the recording then I would do:

    if love.recorder.length() > duration then
        sourcedata = love.recorder.stop()
    end
    
  11. Pablo Mayobre

    Side note: Source:length would do the same as Source:getDuration but is shorter and can be easily understanded (getDuration can be easily understanded too).

    But this doesn't matter, I just want to point out that length/getDuration and tell do different things

  12. Alex Szpakowski

    I don't like that implementation, I should be able to record indefinetly until I say stop

    There are some technical and performance reasons why the implementation I described is the most likely one.

    If i want to set the duration of the recording then I would do:

    In the likely implementation I'm describing, you can still stop the recording early like that.

    Regardless, that's very off-topic for the actual issue - and the code you posted still gives the duration you've been recording for, just like Source:tell gives the duration the source has been playing for.

  13. Pablo Mayobre

    The duration of the recorder is the same as the duration of the whole SourceData that you have until that time, If you stopped recording just after you got the duration of the recording then the duration of the recording would be equal to the duration of the whole SourceData, and that differs from what tell does.

  14. Pablo Mayobre

    This are the functions I would love to have: Source:seek() Source:tell() Source:length() love.recorder.setDuration() love.recorder.start() love.recorder.getLength() love.recorder.stop()

  15. Alex Szpakowski

    The duration of the recorder is the same as the duration of the whole SourceData that you have until that time, If you stopped recording just after you got the duration of the recording then the duration of the recording would be equal to the duration of the whole SourceData, and that differs from what tell does.

    There's no such thing as a SourceData, but disregarding that, you can flip it around and say the same thing about Sources (except they go in the other direction, from data into hardware instead of from hardware into data):

    The value returned by Source:tell is the same as the duration of the whole buffer that you have until that time. If you stopped playback just after you call Source:tell, then the duration of the sound that played back would be equal to the duration of what was used to play the audio.

    As an aside, Source:getDuration doesn't exist - mostly because (IIRC) it's not really possible to be completely accurate with it for streaming Sources.

  16. Pablo Mayobre

    Good points, but I think that seek and tell should stay like that, even if they are not consistant with the recorder API. Thanks for the explanation It made lots of sense

  17. Bart van Strien

    Since most of this discussion has focused on recording, let me deal with the other frameworks name instead:

    • offset: Yes, it's an offset, it wouldn't be the name I'd be looking for, though.
    • position: We already have source positions, and they have nothing to do with time. In case you're thinking of renaming get/setPosition, they actually describe a physical position.
    • time: Possible, but getTime and setTime just feel awkward.
    • seek/tell: Matches up with Files, seeking is something found in media players everywhere, tell, well, not so much, but is usually seen as the opposite.

    I'm not saying seek/tell are perfect, but in my opinion they do qualify for the least bad.

  18. hahawoo reporter

    @Alex Szpakowski I know. :( But, it's only one character more than get/setBackgroundColor. :P

    I think the verbosity makes it more obvious what it does though: Source:tell vs. Source:getPlaybackPosition... I think with the latter it's easier to tell (tee hee) what's going on.

    And plus, other than seek/tell in Files, I think this may be the only place in the API where clear getter/setter functions aren't named get/setState.

    @Bart van Strien Maybe... Would the problem be that someone could mistake set/getPosition for methods which change the playback position? There's nothing stopping this misunderstanding currently, but I guess if that person had read about set/getPlaybackPosition they might be more prone to misremember. Disambiguation doesn't strike me as a huge concern, but they are similar names.

  19. hahawoo reporter
    • changed status to open

    I think I've found a good name! Maybe.

    Source:set/getPlaybackProgress

    (Also Video:set/getPlaybackProgress too now.)

    Just to reiterate the points I still think are valid about this:

    • The naming of seek/tell for Sources and Videos is conspicuously inconsistent with other functions in the API.
    • seek would be a fine name if it wasn't for this inconsistency, tell less so in my opinion.
    • I don't think the verbosity of set/getPlaybackProgress is a problem considering other function names in the API and how often these functions would be used.
  20. Raidho

    As far as data string navigation goes, seek/tell are the industry standard and are used in virutally every single existing framework and library that does this. Maybe there are better names (which have not been presented this far, none of them are good replacement), but these were around since DOS and are extremely ubiquitous, so even if you found a good replacement, changing them for any reason whould achieve nothing but confuse people.

    It's as if you renamed shaders to "pixel effects". That will just accomplish nothing.

  21. hahawoo reporter

    To be clear, I'm not suggesting changing the seek/tell data string navigation function names (i.e. the File object methods), only for Sources and Videos.

  22. hahawoo reporter

    @Gabe Stilez, I agree that seek is clear, I don't know if tell is but it is analogous to the File method.

    Just for the fun of discussion, I'll share some ideas I have on LOVE's naming which are relevant to this particular issue, all just my current opinion of course:

    • If the name is descriptive enough there may be less reliance on docs. I think it's useful if a function's name reads similarly to its description. (I could even imagine alternative docs where descriptions aren't always needed if the function is simple in purpose and its name is clear enough. Maybe!)

    • All else being equal, I prefer shorter names because they are easier to read and write and are cuter.

    • With consistency, in this case setters/getters prefixed by "set/get" (although LOVE also has boolean-returning getters prefixed "is" and "has"), you can get benefits which may be unintended. For example, see this section of the SLAM library, and how it makes use of how the functions share prefixes. Also see this API reference that I made, and how the function navigation is simplified by the shared prefixes.

    • Also, when setters and getters differ only by prefix you only have to remember one "thing", i.e. the name of the state which it relates to ("playback progress" in this case). If you ever see setPlaybackProgress in code or otherwise remember it, you can assume that getPlaybackProgress exists too.

    • I think that consistency is a sign of quality. Inconsistency is kind of like a spelling mistake, it may still be obvious what is meant, but it's not confidence inspiring.

  23. hahawoo reporter

    Now I think I prefer get/setPlaybackPosition, and renaming the listener functions to disambiguate get/setPosition and also because I think they might be better names (maybe), i.e. get/setListenerPosition, get/setListenerOrientation and get/setListenerVelocity.

    The reason I think they might be better is that the playback functions (love.audio.play etc.) are named the same as Source methods and operate on Sources, whereas the listener functions are also named the same as Source methods but operate on the listener, i.e. there is no indication in their naming of what they operate on.

    (Not that the playback functions would make sense for the listener of course, but both Sources and the listener have positional state.)

    I would also suggest that the names are more self-descriptive, i.e. setListenerPosition "sets the listener position".

    I don't think get/setDistanceModel and get/setDopplerScale would need to be changed because I think that these are about the relationship between Sources and the listener, and neither would get/setVolume because this is about the relationship between LÖVE's audio world and the computer's audio system, to the best of my understanding.

  24. Log in to comment