agfline / LibAAF

Library for Advanced Authoring Format (AAF) file reading.
GNU General Public License v2.0
25 stars 5 forks source link

Ardour Support #5

Closed agfline closed 6 months ago

agfline commented 1 year ago

This thread continues the discussion previously started here

agfline commented 1 year ago

Thank you John for your help with git. I think I finally managed to have a local fork of the Ardour/aaf branch that is not detached, and which can be synced with Ardour/aaf. It's in ardour_aaf_support/aaf, note that the aaf branch is now set as default. I successfully did a bunch of commits last night.

each time I do another 'fetch' here, git rebase keeps placing those same entries at the top of the list - which suggests that they no longer exist in the official repo

I think it is due to my struggling with git and the ardour_aaf_support repos, I have also seen weird stuff at some point. Have you tried to start a brand new clone of the ardour_aaf_support/aaf branch ?

johne53 commented 1 year ago

Thanks for the quick response Adrien. I guess I could ditch my existing repo and clone it all over again but it's taken me the best part of 2 days to make 'adour_aaf_support buildable with MSVC - so I'd prefer to leave that as a last resort. In the first instance I'll try switching to the AAF branch. In theory, that should carry on working here and hopefully it'll accept your future updates.

johne53 commented 1 year ago

I'll try switching to the AAF branch. In theory, that should carry on working here

Yes, that's building fine here (though it'll be a few days before I get around to building the aaf branch that Robin's added...)

In the meantime @agfline, did you manage to reproduce those problems I reported relating to 100_BARS.aaf? It'd be interesting to know if they're reproducible with your (Linux?) code - or if they're only showing up in Windows.

agfline commented 1 year ago

In the meantime @agfline, did you manage to reproduce those problems I reported relating to 100_BARS.aaf?

Indeed, I got the 0 Hz / 16398 bits issue on Linux too. This is the case for all AAF files containing embedded essences formatted in AIFC, I still have to investigate though.

agfline commented 1 year ago

Got it... There's a missing __attribute__((packed)) ;-)

johne53 commented 1 year ago

Great news! I'll test it here later today and let you know.

johne53 commented 1 year ago

Hmmm... I tried 100_BARS.aaf here and it no longer shows me the strange sample rate and bit depth. It even creates a .ardour session now - BUT - I'm seeing exceptions now at line 212 in aafi_release() (in AAFIFace.c ) :-

if ( (*aafi)->ctx.options.media_location ) {
    free( (*aafi)->ctx.options.media_location ); // <--- CRASHES HERE !!!!
}

and when I later try to load the generated .ardour session, it still contains no audio :-(

agfline commented 1 year ago

Ok. No time to fix it before tonight, but meanwhile you can try to add --media-location /path/to/wherever/you/want/ to new_aaf_session

johne53 commented 1 year ago

Hi Adrien - I added a --media-location path and just for the hell of it, I configured my debugger to skip the crashing line temporarily. But although it then didn't crash, the created session still doesn't contain any audio when loaded into Ardour. Basically, media files still aren't getting created at the moment.

agfline commented 1 year ago

Did you set --media-cache ? Can you show the console output ?

johne53 commented 1 year ago

No I didn't set --media-cache, only --media-location (I'd never needed to set either of them up to now...)

I'm just closing down here for the night but here's the output i see when the problem occurs:-

 Composition Name       : 100_BARS
 Composition Start      : 23814000
 Composition End        : 33521292
 Composition SampleRate : 44100 Hz
 Composition SampleSize : 16 bits

[i] F:\+GTK-SOURCES\ardour_aaf_support\session_utils\new_aaf_session.cc : main() on line 989 : Using AAF file sample rate : 44100 Hz
[i] F:\+GTK-SOURCES\ardour_aaf_support\session_utils\new_aaf_session.cc : main() on line 997 : Using AAF file bit depth : 16 bits
[i] F:\+GTK-SOURCES\ardour_aaf_support\session_utils\new_aaf_session.cc : main() on line 1021 : Using AAF composition name for Ardour session name : 100_BARS
ARDOUR_CONFIG_PATH not set in environment
ARDOUR_DATA_PATH not set in environment
*** WEAK-JACK: initializing
*** WEAK-JACK: OK. (0)
locate to 0 took 527 usecs for 1 tracks = 527 per track
[e] F:\+GTK-SOURCES\ardour_aaf_support\session_utils\new_aaf_session.cc : main() on line 1112 : Could not extract audio file from AAF : media cache was not set.
[e] F:\+GTK-SOURCES\ardour_aaf_support\session_utils\new_aaf_session.cc : main() on line 1112 : Could not extract audio file from AAF : media cache was not set.
: [ERROR]: Session start(23814000)/end(33521292))
locate to 0 took 204 usecs for 1 tracks = 204 per track
Butler drops pool trash
Graph::drop_threads() sema-counts: 0, 0, 1
agfline commented 1 year ago

