audio-cb: add audio callback driver #5566

deltabeard · 2018-02-24T13:22:49Z

A basic audio callback driver for libmpv. Returns samples in S16 stereo
format to a buffer provided when calling mpv_audio_callback().

Tested working with libretro-mpv.

I agree that my changes can be relicensed to LGPL 2.1 or later.

Signed-off-by: Mahyar Koshkouei mahyar.koshkouei@gmail.com

ghost · 2018-02-24T17:17:18Z

I don't like how this makes the audio callback a global thing. It should be connected to a mpv_handle at least, plus ideally a way that identifies the audio source (in case if mpv ever becomes able to use multiple AOs, e.g. one for each audio track).

I'm also not sure whether restriction it to s16/stereo is OK, and the sample rate will still be unknown (or subject to race conditions) to the API user.

ghost · 2018-02-24T17:21:34Z

Also, as a bikeshed comment: maybe the "-cb" should be eliminated from the name. The opengl-cb API is already being deprecated and essentially renamed (as part of the change) in my render API PR.

deltabeard · 2018-02-24T17:49:16Z

Thanks for your comments.

The restriction to S16 stereo isn't great, but it was my minimum requirement. I can make it so that the frontend application can call a function to restrict supported output formats, should the frontend not support all the formats supported by mpv. I will only be able to test S16 Stereo however, as the frontend I'm supporting only supports that format.
Or, the driver could be rewritten to have a "create", "render", and "free" function akin to the render API. Where the frontend can specify supported audio formats when calling the create function. This may make the API more consistent.

I'll have a look at mpv_handle and audio source identification.
I'm unsure how to resolve the sample-rate issue. Maybe returning a struct from the callback function that contains the audio format, channels, and the sample rate of the selected track?

The "-cb" was there because I began writing this last year. This is a simple change. What do you suggest I rename it to? Is renaming the audio callback driver to "libmpv" okay?

ghost · 2018-02-25T17:18:07Z

I'll have a look at mpv_handle and audio source identification.

Currently there's only at most 1 AO per mpv core. The same is true for VOs. But if more than 1 VO is supported in the future, the new render API can in theory easily support it by adding a new mpv_render_param that references with VO with a name or so. This is purely an API thing and internally there's nothing for it.

The "-cb" was there because I began writing this last year. This is a simple change. What do you suggest I rename it to? Is renaming the audio callback driver to "libmpv" okay?

Should be fine. The new render API also uses libmpv as VO name.

I'm unsure how to resolve the sample-rate issue. Maybe returning a struct from the callback function that contains the audio format, channels, and the sample rate of the selected track?

That's a possibility, but doesn't work for all audio params currently, because the memory size/organization depends on channel layout and sample format. It'd also just be possible that the client simply forces a specific format via the global mpv options, but that's also rather restrictive.

I'm also a bit nervous that the API ignores playback states (like pausing etc).

haasn · 2018-03-01T04:38:20Z

I will most likely have a need for this API in the future, and may be able to provide API design comments based on my requirements at that time.

deltabeard · 2018-03-01T15:09:29Z

I will most likely have a need for this API in the future, and may be able to provide API design comments based on my requirements at that time.

Thanks, that'll be helpful.

leavittx · 2018-03-14T06:32:03Z

I would also need audio callback functionality soon!
Having an ability to specify the sample format (interleaved float32 is what I need the most), sample rate, number of output audio channels would be essentional here!

haasn · 2018-03-27T08:35:18Z

Some feedback:

I will need to give the output frequency at initialization time, which will not change. I can not easily handle arbitrary frequencies, and would prefer it if libmpv takes care of them.
I only require support for stereo
I need samples in float32 format. (Although conversion is fortunately easy, here)
I need to have an audio driver per libmpv core instance. (So it needs to be associated with the mpv_handle)

deltabeard · 2018-03-27T09:00:48Z

Thanks for the feedback. Due to my work commitments, I won't be able to continue working on this until June.

A basic audio callback driver for libmpv. Returns samples in S16 stereo format to a buffer provided when calling mpv_audio_callback(). Tested working with libretro-mpv. Signed-off-by: Mahyar Koshkouei <mahyar.koshkouei@gmail.com>

