Add new API method for seeking audio streams using seconds; bugfix where one method returns success when NULL is parsed; typo corrected in a comment

[HeroesOfBalkan]

Since there is one API method called ma_sound_seek_to_pcm_frame() available to end-users, it would be nice to have similar API methods that use seconds instead of PCM frames (as there are other alternative methods having their PCM equivalents).
Three new methods are introduced:

• ma_result ma_sound_seek_to_second(ma_sound*, float) (matches ma_sound_seek_to_pcm_frame(ma_sound*, ma_uint64)),
• ma_result ma_data_source_seek_to_second(ma_data_source*, float) (matches ma_data_source_seek_to_pcm_frame(ma_data_source*, ma_uint64), and
• ma_result ma_data_source_seek_seconds(ma_data_source*, float, float*) (matches ma_data_source_seek_pcm_frames(ma_data_source*, ma_uint64, ma_uint64*))

While reading definitions of those inspired functions, I noticed that there is (I suppose) a bug in ma_result ma_data_source_seek_to_pcm_frame(ma_data_source *pDataSource, ma_uint64 frameIndex) where this function returns MA_SUCCESS when pDataSource is NULL, while other similar functions return MA_INVALID_ARGS. It should not return successful operation on uninitialized data source pDataSource. That was a one line change. Another thing I've noticed was a typo in a comment inside the same function definition, so I quickly corrected that in the same commit where this bugfix is done.

To test new changes, I wrote a mini program (main.c):

#include <stdio.h>
#include <stdlib.h>

#define MINIAUDIO_IMPLEMENTATION
#include "miniaudio.h"

int main() {
  ma_result result;
  ma_engine engine;
  ma_sound sound;
  ma_data_source *data_src;
  char shit;

  // Checking if pDataSource is passed properly to `ma_data_source_seek_to_pcm_frame()` method...
  result = ma_data_source_seek_to_pcm_frame(NULL, 441000); // 10s for sampleRate = 44'100
  printf("Bugfix check result: %d\n", result);

  result = ma_engine_init(NULL, &engine);
  if (result)
    return result;

  result = ma_sound_init_from_file(&engine, "HITC.mp3", 0, NULL, NULL, &sound);
  if (result)
    return result;

  // Testing method no. 1:
  printf("Song \"Heavy is the Crown\" is loaded :D\n");
  result = ma_sound_seek_to_second(&sound, 6.0);
  if (result)
    printf("bruh\n");

  ma_sound_start(&sound);
  printf("Press enter to continue...\n");
  scanf("%c", &shit);
  while (getchar() != '\n');

  ma_sound_stop(&sound);

  data_src = sound.pDataSource;

  // Testing method no. 2:
  ma_data_source_seek_to_second(data_src, 42.5f);
  ma_sound_start(&sound);
  printf("Press enter to continue 2...\n");
  scanf("%c", &shit);
  while (getchar() != '\n');

  // Testing method no. 3
  float seekd;
  ma_uint64 fseekd;
  ma_sound_stop(&sound);
  // ma_data_source_seek_seconds(data_src, 10.0f, &seekd);
  ma_data_source_seek_pcm_frames(data_src, 441000, &fseekd);
  /* Unable to test `ma_data_source_seek_seconds()`
     Reason: There's a NULL dereference inside of
             Miniaudio implementation.
             `ma_data_source_seek_pcm_frames()` is
             also affected
  */

  printf("Press enter to exit...\n");
  scanf("%c", &shit);

  ma_sound_stop(&sound);
  ma_sound_uninit(&sound);
  ma_engine_uninit(&engine);

  printf("secs seekd: %f\n", seekd);
  
  return 0;
}

And compiled with gcc main.c -o main -lm -ldl -pthread -g

Two new methods ma_sound_seek_to_second() and ma_data_source_seek_to_second() work as expected, and ma_data_source_seek_to_pcm_frame() returns error properly...

...but the last new method ma_data_source_seek_seconds() leads to segmentation fault. Though it was something in my implementation, so I tried out quickly with its pair ma_data_source_seek_pcm_frames(), and it also segfaults. Those two functions lead to segmentation fault because... TLDR: NULL value is passed to second argument void *pFramesOut of ma_data_source_read_pcm_frames() function, where further a NULL value was dereferenced.

On the eye, ma_data_source_seek_seconds() seems fine. For this segfault problem, I can create an issue or make separate PR, but I'll discuss with you first on what's appropriate action.

[HeroesOfBalkan]

About this Segmentation Fault, I used Valgrind to see where it happened, and this is its output:

==29925== Invalid write of size 4
==29925==    at 0x4B0F1A: ma_dr_mp3_s16_to_f32 (miniaudio.h:92665)
==29925==    by 0x4B11FC: ma_dr_mp3_read_pcm_frames_f32 (miniaudio.h:92724)
==29925==    by 0x450330: ma_mp3_read_pcm_frames (miniaudio.h:63084)
==29925==    by 0x44FBB5: ma_mp3_ds_read (miniaudio.h:62749)
==29925==    by 0x4491D6: ma_data_source_read_pcm_frames_within_range (miniaudio.h:57534)
==29925==    by 0x449440: ma_data_source_read_pcm_frames (miniaudio.h:57649)
==29925==    by 0x453025: ma_decoder_read_pcm_frames (miniaudio.h:65488)
==29925==    by 0x4509E8: ma_decoder__data_source_on_read (miniaudio.h:64244)
==29925==    by 0x4491D6: ma_data_source_read_pcm_frames_within_range (miniaudio.h:57534)
==29925==    by 0x449440: ma_data_source_read_pcm_frames (miniaudio.h:57649)
==29925==    by 0x45DA28: ma_resource_manager_data_buffer_read_pcm_frames (miniaudio.h:69525)
==29925==    by 0x45CC23: ma_resource_manager_data_buffer_cb__read_pcm_frames (miniaudio.h:69113)
==29925==  Address 0x0 is not stack'd, malloc'd or (recently) free'd
==29925== 
==29925== 
==29925== Process terminating with default action of signal 11 (SIGSEGV): dumping core
==29925==  Access not within mapped region at address 0x0
==29925==    at 0x4B0F1A: ma_dr_mp3_s16_to_f32 (miniaudio.h:92665)
==29925==    by 0x4B11FC: ma_dr_mp3_read_pcm_frames_f32 (miniaudio.h:92724)
==29925==    by 0x450330: ma_mp3_read_pcm_frames (miniaudio.h:63084)
==29925==    by 0x44FBB5: ma_mp3_ds_read (miniaudio.h:62749)
==29925==    by 0x4491D6: ma_data_source_read_pcm_frames_within_range (miniaudio.h:57534)
==29925==    by 0x449440: ma_data_source_read_pcm_frames (miniaudio.h:57649)
==29925==    by 0x453025: ma_decoder_read_pcm_frames (miniaudio.h:65488)
==29925==    by 0x4509E8: ma_decoder__data_source_on_read (miniaudio.h:64244)
==29925==    by 0x4491D6: ma_data_source_read_pcm_frames_within_range (miniaudio.h:57534)
==29925==    by 0x449440: ma_data_source_read_pcm_frames (miniaudio.h:57649)
==29925==    by 0x45DA28: ma_resource_manager_data_buffer_read_pcm_frames (miniaudio.h:69525)
==29925==    by 0x45CC23: ma_resource_manager_data_buffer_cb__read_pcm_frames (miniaudio.h:69113)
==29925==  If you believe this happened as a result of a stack
==29925==  overflow in your program's main thread (unlikely but
==29925==  possible), you can try to increase the size of the
==29925==  main thread stack using the --main-stacksize= flag.
==29925==  The main thread stack size used in this run was 8388608.
==29925== 
==29925== HEAP SUMMARY:
==29925==     in use at exit: 5,377,601 bytes in 357 blocks
==29925==   total heap usage: 2,702 allocs, 2,345 frees, 5,641,214 bytes allocated

