NativeInstruments / ni-media

NI Media is a C++ library for reading and writing audio streams.
MIT License
235 stars 34 forks source link

Feature add linux gstreamer support #11

Open hhoegelo opened 6 years ago

hhoegelo commented 6 years ago

If you like, you can pull my attempt of adding mp3, m4a, alac and wma support on linux platform by using gstreamer framework. I didn't manage to have all tests green, though. For example, the 'interlaced_by_67_frames'-tests are off by 1 sample, which I - so far - couldn't find a reason for.

Most likely other people won't notice my fork. I hope, if you pull it into the base fork, we have a higher chance that anyone can help fixing.

marcrambo commented 6 years ago

Hey Henry!

That's great! Thanks a lot for the contribution. I will look at it as soon as possible. In the mean time could you please update the travis-ci to retrieve the gstreamer package so that the linux build passes?

Thanks, Marc

wolfv commented 6 years ago

Is this still being worked on? I would be quite interested in using the library with these features for xtensor-io...

marcrambo commented 6 years ago

We'd need to resolve the following issues before we can merge: 1) Rerun the travis build to verify the build is passing 2) Fix the failing tests that Henry mentioned earlier. These failing tests mean that sample accurate seeking is not working as expected. We should investigate whether this is a bug ni-media or gstreamer and fix it.

Henry was kindly asking for people who can help out with fixing 2). I already had a look into it a few weeks ago but got caught up with other work unfortunately. @wolfv maybe you would like to have a look?

wolfv commented 6 years ago

I ran the tests locally on Fedora 27 and I got the following output:

Running tests...
Test project /home/wolfv/Programs/ni-media/build
    Start 1: pcm_test_run
1/2 Test #1: pcm_test_run .....................   Passed  910.07 sec
    Start 2: audiostream_test_run
2/2 Test #2: audiostream_test_run .............***Exception: Child aborted  0.40 sec

50% tests passed, 1 tests failed out of 2

Total Test time (real) = 910.48 sec

The following tests FAILED:
      2 - audiostream_test_run (Child aborted)
Errors while running CTest
make: *** [Makefile:84: test] Error 8

So I guess the test is really failing. Can you give me some more insights where to look? I really don't know much about any of the components, so I am not sure I can do it...

wolfv commented 6 years ago

ok, the failing test was related to not having checked out the Git LFS files.

marcrambo commented 6 years ago

Yes, the git lfs files need to be pulled manually before running the test. We ran into github lfs bandwidth limitations so we disabled the automatic lfs pull.

marcrambo commented 6 years ago

Btw, the tests are currently disabled on travis and appveyor because of the mentioned lfs bandwidth limitation we ran into. We have more bandwidth now so i am currently trying to reenable the tests on the ci but am currently facing some other compile errors with newer boost version. I hope i can make a PR soon to reenable the tests.

wolfv commented 6 years ago

ok, got it now (also copied reference_files to fuzz_files as there was no fuzz_files folder). I am seeing some issues with ifstream_test read_beginning_of_stream_twice etc.

maybe i can look into it, but it would definitly be faster if someone with more experience does it :)

marcrambo commented 6 years ago

ah the fuzz files are missing. that probably happened during one of my upstream merges and went unnoticed because the tests are disabled. i will upload them. thanks for the feedback

wolfv commented 6 years ago

Good to know.

Well I tried to debug a bit further, and I get "gstreamer_file_source: pipeline doesn't preroll into paused state" for m4a files, and "gstreamer_file_source: could not query duration from gstreamer" for MP3 files (WAV works fine, but doesn't seem to use gstreamer).

I am not sure if I am not linking correctly against gstreamer -- on the command line the following:

gst-launch filesrc location=.../sin440.1.44100.44100.cbr192.mp3 ! mad ! audioconvert ! audioresample ! alsasink

appears to work fine though.

marcrambo commented 6 years ago

I updated the master branch to include the missing fuzz files and reenabled the tests on the ci. I didn't have time so far to look any further into the failing tests. I hope i can find a few hours next week.

wolfv commented 6 years ago

That would be very cool. We'd be more than happy to package ni-media for anaconda when this feature is in!

Le 18 mars 2018 09:49, "Marc Boucek" notifications@github.com a écrit :

I updated the master branch to include the missing fuzz files and reenabled the tests on the ci. I didn't have time so far to look any further into the failing tests. I hope i can find a few hours next week.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/NativeInstruments/ni-media/pull/11#issuecomment-373982046, or mute the thread https://github.com/notifications/unsubscribe-auth/AA2BPtwqedLy-HE5hajOGS5yFC-Zo9nfks5tfh-GgaJpZM4RUlgW .

marcrambo commented 6 years ago

I fixed the "gstreamer_file_source: could not query duration from gstreamer" exception, and now most tests pass. I will try to fix the remaining tests later today. I pushed to my fork https://github.com/marcrambo/ni-media/tree/feature-add-linux-gstreamer-support in case someone wants to have a look.

wolfv commented 6 years ago

cool!

marcrambo commented 6 years ago

So i digged deeper into the failing tests and i see two remaining issues:

1) There seems to be a race condition in wait_for_async_operation() which is called in preroll_pipeline(). I randomly get a"gstreamer_file_source: pipeline doesn't preroll into paused state" exception on stream construction. This is independent of whether the source is mp3 or mp4.