ARDOUR_CONFIG_PATH not set in environment ARDOUR_DATA_PATH not set in environment

Those are Ardour messages I don't have with Linux. Don't know if they are important or not.

[e] F:+GTK-SOURCES\ardour_aaf_support\session_utils\new_aaf_session.cc : main() on line 1112 : Could not extract audio file from AAF : media cache was not set.

Explicitly says media cache was not set. Before I made the --media-cache parameter, the extract location of audio files was hard coded /tmp/ which doesn't work on windows. I updated --help accordingly, but I should have told you.

Actually when you are working with embedded files you have to set --media-cache and when you are working with external media files, you can set --media-location if the library can't locate the files on its own.

johne53 commented 1 year ago

ARDOUR_CONFIG_PATH not set in environment ARDOUR_DATA_PATH not set in environment

Those are Ardour messages I don't have with Linux. Don't know if they are important or not.

I'm pretty sure they're not.

Explicitly says media cache was not set. Before I made the --media-cache parameter, the extract location of audio files was hard coded /tmp/ which doesn't work on windows.

It seemed to work for me - but anyway... I've now set --media-cache to be F:/ here and it does produce an audio file called F:/100_BARS _1.wav - although it hasn't helped much. I see a problem now in the first two lines of import_sndfile_as_region(), namely:-

wstring ws(audioEssence->usable_file_path);     // <--- reports "Error reading characters of string."
string usable_file_path(ws.begin(), ws.end());  // <--- 'usable_file_path' is now set as "F/10BR 1wv¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦"

and needless to say, the code crashes almost immediately :-(

My guess would be a possible problem with character encoding. Maybe you're assuming that strings here will either be narrow character or wide character? But if you're obtaining those strings from the Ardour code, they'll almost certainly be getting returned in UTF-8 format.

johne53 commented 1 year ago

ARDOUR_CONFIG_PATH not set in environment I've now set --media-cache to be F:/ here and it does produce an audio file called F:/100_BARS _1.wav - although it hasn't helped much.

string usable_file_path(ws.begin(), ws.end());  // <--- 'usable_file_path' is now set as "**F/10BR 1wv*<etc>¦

It didn't occur to me yesterday but this morning I realised something:-

F/10BR 1wv is in fact every alternate letter from F:/100_BARS _1.wav (and with the NULL terminator missing)

So maybe not a problem with UTF-8 but surely this is some kinda problem with character encoding??

johne53 commented 1 year ago

And something else just occurred to me...

the extract location of audio files was hard coded /tmp/ which doesn't work on windows

By a stroke of luck I'd chosen F:/ as my extract location and I just noticed a huge bunch of spurious .wav files from the past few weeks (which I didn't put there...)

So it's looking like the code wasn't set to the user's temp folder on Windows - but in fact it'd been trying to use the root folder of whichever drive the sessions were getting created on. And if that drive happened to be the user's C:\ drive, that'd definitely explain why the software wasn't being allowed to write its .wav files there.

agfline commented 1 year ago

Indeed, that makes sense.

The --media-cache parameter will eventually be removed. A good practice will be to extract audio files to some system-dependent temp folder, and delete them once they are imported into Ardour session. Still, I think I'll make a dont-clear-cache parameter so one can investigate exported files if needed.

wstring ws(audioEssence->usable_file_path); // <--- reports "Error reading characters of string." string usable_file_path(ws.begin(), ws.end()); // <--- 'usable_file_path' is now set as "F/10BR 1wv¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦"

I'm going to investigate this today.

agfline commented 1 year ago

Since I don't know how to build Ardour on windows, I just made a small Cpp test program to reproduce your issue on windows, testing against the same 100_BARS.aaf file. I can't reproduce, neither Error reading characters of string nor the weird name F/10BR 1wv. Are you working with the latest libAAF / new_aaf_session.cc ?

johne53 commented 1 year ago

Yes, in fact I updated it again this morning and I still see the problem.

Whereabouts does audioEssence->usable_file_path get assigned? I'll try putting a breakpoint there and see if the assignment looks okay.

agfline commented 1 year ago