By using the "af" option string command, the audio callback driver may be configured to output audio in a specific format. For example, prior to calling mpv_audio_callback(), the following command is called to set the audio filled in the buffer to a specific format: mpv_set_option_string(mpv, "af", "format=s16:48000:stereo"); Signed-off-by: Mahyar Koshkouei <mahyar.koshkouei@gmail.com>

Signed-off-by: Mahyar Koshkouei <mahyar.koshkouei@gmail.com>

deltabeard · 2018-06-08T19:09:58Z

Hi all. Apologies for the delay.

This is still a work in progress. So far, I have the audio driver tied to an mpv_handle so more than one instance should be able to use it (but I have not tested this yet).

The audio driver currently fills the buffer with whatever the audio format the input audio stream is (as long as it isn't planar *). Currently, to specify a specific format of audio, the user application must call a command similar to mpv_set_option_string(mpv, "af", "format=s16:48000:stereo"); which will convert the audio stream to the one requested.

Instead of having to use such a command to force a specific format of audio, I plan on adding options to the libmpv audio driver to allow the user application to specify which audio formats, sample rates and channels they support, similarly to the ao_null and ao_sdl driver (the latter which seems to force s16 if the input audio stream is not compatible). This means that libmpv will not need to unnecessarily convert an input audio stream if the user application supports it.

* I did have an issue whereby a media file with an AF_FORMAT_FLOATP audio stream was causing a arithmetic exception:
0x00007fffed769b58 in ao_read_data (ao=0x7fffc45a5e40, data=0x7fffffffbdf0, samples=521, out_time_us=11474857) at ../audio/out/pull.c:184
Forcing non-planar audio formats in the libmpv audio driver stops this issue from occurring. I'm not sure why this happens, or if this is actually an issue.

Signed-off-by: Mahyar Koshkouei <mahyar.koshkouei@gmail.com>

deltabeard · 2018-06-14T18:54:03Z

I think I have completed the feature now. The libmpv audio driver now has channel layout, sample rate and audio format options that may be specified to obtain the audio frames in the format requested.

Planar formats cause an arithmetic exception, and are therefore disabled. And whilst multiple channel layouts may be specified, only one sampling rate and and one audio format may be specified.

I have not tested multi-thread support, but as the driver is now associated with an mpv_handle, this should work. Nor have I tested the changing of the output layout, sampling rate and format options mid-stream.

I have tested this branch with mpv-libretro which sets the output audio to s16, stereo only, and at 48000Hz.

Please let me know if I have missed anything.

Thanks.

haasn

Can't comment too much on the audio stuff itself

haasn · 2018-06-19T19:42:04Z

audio/out/ao.c

@@ -92,6 +93,7 @@ static const struct ao_driver * const audio_out_drivers[] = {
 #if HAVE_COREAUDIO
    &audio_out_coreaudio_exclusive,
 #endif
+    &audio_out_libmpv,


Shouldn't this have the highest priority? (cf. video_out_libmpv)

haasn · 2018-06-19T19:43:03Z

audio/out/ao_libmpv.c

+    .resume    = resume,
+    .priv_size = sizeof(struct priv),
+    .priv_defaults = &(const struct priv) {
+        .init = false,


This is implicit

haasn · 2018-06-19T19:43:54Z

audio/out/ao_libmpv.c

+    if (af_fmt_is_planar(priv->format))
+        MP_ERR(ao, "planar format not supported\n");
+
+    if (priv->format)


Style: Always use { } for if/else statements

haasn · 2018-06-19T19:46:47Z

audio/out/ao_libmpv.c

+    if (priv->init == false)
+    {
+        MP_ERR(ao, "libmpv audio output not initialized\n");
+        return -4;


What is this magic number?

haasn · 2018-06-19T19:48:32Z

libmpv/client.h

+ * @param len Length of the buffer.
+ * @return Number of samples in the buffer, or negative on error.
+ */
+int mpv_audio_callback(mpv_handle *ctx, void *buffer, int len);


How do I know when to call this?

haasn · 2018-06-19T19:50:32Z

Oh, also, you need to document the --ao-libmpv- options in the man page

ofry · 2019-02-13T14:03:12Z

@deltabeard @haasn Any reasons which preventing to merge this?

deltabeard · 2019-02-13T14:09:17Z

My not having fixed the issues that @haasn raised is blocking this merge. I didn't have much spare time and I eventually lost interest in the application that was going to use this feature, so I haven't worked on this in a very long time.

Somebody else whose interested in this feature may work on it, otherwise it may as well be rejected. Sorry.

orbea · 2019-02-13T20:20:02Z

@deltabeard If you don't mind sharing, what led you to lose interest?

It seems this being fixed is a blocker for fixing --enable-mpv with RetroArch, but I don't feel confident finishing this myself...

deltabeard · 2019-02-14T09:55:45Z

Hi @orbea. I lost interest for the following reasons in no particular order:

There is audio stuttering issues in libretro-mpv when using this audio callback driver in libmpv. I didn't know how to fix this, and received no help in discovering what the issue was. It is not clear if the issue is in the audio callback driver in this patch, or if the issue is with libretro-mpv (edit: which is why I hadn't opened an issue; maybe I should have done).
libmpv does not seem to (or at least didn't) work properly on Raspberry Pi. I kept getting a blank screen, but could see subtitles. I opened an issue here: No video displayed with gpu video output on Raspberry Pi #5405, Blank screen on Raspberry Pi libretro/libretro-mpv#12. I kept getting a blank screen in RetroArch too: (BOUNTY) get_proc_address returns NULL on Raspberry Pi (OpenGL ES2) ($10) libretro/RetroArch#6137
It seems I couldn't get libmpv to compile/work on Windows? Issue: Segmentation fault with libmpv in MSYS2-MinGW64 #5853, Test on Windows libretro/libretro-mpv#16 I don't really remember the details, but it meant I could only test libretro-mpv on Linux.
I lost interest in the Raspberry Pi. The video drivers (at least at that time; they probably still are) are shit. The Raspberry Pi is all closed and proprietary and fuck all video decoding works properly. There is little interest in fixing this. Maybe VC4 has fixed this since, but I couldn't care less anymore. My interest in libmpv and libretro-mpv was for use with the Raspberry Pi, so I could make a small, streamlined application for playing games and watching movies without Kodi.
On the Raspberry Pi, mpv drops frames when subtitles change text. I don't know if there's an issue for this, or if this is still an issue.
Regarding RetroArch, the issue with the videoplayer (ffmpeg core) isn't the lack of hardware acceleration. mpv plays many videos fine without hardware acceleration on the same system. The reason for the very high CPU usage when decoding videos in the ffmpeg core of RetroArch must be investigated and fixed. One of the main reasons for libretro-mpv was easy hardware acceleration support for embedded systems. Since this didn't work on the Raspberry Pi, I lost interest even more. I think rewriting or fixing the ffmpeg core, and adding hardware acceleration support to it is better.
My time was and is limited. I was in my final year of study, and did not have enough time and patience to dedicate to this timesink of a project. All the code is open, so anyone else could have picked up the project.

Here's another issue which tracked libretro-mpv progress if anyone is interested: libretro/RetroArch#5070

This was a quick mind dump, so apologies in advance for mistakes. There may have been other issues. I don't really remember since it's been a long time now.

Kind Regards.

orbea · 2019-02-14T14:49:21Z

Thanks a lot of the background, unfortunately I can't do much to help with these concerns.

I think currently the mpv-libretro core is supposed to work on windows to some degree and another possible alternative to fixing this is to just use libretro for audio. I'm not sure what the best route would be though.

ghost · 2019-09-21T14:03:04Z

audio/out/ao_libmpv.c

+    if (priv->format)
+        ao->format = priv->format;
+    else {
+        /* Required as planar audio causes arithmetic exceptions in pull API. */


ghost

Is any still interested in this? The current approach is a bit too singleton-ish/hardcoded for my tastes.

deltabeard · 2019-09-21T14:06:31Z

Is any still interested in this? The current approach is a bit too singleton-ish/hardcoded for my tastes.

No, I lost interest a while back. I explained why in my earlier post on 14 Feb.
Thanks for considering the pull request.

deltabeard added 4 commits June 6, 2018 16:43

audio-cb: add audio callback driver

904ec98

A basic audio callback driver for libmpv. Returns samples in S16 stereo format to a buffer provided when calling mpv_audio_callback(). Tested working with libretro-mpv. Signed-off-by: Mahyar Koshkouei <mahyar.koshkouei@gmail.com>

audio-cb: allow all non-planar audio

602fca1

Signed-off-by: Mahyar Koshkouei <mahyar.koshkouei@gmail.com>

libmpv: rename audio-cb to libmpv

0211859

Signed-off-by: Mahyar Koshkouei <mahyar.koshkouei@gmail.com>

deltabeard force-pushed the ao_cb branch from 36ea844 to 0211859 Compare June 8, 2018 18:40

deltabeard added 3 commits June 13, 2018 00:34

libmpv: add options to ao-libmpv

c074ea7

Signed-off-by: Mahyar Koshkouei <mahyar.koshkouei@gmail.com>

libmpv: add libmpv audio driver documentation

41a59e1

Signed-off-by: Mahyar Koshkouei <mahyar.koshkouei@gmail.com>

libmpv: code style and naming fixes

7346e4c

Signed-off-by: Mahyar Koshkouei <mahyar.koshkouei@gmail.com>

haasn reviewed Jun 19, 2018

View reviewed changes

deltabeard mentioned this pull request Dec 28, 2018

Buildbot compatibility libretro/libretro-mpv#17

Open

This was referenced Feb 13, 2019

Building libretro-mpv core libretro/libretro-super#691

Open

[feature request] Allow building with shared lua libretro/RetroArch#8153

Open

orbea mentioned this pull request Feb 28, 2019

Support lua51. RetroAchievements/rcheevos#15

Closed

deltabeard mentioned this pull request Jun 27, 2019

Fix audio stuttering issues in high resolution & frame rate videos libretro/libretro-mpv#18

Open

6 tasks

ghost reviewed Sep 21, 2019

View reviewed changes

ghost suggested changes Sep 21, 2019

View reviewed changes

deltabeard closed this Sep 21, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

audio-cb: add audio callback driver #5566

audio-cb: add audio callback driver #5566

deltabeard commented Feb 24, 2018

ghost commented Feb 24, 2018

ghost commented Feb 24, 2018

deltabeard commented Feb 24, 2018 •

edited

Loading

ghost commented Feb 25, 2018

haasn commented Mar 1, 2018

deltabeard commented Mar 1, 2018

leavittx commented Mar 14, 2018 •

edited

Loading

haasn commented Mar 27, 2018

deltabeard commented Mar 27, 2018

deltabeard commented Jun 8, 2018

deltabeard commented Jun 14, 2018

haasn left a comment

haasn Jun 19, 2018

haasn Jun 19, 2018

haasn Jun 19, 2018

haasn Jun 19, 2018

haasn Jun 19, 2018

haasn commented Jun 19, 2018

ofry commented Feb 13, 2019

deltabeard commented Feb 13, 2019

orbea commented Feb 13, 2019

deltabeard commented Feb 14, 2019 •

edited

Loading

orbea commented Feb 14, 2019

ghost Sep 21, 2019

ghost left a comment

deltabeard commented Sep 21, 2019

audio-cb: add audio callback driver #5566

audio-cb: add audio callback driver #5566

Conversation

deltabeard commented Feb 24, 2018

ghost commented Feb 24, 2018

ghost commented Feb 24, 2018

deltabeard commented Feb 24, 2018 • edited Loading

ghost commented Feb 25, 2018

haasn commented Mar 1, 2018

deltabeard commented Mar 1, 2018

leavittx commented Mar 14, 2018 • edited Loading

haasn commented Mar 27, 2018

deltabeard commented Mar 27, 2018

deltabeard commented Jun 8, 2018

deltabeard commented Jun 14, 2018

haasn left a comment

Choose a reason for hiding this comment

haasn Jun 19, 2018

Choose a reason for hiding this comment

haasn Jun 19, 2018

Choose a reason for hiding this comment

haasn Jun 19, 2018

Choose a reason for hiding this comment

haasn Jun 19, 2018

Choose a reason for hiding this comment

haasn Jun 19, 2018

Choose a reason for hiding this comment

haasn commented Jun 19, 2018

ofry commented Feb 13, 2019

deltabeard commented Feb 13, 2019

orbea commented Feb 13, 2019

deltabeard commented Feb 14, 2019 • edited Loading

orbea commented Feb 14, 2019

ghost Sep 21, 2019

Choose a reason for hiding this comment

ghost left a comment

Choose a reason for hiding this comment

deltabeard commented Sep 21, 2019

deltabeard commented Feb 24, 2018 •

edited

Loading

leavittx commented Mar 14, 2018 •

edited

Loading

deltabeard commented Feb 14, 2019 •

edited

Loading