2) Some read_interlaced tests fail as the gstream seek opration seems to sometimes be 1 frame off. This happens for both mp3s and mp4s ( alac as well as aac encoded ). This points to a bug in gstreamer and not in the decoder itself as otherwise the behaviour would differ for mp3s and mp4s.

I believe 1) should be fairly easy to fix and is mandatory before merging this PR.

I guess 2) might take quite a while to fix. I am not even sure if we can find a workaround on the client side or if this has to be fixed upstream in gstreamer. We had similar seeking issues using media_foundation on windows and it was quite painful to find a workaround. I guess people interested in having mp3 / mp4 support in ni-media on linux might not care so much about the potential 1 frame offset on seek so it would be an option to merge this PR nonetheless. We can disable the failing tests and mark the gstreamer support as experimental documenting the 1 frame offset bug.

hhoegelo commented 6 years ago

Yesterday, I told gstreamer core developer "slomo" (@sdroege) about the 'one sample off'-problem and he told me, he will have a look. Beside from this, he told me that running the GMainLoop manually for pre-rolling the pipeline is not necessary, so I will remove g_main_loop_iterate if I find the time. Same for bus messages - there are other ways to receive messages from the bus - no need to drive a GMainLoop. Also, I found a bug: the mime type of the stream is not detected properly and nailed to 'mp3' instead.

sdroege commented 6 years ago

@hhoegelo Can you explain the easiest way to reproduce the 1-sample-off problem and what information you gathered so far?

If I understand correctly, this happens on specific files (WMA? Or everything? All WMA files or a specific one?) and if you decode from the beginning vs. you seek to a position, you would end up one sample off? Is it always absolutely one sample, or would it sometimes be more or less, or would every seek make it one more sample off?

Re main loop: this is not required, correct. You have multiple options, but first of all: why do you care? You want to block until error or reading samples is possible? And then blocking-read the samples?

1) Blocking retrieval of messages from the GStreamer bus to know when pre-rolling has happened. This does not help you much here because you also block on reading samples and you want to continue getting bus message to know when an error happened 2) Integration of the GStreamer bus into whatever event loop you're using here. Details depend on how that event loop work 3) Synchronously getting messages from the bus and handling them from there. You could use that to get notified about errors once pre-rolling has happened in 1) for example. Note that you must call things like setting pipeline states from a different thread then the sync message handler.

From what I saw in the code so far I would suggest a combination of 1) and 3). You block until error or the sink is ready (you received the ASYNC_DONE message on the bus), by calling gst_bus_timed_pop_filtered(bus, whatever_timeout_you_want_in_ns, GST_MESSAGE_ERROR | GST_MESSAGE_EOS | GST_MESSAGE_ASYNC_DONE). In addition you add a sync bus handler (gst_bus_set_sync_handler) that ignores all messages until ASYNC_DONE is received. Once ASYNC_DONE is returned from gst_bus_timed_pop_filtered you are pre-rolled and can start. If instead ERROR or EOS is received you know that nothing will ever work here. You then start pulling from the appsink, but if the sync bus handler notifies you about ERROR, you asynchronously report that somehow and shut down the pipeline (gst_element_call_async might be useful for the latter).

sdroege commented 6 years ago

Sorry if some comments appeared twice, GitHub had some kind of hickup while I was doing the review and returned a few server errors

kevin-- commented 6 years ago

thanks for the thorough look @sdroege really appreciate seeing this level of collab

marcrambo commented 6 years ago

@hhoegelo can you please pull in my branch https://github.com/marcrambo/ni-media/tree/feature-add-linux-gstreamer-support? I reenabled the tests and commited a fix for all the tests failing due to an exception thrown while retrieving the track length. Also I improved the logging so we have a better overview of which tests are still failing.

marcrambo commented 6 years ago

@sdroege Thanks a lot for your help which is really greatly appreciated! I was wondering if it was possible to run gstreamer in "synchronous" mode i.e. without having to do "preroll pipeline" and so on. Calls to gestream_source::read don't need to be realtime capable i.e. non blocking. Actually all the other backend sources are synchronous (except itunes_source which also does some sort of asynchronous prerolling / caching as in gstreamer )

sdroege commented 6 years ago

@sdroege Thanks a lot for your help which is really greatly appreciated! I was wondering if it was possible to run gstreamer in "synchronous" mode i.e. without having to do "preroll pipeline" and so on. Calls to gestream_source::read don't need to be realtime capable i.e. non blocking. Actually all the other backend sources are synchronous (except itunes_source which also does some sort of asynchronous prerolling / caching as in gstreamer )

@marcrambo That's exactly how it would behave with my suggestion (1 + 3) from comment https://github.com/NativeInstruments/ni-media/pull/11#issuecomment-381393685

While GStreamer is always async to some degree, there is API that makes it easy enough to build a sync API on top