I used GDB as well to see the values and how they propagated. What I've noticed is that when ma_data_source_read_pcm_frames() is called in those 2 functions, a NULL value has been passed to second argument pFramesOut, which through the whole execution it has never been checked against NULL. In ma_dr_mp3_read_pcm_frames_f32() the pFramesOut has been passed to third parameter pBufferOut, in which further (based on format) operations on NULL address were done.

Posting this here for now, just in case

[mackron]

Thanks. I can see the null dereference bug but haven't had a chance to fix that just yet. I'll address that separately to this PR.

As for the PR itself, I support the idea behind this in principle, but a few minor things:

1. In ma_data_source_seek_to_second(), I think you're calling straight into the vtable? This should instead call into ma_data_source_seek_to_pcm_frame(). I just like to keep calling into the vtable callbacks as restricted as possible.
2. These lines need a cast: frameIndex = secondIndex * sampleRate;
3. ma_data_source_seek_seconds() should call into ma_data_source_seek_pcm_frames() instead of ma_data_source_read_pcm_frames(). This way if I change the forward-seek logic later on, the seek_seconds() function should "just work" without modification.
4. ma_sound_seek_to_second() should call into ma_sound_seek_to_pcm_frame() thereby avoiding duplicating the atomic exchange code (I just like to keep atomic stuff to a minimum as a matter of principle since it can get a bit fiddly and error prone).
5. Pretty minor, but let's rename seekIndex to something like seekPointInSeconds.

Other than that I can't see any issues from my quick scan just now. Thanks.

[HeroesOfBalkan]

I corrected those things you have mentioned. Additionally, I've found an uninitialized variable bug present in my initial code (oops), and fixed it quickly.

About the 5th point: I couldn't find seekIndex variables, so I assumed you've meant secondIndex and renamed them to seekPointInSeconds.

I tested with the same mini program (main.c), but with -Wall -Wextra -Wfloat-conversion flags this time.

[mackron]

I finally got around to fixing up that crash in ma_data_source_seek_pcm_frames(). It's in the dev branch. I could make it more efficient, but since this crash is so blatant and this is the first anyone has ever reported it, it's obviously not a very frequently used function so I don't care too much about optimizing it right now.

Yes I did indeed mean secondIndex. And the warning flags I compile with are -Wall -Wextra -Wpedantic, but it's not a big deal if warnings slip through - I can easily fix them as they're reported. Those casting warnings were just something I noticed when scanning your code the other day so I figured I'd mention it.