LibAAF icon indicating copy to clipboard operation
LibAAF copied to clipboard

Ardour Support - LibAAF v1.0

Open agfline opened this issue 1 year ago • 67 comments

Continuation of thread #5

agfline avatar Mar 14 '24 12:03 agfline

x42 - Robin - I know that Ardour8.4 got released only recently but does Ardour tend to make interim releases with bug fixes etc?

johne53 avatar Mar 14 '24 13:03 johne53

does Ardour tend to make interim releases with bug fixes etc?

No. Only if a very severe issue (data-loss, crash at start etc) comes up we do a new release (e.g. 8.3 was very short lived).

There are however https://nightly.ardour.org builds available.

x42 avatar Mar 14 '24 13:03 x42

I thought I might post something on the Mixbus forum to advise people that AAF's now available in Ardour - but can non-developers access those nightly builds?

johne53 avatar Mar 14 '24 13:03 johne53

can non-developers access those nightly builds?

Certainly. Everyone can download the demo version, and all subscribers, or users who paid more than $45 for a single build have access to the full versions.

x42 avatar Mar 14 '24 13:03 x42

@agfline - this is mostly a heads-up as it's something that's been confusing me...

In further testing, I've noticed that imported sessions are often created now with markers on the Mins:Secs ruler or the Location Markers ruler. They're generally in pairs so I'm wondering if they're in fact range markers (and therefore on the wrong ruler bar??)

Capture-12

johne53 avatar Mar 16 '24 08:03 johne53

Regarding AAF markers :

If a marker has a duration, it is added to Ardour like this :

new Location (*s,  markerStart, markerEnd, marker->name, Location::Flags (Location::IsRangeMarker));

Therefore it should appear on the Range Markers ruler.

If a marker has no duration, it is added to Ardour like that :

new Location (*s,  markerStart, markerStart, marker->name, Location::Flags (Location::IsMark));

Regarding Session start/stop :

Those are not set as markers, but using the following :

s->maybe_update_session_range (start, end);

Is there anything we could improve ?

agfline avatar Mar 16 '24 13:03 agfline

I've noticed that imported sessions are often created now with markers on the Mins:Secs ruler

Those red triangles show selection-range. They are not markers, and they are always on the top (like playhead triangle) regardless of which rulers are visible.

x42 avatar Mar 16 '24 14:03 x42

Is there anything we could improve ?

Nope. that is correct.

x42 avatar Mar 16 '24 14:03 x42

Option 1: directly import the files without intermediate save to $TMPDIR

Does Ardour already support importing files out of a buffer/callback ? From my understanding, it would require a custom Session::import_files (ImportStatus& status) function...

If nobody have time to work on it we can also deal with it later, since we already have a working import process. What do you think ?

agfline avatar Mar 16 '24 20:03 agfline

Does Ardour already support importing files out of a buffer/callback

yes. e.g FFMPEGFileImportableSource which runs ffmpeg, and the raw output is read from a pipe (buffer). similarly Mp3FileImportableSource and other ImportableSources.