In your case (embedded audio file extraction) : https://github.com/agfline/LibAAF/blob/3bacf0f772f6de6a5bb5e80322d352f6ce8b0038/src/AAFIface/AAFIAudioFiles.c#L1623

johne53 commented 1 year ago

Okay, I can see a couple of problems. Firstly, swprintf() expects the number of characters to print, not the number of bytes. So you don't need to multiply by sizeof(wchar_t))

And this surprised me a bit but L"%s", filePath doesn't seem to produce a wide-character string here. You'd think it would but it didn't when I tested it. In the end, I made it work by using:-

mbstowcs( audioEssence->usable_file_path, filePath, strlen(filePath) );

That corrected this particular issue but I then hit a new set of errors:-

: [ERROR]: FFMPEGFileImportableSource: Failed to read file metadata
: [ERROR]: Import: cannot open input sound file "100_BARS _1.wav"
  [e] F:\+GTK-SOURCES\ardour_aaf_support\session_utils\new_aaf_session.cc : main() on line 1149 : Could not import '100 BARS _1' to session.

and Windows tells me that 100 BARS _1.wav is an invalid WAV file. I'm really quite surprised that any of this works for you !

agfline commented 1 year ago

swprintf() expects the number of characters to print, not the number of bytes. So you don't need to multiply by sizeof(wchar_t))

Thanks for pointing that out.

L"%s", filePath doesn't seem to produce a wide-character string here

Well, it does actually work on my Linux and Windows VM. I didn't know about mbstowcs() and since it works for you, I'll switch to that function instead, no problem.

100 BARS _1.wav is an invalid WAV file

Can you send me that wav file so I take a look ?

I'm really quite surprised that any of this works for you !

And yet, it does work pretty well ! And I hope it will be soon working for you as well ;)

johne53 commented 1 year ago

Yes, give me a few minutes and I'll send you 100 BARS _1.wav. In the meantime I found this article which explains why L"%s" doesn't work:-

https://learn.microsoft.com/en-us/cpp/c-runtime-library/format-specification-syntax-printf-and-wprintf-functions?view=msvc-170

it seems that for the wprintf range of functions, L"%S" would need to use an upper-case 'S' to accommodate 'filePath' being a narrow character string.

We're slowly getting there :-)

agfline commented 1 year ago

Just made an hash comparison between your file and mine and... they are exactly the same.

There are multiple ways to embed audio in AAF. In 100_BARS.aaf, each audio file is stored as a complete AIFC file. In such case, the extraction process is simply to take the file data stream, and to write it somewhere on your disk.

There is still an issue with file extension being .wav, but I highly doubt ffmpeg is yelling about that.

: [ERROR]: FFMPEGFileImportableSource: Failed to read file metadata : [ERROR]: Import: cannot open input sound file "100_BARS _1.wav"

Wouldn't be some reading persmission/access issue ? Maybe some Ardour Guru have an idea ?

johne53 commented 1 year ago

I suspect it's yelling about the fact that it's not a WAV file. For every other WAV file here (including your older ones) the first 12 bytes invariably look like this:-

RIFF<4 bytes - possible length?>WAVE

But your newer ones don't have that section any more - so maybe there's some other header info missing too ?

agfline commented 1 year ago

Like I said, it's not a WAV file, it's an AIFF/AIFC file with .wav file extension. Even though file extension should be fixed to .aiff, the file should be well parsed by any player or ffmpeg. Not to mention the exact same file gets imported into Ardour on Linux.

agfline commented 1 year ago

it seems that for the wprintf range of functions, L"%S" would need to use an upper-case 'S' to accommodate 'filePath' being a narrow character string.

I just made a change to test that out. I have a lot of other uses of swprintf() where mbstowcs() wont be an option, so when you have time can you confirm it's working ?

Also fixed the .aif file extension, although I doubt it fixes your issue.

johne53 commented 1 year ago

Yes, both those fixes have worked and interestingly, the session's audiofiles folder contains audio files in wav format (like it did originally). I've only tested that one session so I'll need to check some others here later - but in the meantime...

With regard to the upper-case L"%S" would you mind checking your code in case there might be other places where the same change is needed? Thanks...

agfline commented 1 year ago

From my understanding Ardour is doing the conversion on import, so it ends up with only wav files in session folder, no matter the input file format.

With regard to the upper-case L"%S" would you mind checking your code in case there might be other places where the same change is needed?

That's precisely why I asked you for confirmation, because there are a lot of swprintf() waiting to be fixed ;)

johne53 commented 1 year ago

