Prepare sdt loader for ffmpeg #522
Conversation
rwlib/source/loaders/LoaderSDT.hpp
Outdated
|
|
||
| /// Load the structure of the archive | ||
| /// Omit the extension in filename | ||
| bool load(const std::string& filename); | ||
| bool load(const rwfs::path& path); | ||
|
|
||
| /// Load a file from the archive to memory and pass a pointer to it | ||
| /// Warning: Please delete[] the memory in the end. |
|
@madebr has suggested on IRC to prevent linking FFmpeg to rwlib. I'll try something like this: sdt loader provides raw data and SoundManager deals with it. |
|
Updated, please review. ;) |
rwengine/src/audio/SoundManager.cpp
Outdated
| #endif | ||
|
|
||
| // Expose audio metadata | ||
| channels = codecContext->channels; |
There was a problem hiding this comment.
nit: using size_t for int. WaveHeader uses uint16_t while AVCodecContext uses int..
There was a problem hiding this comment.
Here you think about casting or changing some variable's type?
There was a problem hiding this comment.
I was thinking of casting. Tho, as uint16_t is being used in WaveHeader is size_t really needed.
rwlib/source/loaders/LoaderSDT.cpp
Outdated
| @@ -65,7 +43,7 @@ bool LoaderSDT::findAssetInfo(size_t index, LoaderSDTFile& out) { | |||
| return false; | |||
| } | |||
|
|
|||
| char* LoaderSDT::loadToMemory(size_t index, bool asWave) { | |||
| std::unique_ptr<char[]> LoaderSDT::loadToMemory(size_t index, bool asWave) { | |||
| LoaderSDTFile assetInfo; | |||
There was a problem hiding this comment.
assetInfo is also declared in header so is this intentional?
rwlib/source/loaders/LoaderSDT.cpp
Outdated
|
|
||
| WaveHeader* header = reinterpret_cast<WaveHeader*>(raw_data); | ||
| WaveHeader* header = reinterpret_cast<WaveHeader*>(raw_data.get()); |
|
Done. ;) |
rwengine/src/audio/SoundManager.cpp
Outdated
| auto ioBuffer = static_cast<uint8_t*>(av_malloc(ioBufferSize)); | ||
|
|
||
| /// Convert memory, in order to match required layout for ffmpeg | ||
| memcpy(input.ptr, raw_sound.get(), input.size); |
There was a problem hiding this comment.
Why does this need to be copied? Can't the memory for raw_sound be given to the input?
There was a problem hiding this comment.
I don't like pointer casting, but memory layout should match. I'll check.
| /// Alocate memory for buffer | ||
| /// Memory freeded at the end | ||
| static constexpr size_t ioBufferSize = 4096; | ||
| auto ioBuffer = static_cast<uint8_t*>(av_malloc(ioBufferSize)); |
There was a problem hiding this comment.
You can use unique_ptr with a custom deleter to handle specific dealloc functions from C APIs.
There was a problem hiding this comment.
I think it'll be confusing for reading.
Also, actually buffer is deleted before context, this change will flip order. FFmpeg really likes to change api, so I think it's better to be as close as possible to "template" use.
rwengine/src/audio/SoundManager.cpp
Outdated
| if (avformat_open_input(&formatContext, "nothint", nullptr, nullptr) != 0) { | ||
| av_frame_free(&frame); | ||
| RW_ERROR("Error opening audio file (" << index << ")"); | ||
| return; |
There was a problem hiding this comment.
Should formatContext->pb be freed in these early branches?
There was a problem hiding this comment.
Should be, I mean it needs rebase.
Function loadSound returns AVFormatContext with custom AVIOContext.
|
LGTM |
ghost
deleted the
ghost
mentioned this pull request
Extracted from #420.