We could derive a new class AAFImportableSource is-a ImportableSource`.

What API is available from libAAF? read, seek or a buffer and size?

x42 avatar Mar 16 '24 20:03 x42

I got a function which retrieves a complete stream out of CFB sectors, and allocates a buffer.

https://github.com/agfline/LibAAF/blob/336dda2c1eed72aa459260661a29aff31fd8335d/src/LibCFB/LibCFB.c#L729

... but it's beyond API. I could make something like :

aafi_getEmbeddedFile( aafiAudioEssenceFile *essenceFile, unsigned char *buf, size_t *bufsz )

agfline avatar Mar 16 '24 21:03 agfline

...and what if an AAF contains a 3 hour long podcast that doesn't fit in RAM? or a festival live show where a track can be 14+ hours long?

Well, I suppose we can ask a user to get more memory :)

x42 avatar Mar 16 '24 21:03 x42

I'm curious about unsigned char* -- what data-format does AAF use for Audio?

It seems like cfb_getStream is close. Ideally with an opaque handle (libardour needs not know about libAAF internal types), and an API that mirrors standard read(3) API would be best.

Compare to libsndfile's virtual file I/O which does that.

x42 avatar Mar 16 '24 21:03 x42

I'm curious about unsigned char* -- what data-format does AAF use for Audio?

cfb_getStream() is not specifically intended for audio. It simply retrieves a data stream out of CFB sectors, hence the unsigned char* generic type. Basically, AAF can store 3 types of audio files : WAV/BWAV, AIFF, or raw PCM. When retrieving a WAV/BWAV or AIFF file, the buffer returned by cfb_getStream() holds the full file with "header".

It seems like cfb_getStream is close. Ideally with an opaque handle (libardour needs not know about libAAF internal types), and an API that mirrors standard read(3) API would be best.

Sounds good to me. But the question is how do we handle raw PCM ? All required data (sample rate/size, etc.) are available in aafiAudioEssenceFile structure. But to be really opaque, I guess libaaf should build and return a valid WAV "header" prior to raw PCM data, so Ardour receives a complete WAV file.

Does Ardour need to be aware of file type ?

agfline avatar Mar 16 '24 22:03 agfline

Does Ardour need to be aware of file type ?

No, libsndfile handles this.

I was just curious, for some generic read function I'd expect the data-buffer to be void*, not unsigned char*.

Would the following API work for you?

ssize_t aaf_read_stream (void* aaf_handle, void* buf, size_t nbyte, off_t offset)

This follows POSIX pread(3): Read nbyes at byte-offset offset into buf and return the number of bytes read.

x42 avatar Mar 16 '24 22:03 x42

But the question is how do we handle raw PCM ?

hmm. We could special case this, perhaps another ImportableFileSource? How do you currently deal with this when using a file-cache?

All this is mess is why I don't want to deal with AAF, I'm amazed that you can put up with it :)

x42 avatar Mar 16 '24 22:03 x42

Would the following API work for you?

Yes it would. Note that AAF stores its content in a kind of FAT32 structure. So ideally, we should find a way to stick to a 512B/4096B sector size boundary for nbytes and offset...

How do you currently deal with this when using a file-cache ?

I'm writing fmt, bext and data chunks prior to raw PCM so the file becomes a readable WAV. I can easily reproduce it here, maybe with a dedicated function that returns a WAV header buffer only if essence is raw PCM ? This would maintain a sector-aligned read for raw PCM, with the actual read function...

All this is mess is why I don't want to deal with AAF, I'm amazed that you can put up with it :)

Challenge. That's what it's all about ;)

agfline avatar Mar 16 '24 23:03 agfline

Guys - please don't lose sight of the fact that AAF has only been fixed in the recent Ardour nightlies. For official releases (whether Ardour or Mixbus) there's still been nothing released with a working version of AAF import. That's (surely?) the most urgent problem to fix.

johne53 avatar Mar 17 '24 08:03 johne53

Maybe the next Ardour release could be based on current working code and we could work on this in a dedicated fork ?

agfline avatar Mar 17 '24 08:03 agfline

Hi Adrien - Did Dingo ever send you any ProTools AAF's? He sent me one called Johne_PT_AAF_test-A.aaf but it's crashing now in 'protools_post_processing()'

Just wondering if you could try it at your end sometime? (or any ProTools import)

[Edit...] It seems to be connected to your recent change to support multichannel clips.

johne53 avatar Mar 19 '24 11:03 johne53

Just wondering if you could try it at your end sometime? (or any ProTools import)

I don't have that file, nor any file from Dingo. But I bought ProTools a few weeks ago to make test files and I have had no issue with those. Can you send me that file ?

agfline avatar Mar 19 '24 11:03 agfline

Thanks John. Just had a quick look at your file. It crashes with Ardour but aaftool reads it with no issue (with both --pt-remove-sae and --pt-true-fades set, which enable protools post processing) I'm not sure how I can debug it with Ardour...

agfline avatar Mar 19 '24 12:03 agfline

I'm not sure how I can debug it with Ardour...

https://ardour.org/debugging_ardour :)

x42 avatar Mar 19 '24 13:03 x42

I can let you know why the crash occurs although it doesn't make a lotta sense...

		while (audioItem != NULL) {
			aafiTimelineItem* audioItemNext = audioItem->next;

			if (audioItem->type != AAFI_AUDIO_CLIP) {
				audioItem = audioItem->next; // <--- THIS LINE !!!!
				continue;
			}

			aafiAudioClip* audioClip = (aafiAudioClip*)audioItem->data;

The indicated line gets reached on about the third iteration of the 'while' loop. And for whatever reason, audioItem->next; returns a garbage value.

Shortly before the crash (and further down in the while loop) there's a similar line (though not identical) saying:-

audioItem = audioItemNext;

and for whatever reason, that one doesn't crash,

johne53 avatar Mar 19 '24 15:03 johne53

@x42 - I can see that libaaf has been updated in Ardour but for me at the moment, it'd be easier to test in Mixbus. So can you bring the changes across to Mixbus whenever you get a chance? In the meantime, I need to make the following point for Adrien's benefit:-

I'm shocked (yet again) to see that a simple fix has brought with it a whole plethora of unrelated changes. This'll require me to test absolutely everything here all over again and I'm simply not willing to keep doing this, week in and week out. I've been one of the strongest advocates for AAF but I never dreamed for a minute that it'd be undergoing such a constant stream of major changes. Until he regards it as stable, I doubt if Ben will want to include AAF in Mixbus - and until it gets released in Mixbus, we're unlikely to find any users who want it.

So please Adrien, you need to give us some idea of when you expect your codebase to stabilize. Or if there's some underlying reason why it can't be made stable, you need to help us understand why.

And sorry for the rant... haven't had my breakfast yet!

johne53 avatar Mar 20 '24 07:03 johne53

Hi John,

Before the release of libaaf v1.0, there were still many parts of code that needed to be somehow improved or rewritten. That's what I've been working on day and night for 3 months. The v1.0 also brings a solid test script with 63 AAF files to prevent regression in further updates. I run that test script for Linux and Windows before I push every update. Since libaaf v1.0, API and code base can be considered stable.

If you look closer at those changes you mention, there are a few bug fixes but most of those changes are about comment cleaning, code alignment, function/macro renaming and leftover unused code cleaning. No "active code" have been changed beyond bug fixes.

Let me remind you that this is the first time I work on a project of that size in collaboration and from that perspective, there is probably many things I still have to learn. I understand how confusing it is for you, when you see all those changes, even though they are simply "esthetic" changes... Would it be a better practice if I'd maintain a specific Ardour branch in libaaf ? So I could keep working in parallel without polluting Ardour's updates.

Sorry John, I hope we can improve our working process. And many thanks for your hard testing work !

agfline avatar Mar 20 '24 13:03 agfline

Many thanks Adrien and please let me clarify that I appreciate the efforts you're making with your test scripts and the move to UTF-8 and the extra work needed to improve fades etc. It's all great work but we need to find a way to fix occasional "show stoppers" without bringing in massive changes that no-one's tested.

Robin - do we have this problem with other 3rd-party contributions, such as lua, or have you figured out a solution in those cases?

johne53 avatar Mar 20 '24 13:03 johne53

Robin - do we have this problem with other 3rd-party contributions,

The only other 3rd party custom contribution that we have is libptfformat (pro-tools import). In this case damien is rather active and provides user support on IRC (he also curates Ardour's bundled plugins).

During initial development it was normal that various pro-tools sessions did not import correctly or were only imported partially. We just marked that feature as "experimental", or "beta" and collected bug reports. It took over a year until most kinks were sorted out (and fixes are still ongoing).

However, there was no dedicated person like you who did test with various pro-tools session, so bug-fixes were a lot slower and development was more incremental.

x42 avatar Mar 20 '24 19:03 x42

In some ways it was just bad luck that our first release with AAF got made at a time when the code was buggy. So given that Ardour doesn't support interim releases, I'm keen to ensure we don't make the same mistake for release 2 (and hence why I'm getting in a strop about adding untested changes...)

In some ways, Adrien's suggestion about an AAF branch makes sense but I remember from last time that its header files grew increasingly out of sync - meaning that his code became unbuildable here against Mixbus. And of course it doesn't help with the main problem being that Ardour's aaf needs to get updated occasionally and "en masse" rather than git being able to keep things properly in sync with Adrien's libaaf. AFAICT Damien's code also receives "en masse" updates but in his case, ptformat is very much simpler so it's maybe not quite so risky.

I'm starting to think the real solution here would be to abandon libs/aaf and start using the AAF code like any other 3rd party library, such as glib / pango etc? It'd help if Adrien could harmonize his programming style to match the others but even that's less important in a 3rd party lib.

[Edit...] Or how about this..? Adrien creates a special branch for us - but in his own repo, rather than in Ardour - and we then build against that one. We could then pull in from master or cherry-pick individual commits, as required (or maybe that's what Adrien meant in the first place?)

johne53 avatar Mar 21 '24 08:03 johne53

@johne53 it is preferable to keep a copy in Ardour's source tree for various reasons: The AAF API/ABI keeps changing, and updating libAAF also requires updating Ardour's source. Furthermore upstream does not have a cross-platform build system that works on all systems we use Ardour on, and various GNU/Linux distros have not packaged it either. Eventually we could also statically link it. All that being said, the wscript allows to use external libAAF already, which Adrien uses with a local build during development.

I also think you overestimate the resources we have. I would not put additional workload on Adrien to maintain a separate branch. We also won't cherry-pck individual commits to Ardour for the same reason. The idea is to stay in sync with upstream when they make a release.

So given that Ardour doesn't support interim releases

There are daily builds available, and we ask users who rely on a certain new feature to use those.

x42 avatar Mar 21 '24 14:03 x42