Okay, I've tried some more imports here but the only files that'll import are my 100_BARS aaf and your Tiny_Resolve aaf. Everything else fails. AFAICT the only thing they have in common is that they contain just 1 or 2 audio clips.

[Edit...] I just noticed that some of the file paths are containing a spurious double-quote mark which shouldn't be there...

__ERROR ..\..\..\src\AAFIface\AAFIAudioFiles.c:1510 in aafi_extract_audio_essence() : Could not open audio essence file for writing (F:"\Loop_03-02.L.wav) : Invalid argument
[e] F:\+GTK-SOURCES\ardour_aaf_support\session_utils\new_aaf_session.cc : main() on line 1136 : Could not extract audio file 'Loop_03-02.L' from AAF.
__ERROR ..\..\..\src\AAFIface\AAFIAudioFiles.c:1510 in aafi_extract_audio_essence() : Could not open audio essence file for writing (F:"\Sample accurate edit.wav) : Invalid argument
[e] F:\+GTK-SOURCES\ardour_aaf_support\session_utils\new_aaf_session.cc : main() on line 1136 : Could not extract audio file 'Sample accurate edit' from AAF.
__ERROR ..\..\..\src\AAFIface\AAFIAudioFiles.c:1510 in aafi_extract_audio_essence() : Could not open audio essence file for writing (F:"\Loop_03-01.L.wav) : Invalid argument
[e] F:\+GTK-SOURCES\ardour_aaf_support\session_utils\new_aaf_session.cc : main() on line 1136 : Could not extract audio file 'Loop_03-01.L' from AAF.

Notice that the file paths all start with F:"\ when it should be F:\

Maybe I should wait until you've examined those other L"%s" occurrences and see if any others need changing.

[Edit 2...] I found out why it happens. I haven't tracked it down to the code but it happens if I enclose my media-cache path in quotation marks - i.e. if I use--media-cache "F:/" but with a backslash instead of a forward slash (for some reason, this site won't display backslashes sometimes !!)

I could fix it by removing the quote marks but then (presumably) I wouldn't be able to use a path name with a space in it ?

agfline commented 1 year ago

Just fixed the swprintf() string format identifiers.

I could fix it by removing the quote marks but then (presumably) I wouldn't be able to use a path name with a space in it ?

This seems more related to the windows console environment, which I really just don't know. Just found this online : https://www.howtogeek.com/694949/how-to-escape-spaces-in-file-paths-on-the-windows-command-line/

johne53 commented 1 year ago

If you're planning to remove --media-cache at some point, I guess that'll solve the problem anyway. But in the meantime, would you mind importing a few AAF files at your end. I noticed that for new_aaf_session.cc commit #1f0f49c77d12 says "fix wrong region positioning".

But what I'm finding now (here) is that for every imported timeline region (on any track) they now all start at exactly the same timecode :-(

agfline commented 1 year ago

But what I'm finding now (here) is that for every imported timeline region (on any track) they now all start at exactly the same timecode :-(

I don't understand what you mean ? Here is what I see when I create a session from Resolve 18.5.aaf and using --media-location LibAAF/test/res/

screenshot3

All clips positioning and duration match the original Resolve timeline. Notice the session start at 01:00:00:00

johne53 commented 1 year ago

Thanks Adrien - I managed to download Resolve 18.5.aaf but for whatever reason, it wouldn't allow me to access the actual audio files. But would you mind trying again with some embedded AAF files when you've some time (all my test files here use embedded media).

Here's what I'm seeing with a small embedded AAF called AX-TEST.aaf:-

Capture-11

Notice how every timeline region has been re-positioned to start at the same time as the Start marker - but here's how it always used to look:-

Capture-12

I'm seeing the same issue here with any embedded AAF.

agfline commented 1 year ago

Is it possible for you to send me one of your files ? What software did you use to make them ?

johne53 commented 1 year ago

That particular one came from Avid Media Composer (though admittedly it's quite an old file...) Most of my test files would have come from the BBC or other TV companies.

It's one of the smaller files here but still around 22MB - so too big to attach to an email. If there's somewhere I could upload it for you, just email me a link and I'll be happy to try.

The problem here is that most of my test AAF's are huge files of between 300MB and 1GB - so not even easily uploadable :-1:

agfline commented 1 year ago

From what I have seen, Avid Media Composer has one of the most complete implementation of AAF (Avid is pretty involved in AAF development). So depending on the Avid export settings, you can end up with quite complex AAF file structures.

Sadly, I ain't got no server on which you could upload your files. Maybe some kind of wetransfer could do it for the smallest ones ? Don't know the file size limit on github, but maybe you could make a repos ?

In the meantime, you still can run AAFInfo.exe --trace --trace-meta --aaf-summary --aaf-clips and post the output here, so I can see a bit about the internal file structure, and if clips position are correctly retrieved from the file.

Do you have and idea about how the original timeline should look like ? If you don't, Davinci Resolve is free and has a not-so-terrible AAF file support. It could be used to open AAF files for reference.

johne53 commented 1 year ago

Hi Adrien - the original timeline looked like the 2nd image I posted, above.

I used to have a Dropbox account here but Dropbox and Visual Studio were constantly fighting with each other so I had to drop Dropbox.

I'll email the AAFInfo printout you asked for

agfline commented 1 year ago

Thanks, I think I've found what's going on.

johne53 commented 1 year ago

This morning I found my original import of AX-TEST in a very early version of Mixbus - and interestingly, it doesn't look anything like the above images (in fact it looks like yours might've merged tracks 1+2 with tracks 8+9 somehow...). So here's the layout you should probably be aiming for:-

Capture-13

AX-TEST.aaf is pretty small (around 20MB) so I could probably upload it to my Microsoft OneDrive account which will allow me to share it with you (I've never used it for sharing but I don't think you'd also need OneDrive...)

agfline commented 1 year ago

Should be fixed now.

johne53 commented 1 year ago

Okay, we're back to your first layout now but here's what I've noticed, compared to the intended layout:-

1) The first region on tracks 1+2 look like they're the regions which should be on tracks 8+9 (see above)
2) The region on track 5 should be on track 6 (i.e. track 5 was empty in the original session)

Let me know if you want a link for downloading AX-TEST.aaf - but I'd be happy for you to take a few days rest (you must've been pretty busy this past week !!)

