`AudioFrame.copyfrom()` should move the internal `used_size` marker

[microbit-carlos]

Using the latest version (at the time of writing) of the recording & playback branch: https://github.com/microbit-foundation/micropython-microbit-v2/commit/0b06914c71c18533da90df85230ac198578669bf Hex: https://github.com/microbit-foundation/micropython-microbit-v2/actions/runs/8416237764?pr=163

>>> a = microphone.record(100)
>>> b = audio.AudioFrame(100)
>>> b.copyfrom(a)
>>> # This plays a short audio
>>> audio.play(a)
>>> # This does not
>>> audio.play(b)
>>> # And the  sound data is there
>>> for i in range(len(a)):
...     assert a[i] == b[i]
... 
>>> 

If we manually edit the last byte in b it then does play a short clip:

>>> b[len(b) - 1] = 255
>>> audio.play(b)
>>> 

[dpgeorge]

Copying comment https://github.com/microbit-foundation/micropython-microbit-v2/pull/163#issuecomment-2068519700 here:

This needs discussion. copyfrom() actually allows copying from any object with the buffer protocol (eg bytes, memoryview) and they won't necessarily have the used_size attribute.

This method should probably just set used_size to the total bytes copied fro the buffer-like object. The only issue there is copying from another AudioFrame it will copy the total allocated size, not the used_size, of the other AudioFrame.

[microbit-carlos]

Yes, I think AudioFrame.copyfrom() should set used_size to to the last byte copied from the input argument (any buffer-like object).

So, if we start with a long AudioFrame that has data all the way to the end, and then we copy a short buffer into the AudioFrame, then we would end up with the used_size marker at length of the small buffer. But I think this makes sense only if we also provide accessors to the used_marker (https://github.com/microbit-foundation/micropython-microbit-v2/issues/196), otherwise there is "magic" that cannot be changed without odd workarounds (like assigning to itself the last indexable value, e.g audio_frame[len(audio_frame) - 1] = audio_frame[len(audio_frame) - 1] vs audio_frame.set_marker(len(audio_frame))).

The question would be, do we special-case copying from another AudioFrame? We'd have a few options, for all of these we can consider the following set up code:

   af_long = audio.AudioFrame(duration=5000)
   af_short = audio.AudioFrame(duration=2000)
   microphone.record_into(af_short, wait=False)
   sleep(1000)    # Record for only 1 second, af_short is only half filled
   microphone.stop_recording()

1. The AudioFrame is fully copied, including the used_size value

   af_long.copyfrom(af_short)
   audio.play(af_long) # Plays for 1 second

2. The AudioFrame is fully copied, the used_marker placed at size of the input argument

   af_long.copyfrom(af_short)
   audio.play(af_long) # Plays for 2 second, 1st second audio data, 2nd second empty
   af_long.set_position(af_short.get_position())
   audio.play(af_long) # Plays for 1 second

3. The AudioFrame is only copied until the used_marker

   af_long.copyfrom(af_short)
   audio.play(af_long) # Plays for 1 second

In my opinion, for a user option 1. and 3. make most sense for easy-of-use. Option 2. is not too bad, as it makes sense if you already know about the marker, however I think it's a lot more likely that the users would want 1 second playback instead of 2, so making that default makes sense.

From options 1. and 3. , I'm a lot more inclined on option 1., because option 3. limiting how much data is copied doesn't feel quite right.

---

This might also need to be discussed with https://github.com/microbit-foundation/micropython-microbit-v2/issues/194 in mind, as one of the suggestions is to add an extra argument to have AudioFrame.copyfrom(buffer, index).

af_long = AudioFrame(duration=5000)
af_short = AudioFrame(duration=2000) 
while True:
    if button_a.is_pressed():
        microphone.record_into(af_short, wait=False)
        while button_a.is_pressed():
            pass
        microphone.stop()
        af_long.copyfrom(af_short, index=af_long.get_position())
    if button_b.is_pressed():
        audio.play(large_af)
    sleep(200)

So in these cases we would need to make sure that when calculating the user_size position, it also needs to be offset from the index point (from AudioFrame.copyfrom(buffer, index)).

[microbit-carlos]

This can be separated in two parts:

1. We are in agreement that copyfrom() should move the marker based on the size of the data copied
2. We need to consider if we want to special case when copying from an AudioFrame that might have a marker not at the end of the buffer

So, 1. can be implemented already, and 2 will be considered as part of the new proposal:

• https://github.com/microbit-foundation/micropython-microbit-v2/issues/205

[microbit-carlos]

We've changed the implementation and the new audio objects do not have an "internal marker":

• https://github.com/microbit-foundation/micropython-microbit-v2/issues/205#issuecomment-2221716206