Capture-15

agfline commented 1 year ago

Thank you John, I take time to rest when I need to, don't worry about it :)

I'll be happy to work on your file when you can send me your link. Especially because this seems more like a support issue than an actual bug, and I'm quite curious to investigate on this !

johne53 commented 1 year ago

Okay Adrien, I've set up a OneDrive Share for you - but this was the first time I've set one up and I'm not sure if it'll allow you to download the file or only view it :-(

Hopefully you'll receive an email so let me know if it's okay, John

agfline commented 1 year ago

It's all good, thanks John !

agfline commented 1 year ago

Clip positioning should be fixed now. There is still an issue with 2 clips gain, some timecode mobslots / pulldown objects to investigate, and both of which are causing a small memory leak.

johne53 commented 1 year ago

Yes, that's looking a lot better now though unfortunately, there's been a regression for 100_BARS.aaf which is now showing a couple of things at the wrong timecode. Did I ever send you a link for downloading that one?

agfline commented 1 year ago

Everything should be fixed now. I also added support for regular multi-channel clips (you can test with Resolve 18.5.aaf).

Next :

Improvements :

All of those are already implemented in libAAF. I just need to find out how to implement with Ardour code, any help will be appreciated ;)

x42 commented 1 year ago
  • Clip-based volume automation (if Ardour supports it ?)

It is called called "region gain envelope": After creating an audio-region you can access it via std::shared_ptr<AutomationList> al = audio_region->envelope(); and then set it al->fast_simple_add (when, value) , when is relative to the region's start, and the value is a gain coefficient.

  • Muted clips

region->set_muted (bool);

  • Markers (label, comment, color)

Ardour only supports a label.

Location::Flags flags = Location::Flags (extra_flags|Location::IsMark);
Location *location = new Location (*_session, where, where, markername, flags);
_session->locations()->add (location, true);
  • Make Ardour session start at some timecode offset

    maybe_update_session_range (timepos_t (start_sample), timepos_t (end_sample_));
  • Video file import

Video is an Ardour GUI feature, there is no direct libardour support. State is set via add_extra_xml search gtk2_ardour/ardour_ui_video.cc for for "Videotimeline"

agfline commented 1 year ago

Thank you Robin, that's very helpful !

agfline commented 1 year ago

Regarding maybe_update_session_range (timepos_t (start_sample), timepos_t (end_sample_)); I already call set_session_extents() to set the session start and end. What I'd like to do is set the timeline start time. For now, when an AAF starts at 01:00:00:00, the session is positioned 1h away on the timeline. I'd like to make an option so the timeline does not start at 00:00:00:00 but at 01:00:00:00, or at any time the AAF is set to.