From c77f00903883ea0b0c39b1608f19da28ce7f7191 Mon Sep 17 00:00:00 2001 From: "FeRD (Frank Dana)" Date: Sun, 4 Aug 2019 16:06:54 -0400 Subject: [PATCH 1/6] Remove "dummy" args from ZmqLogger stragglers I somehow missed a few calls, in #266. --- src/AudioReaderSource.cpp | 4 ++-- src/Clip.cpp | 4 ++-- src/FFmpegWriter.cpp | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/AudioReaderSource.cpp b/src/AudioReaderSource.cpp index 4c42d2ed..8195d03b 100644 --- a/src/AudioReaderSource.cpp +++ b/src/AudioReaderSource.cpp @@ -66,7 +66,7 @@ void AudioReaderSource::GetMoreSamplesFromReader() } // Debug - ZmqLogger::Instance()->AppendDebugMethod("AudioReaderSource::GetMoreSamplesFromReader", "amount_needed", amount_needed, "amount_remaining", amount_remaining, "", -1, "", -1, "", -1, "", -1); + ZmqLogger::Instance()->AppendDebugMethod("AudioReaderSource::GetMoreSamplesFromReader", "amount_needed", amount_needed, "amount_remaining", amount_remaining); // Init estimated buffer equal to the current frame position (before getting more samples) estimated_frame = frame_number; @@ -149,7 +149,7 @@ juce::AudioSampleBuffer* AudioReaderSource::reverse_buffer(juce::AudioSampleBuff int channels = buffer->getNumChannels(); // Debug - ZmqLogger::Instance()->AppendDebugMethod("AudioReaderSource::reverse_buffer", "number_of_samples", number_of_samples, "channels", channels, "", -1, "", -1, "", -1, "", -1); + ZmqLogger::Instance()->AppendDebugMethod("AudioReaderSource::reverse_buffer", "number_of_samples", number_of_samples, "channels", channels); // Reverse array (create new buffer to hold the reversed version) AudioSampleBuffer *reversed = new juce::AudioSampleBuffer(channels, number_of_samples); diff --git a/src/Clip.cpp b/src/Clip.cpp index 653cb667..3bf6030d 100644 --- a/src/Clip.cpp +++ b/src/Clip.cpp @@ -616,7 +616,7 @@ std::shared_ptr Clip::GetOrCreateFrame(int64_t number) try { // Debug output - ZmqLogger::Instance()->AppendDebugMethod("Clip::GetOrCreateFrame (from reader)", "number", number, "samples_in_frame", samples_in_frame, "", -1, "", -1, "", -1, "", -1); + ZmqLogger::Instance()->AppendDebugMethod("Clip::GetOrCreateFrame (from reader)", "number", number, "samples_in_frame", samples_in_frame); // Attempt to get a frame (but this could fail if a reader has just been closed) new_frame = reader->GetFrame(number); @@ -634,7 +634,7 @@ std::shared_ptr Clip::GetOrCreateFrame(int64_t number) } // Debug output - ZmqLogger::Instance()->AppendDebugMethod("Clip::GetOrCreateFrame (create blank)", "number", number, "samples_in_frame", samples_in_frame, "", -1, "", -1, "", -1, "", -1); + ZmqLogger::Instance()->AppendDebugMethod("Clip::GetOrCreateFrame (create blank)", "number", number, "samples_in_frame", samples_in_frame); // Create blank frame new_frame = std::make_shared(number, reader->info.width, reader->info.height, "#000000", samples_in_frame, reader->info.channels); diff --git a/src/FFmpegWriter.cpp b/src/FFmpegWriter.cpp index 37f3016a..20940455 100644 --- a/src/FFmpegWriter.cpp +++ b/src/FFmpegWriter.cpp @@ -1389,7 +1389,7 @@ void FFmpegWriter::open_video(AVFormatContext *oc, AVStream *st) { av_dict_set(&st->metadata, iter->first.c_str(), iter->second.c_str(), 0); } - ZmqLogger::Instance()->AppendDebugMethod("FFmpegWriter::open_video", "video_codec->thread_count", video_codec->thread_count, "", -1, "", -1, "", -1, "", -1, "", -1); + ZmqLogger::Instance()->AppendDebugMethod("FFmpegWriter::open_video", "video_codec->thread_count", video_codec->thread_count); } From fc764622926c3cdc223c9ef25888813f2e7fdb58 Mon Sep 17 00:00:00 2001 From: "FeRD (Frank Dana)" Date: Mon, 5 Aug 2019 20:19:50 -0400 Subject: [PATCH 2/6] Update table in HW-ACCEL.md for Doxygen MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit By using actual Unicode characters for the various ✔️ and ❌ marks in the table, Doxygen will format it properly. Also: - Changed to superscripted numerals, for the various callouts - Reformatted notes into a corresponding ordered list - Added invisible spaces after the marks without a callout, to balance centering - Added center-alignment markers to appropriate table columns --- doc/HW-ACCEL.md | 36 +++++++++++++++++++----------------- 1 file changed, 19 insertions(+), 17 deletions(-) diff --git a/doc/HW-ACCEL.md b/doc/HW-ACCEL.md index b8ee7b4e..cbcf5e63 100644 --- a/doc/HW-ACCEL.md +++ b/doc/HW-ACCEL.md @@ -7,20 +7,22 @@ our support for this in the future! The following table summarizes our current level of support: -| | Linux Decode | Linux Encode | Mac Decode | Mac Encode |Windows Decode| Windows Encode | Notes | -|--------------------|------------------------|----------------------|------------------|----------------|--------------|------------------|------------------| -| VA-API | :heavy_check_mark: | :heavy_check_mark: | - | - | - | - | *Linux Only* | -| VDPAU | :heavy_check_mark:(+) |:white_check_mark:(++)| - | - | - | - | *Linux Only* | -| CUDA (NVDEC/NVENC) | :x:(+++) | :heavy_check_mark: | - | - | - |:heavy_check_mark:| *Cross Platform* | -| VideoToolBox | - | - |:heavy_check_mark:| :x:(++++) | - | - | *Mac Only* | -| DXVA2 | - | - | - | - | :x:(+++) | - | *Windows Only* | -| D3D11VA | - | - | - | - | :x:(+++) | - | *Windows Only* | -| QSV | :x:(+++) | :x: | :x: | :x: | :x: | :x: | *Cross Platform* | +| | Linux Decode | Linux Encode | Mac Decode | Mac Encode | Windows Decode | Windows Encode | Notes | +|--------------------|:---------------:|:--------------:|:----------:|:--------------:|:--------------:|:--------------:|------------------| +| VA-API | ✔️   | ✔️   | - | - | - | - | *Linux Only* | +| VDPAU | ✔️ 1 | ✅ 2 | - | - | - | - | *Linux Only* | +| CUDA (NVDEC/NVENC) | ❌ 3 | ✔️   | - | - | - | ✔️   | *Cross Platform* | +| VideoToolBox | - | - | ✔️   | ❌ 4 | - | - | *Mac Only* | +| DXVA2 | - | - | - | - | ❌ 3 | - | *Windows Only* | +| D3D11VA | - | - | - | - | ❌ 3 | - | *Windows Only* | +| QSV | ❌ 3 | ❌   | ❌   | ❌   | ❌   | ❌   | *Cross Platform* | -* *(+) VDPAU for some reason needs a card number one higher than it really is* -* *(++) VDPAU is a decoder only.* -* *(+++) Green frames (pixel data not correctly tranferred back to system memory)* -* *(++++) Crashes and burns* +#### Notes + +1. VDPAU for some reason needs a card number one higher than it really is +2. VDPAU is a decoder only +3. Green frames (pixel data not correctly tranferred back to system memory) +4. Crashes and burns ## Supported FFmpeg Versions @@ -37,7 +39,7 @@ included. The following settings are use by libopenshot to enable, disable, and control the various hardware acceleration features. -``` +```{cpp} /// Use video codec for faster video decoding (if supported) int HARDWARE_DECODER = 0; @@ -76,9 +78,9 @@ in Ubuntu 18.04) for the AppImage to work with hardware acceleration. An AppImage that works on both systems (supporting libva and libva2), might be possible when no libva is included in the AppImage. -* vaapi is working for intel and AMD -* vaapi is working for decode only for nouveau -* nVidia driver is working for export only +* vaapi is working for intel and AMD +* vaapi is working for decode only for nouveau +* nVidia driver is working for export only ## AMD Graphics Cards (RadeonOpenCompute/ROCm) From 5fb9755353d2dc6fe5779c7fd4b22a2760d3b6e4 Mon Sep 17 00:00:00 2001 From: "FeRD (Frank Dana)" Date: Tue, 6 Aug 2019 11:59:55 -0400 Subject: [PATCH 3/6] Fix truncated output filenames in FFmpegWriter --- include/FFmpegUtilities.h | 4 ++++ src/FFmpegWriter.cpp | 5 ++--- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/include/FFmpegUtilities.h b/include/FFmpegUtilities.h index 8a0b7705..0ccf1ca1 100644 --- a/include/FFmpegUtilities.h +++ b/include/FFmpegUtilities.h @@ -149,6 +149,7 @@ #define AV_REGISTER_ALL #define AVCODEC_REGISTER_ALL #define AV_FILENAME url + #define AV_SET_FILENAME(oc, f) oc->AV_FILENAME = av_strdup(f) #define MY_INPUT_BUFFER_PADDING_SIZE AV_INPUT_BUFFER_PADDING_SIZE #define AV_ALLOCATE_FRAME() av_frame_alloc() #define AV_ALLOCATE_IMAGE(av_frame, pix_fmt, width, height) av_image_alloc(av_frame->data, av_frame->linesize, width, height, pix_fmt, 1) @@ -184,6 +185,7 @@ #define AV_REGISTER_ALL av_register_all(); #define AVCODEC_REGISTER_ALL avcodec_register_all(); #define AV_FILENAME filename + #define AV_SET_FILENAME(oc, f) snprintf(oc->AV_FILENAME, sizeof(oc->AV_FILENAME), "%s", f) #define MY_INPUT_BUFFER_PADDING_SIZE FF_INPUT_BUFFER_PADDING_SIZE #define AV_ALLOCATE_FRAME() av_frame_alloc() #define AV_ALLOCATE_IMAGE(av_frame, pix_fmt, width, height) av_image_alloc(av_frame->data, av_frame->linesize, width, height, pix_fmt, 1) @@ -222,6 +224,7 @@ #define AV_REGISTER_ALL av_register_all(); #define AVCODEC_REGISTER_ALL avcodec_register_all(); #define AV_FILENAME filename + #define AV_SET_FILENAME(oc, f) snprintf(oc->AV_FILENAME, sizeof(oc->AV_FILENAME), "%s", f) #define MY_INPUT_BUFFER_PADDING_SIZE FF_INPUT_BUFFER_PADDING_SIZE #define AV_ALLOCATE_FRAME() av_frame_alloc() #define AV_ALLOCATE_IMAGE(av_frame, pix_fmt, width, height) avpicture_alloc((AVPicture *) av_frame, pix_fmt, width, height) @@ -252,6 +255,7 @@ #define AV_REGISTER_ALL av_register_all(); #define AVCODEC_REGISTER_ALL avcodec_register_all(); #define AV_FILENAME filename + #define AV_SET_FILENAME(oc, f) snprintf(oc->AV_FILENAME, sizeof(oc->AV_FILENAME), "%s", f) #define MY_INPUT_BUFFER_PADDING_SIZE FF_INPUT_BUFFER_PADDING_SIZE #define AV_ALLOCATE_FRAME() avcodec_alloc_frame() #define AV_ALLOCATE_IMAGE(av_frame, pix_fmt, width, height) avpicture_alloc((AVPicture *) av_frame, pix_fmt, width, height) diff --git a/src/FFmpegWriter.cpp b/src/FFmpegWriter.cpp index 37f3016a..9d77746d 100644 --- a/src/FFmpegWriter.cpp +++ b/src/FFmpegWriter.cpp @@ -525,9 +525,7 @@ void FFmpegWriter::WriteHeader() { } // Force the output filename (which doesn't always happen for some reason) - snprintf(oc->AV_FILENAME, sizeof(oc->AV_FILENAME), "%s", path.c_str()); - - // Write the stream header, if any + AV_SET_FILENAME(oc, path.c_str()); // Add general metadata (if any) for (std::map::iterator iter = info.metadata.begin(); iter != info.metadata.end(); ++iter) { @@ -543,6 +541,7 @@ void FFmpegWriter::WriteHeader() { if (is_mp4 || is_mov) av_dict_copy(&dict, mux_dict, 0); + // Write the stream header if (avformat_write_header(oc, &dict) != 0) { throw InvalidFile("Could not write header to file.", path); }; From 59fe41714100fac3058343b8dddbc67748b1d850 Mon Sep 17 00:00:00 2001 From: SuslikV Date: Thu, 8 Aug 2019 16:46:07 +0300 Subject: [PATCH 4/6] Unify indentation of the code strings --- include/FFmpegUtilities.h | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/include/FFmpegUtilities.h b/include/FFmpegUtilities.h index 0ccf1ca1..c673305e 100644 --- a/include/FFmpegUtilities.h +++ b/include/FFmpegUtilities.h @@ -105,13 +105,13 @@ // Define this for compatibility #ifndef PixelFormat #define PixelFormat AVPixelFormat - #endif + #endif #ifndef PIX_FMT_RGBA #define PIX_FMT_RGBA AV_PIX_FMT_RGBA - #endif + #endif #ifndef PIX_FMT_NONE #define PIX_FMT_NONE AV_PIX_FMT_NONE - #endif + #endif #ifndef PIX_FMT_RGB24 #define PIX_FMT_RGB24 AV_PIX_FMT_RGB24 #endif @@ -154,7 +154,7 @@ #define AV_ALLOCATE_FRAME() av_frame_alloc() #define AV_ALLOCATE_IMAGE(av_frame, pix_fmt, width, height) av_image_alloc(av_frame->data, av_frame->linesize, width, height, pix_fmt, 1) #define AV_RESET_FRAME(av_frame) av_frame_unref(av_frame) - #define AV_FREE_FRAME(av_frame) av_frame_free(av_frame) + #define AV_FREE_FRAME(av_frame) av_frame_free(av_frame) #define AV_FREE_PACKET(av_packet) av_packet_unref(av_packet) #define AV_FREE_CONTEXT(av_context) avcodec_free_context(&av_context) #define AV_GET_CODEC_TYPE(av_stream) av_stream->codecpar->codec_type @@ -190,7 +190,7 @@ #define AV_ALLOCATE_FRAME() av_frame_alloc() #define AV_ALLOCATE_IMAGE(av_frame, pix_fmt, width, height) av_image_alloc(av_frame->data, av_frame->linesize, width, height, pix_fmt, 1) #define AV_RESET_FRAME(av_frame) av_frame_unref(av_frame) - #define AV_FREE_FRAME(av_frame) av_frame_free(av_frame) + #define AV_FREE_FRAME(av_frame) av_frame_free(av_frame) #define AV_FREE_PACKET(av_packet) av_packet_unref(av_packet) #define AV_FREE_CONTEXT(av_context) avcodec_free_context(&av_context) #define AV_GET_CODEC_TYPE(av_stream) av_stream->codecpar->codec_type @@ -229,7 +229,7 @@ #define AV_ALLOCATE_FRAME() av_frame_alloc() #define AV_ALLOCATE_IMAGE(av_frame, pix_fmt, width, height) avpicture_alloc((AVPicture *) av_frame, pix_fmt, width, height) #define AV_RESET_FRAME(av_frame) av_frame_unref(av_frame) - #define AV_FREE_FRAME(av_frame) av_frame_free(av_frame) + #define AV_FREE_FRAME(av_frame) av_frame_free(av_frame) #define AV_FREE_PACKET(av_packet) av_packet_unref(av_packet) #define AV_FREE_CONTEXT(av_context) avcodec_close(av_context) #define AV_GET_CODEC_TYPE(av_stream) av_stream->codec->codec_type From dfbcb4730bbea7673e4099dcf8d18a2366b50e2a Mon Sep 17 00:00:00 2001 From: SuslikV Date: Mon, 12 Aug 2019 14:09:26 +0300 Subject: [PATCH 5/6] Disable debug logger on close The logger can be closed from the external thread (openshot-qt), while same logger instance is still in use in libopenshot. This change prevents crash on attempts to use debug logger if it was closed. Co-authored-by: Frank Dana --- src/ZmqLogger.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/ZmqLogger.cpp b/src/ZmqLogger.cpp index 0aeeab22..89d2798a 100644 --- a/src/ZmqLogger.cpp +++ b/src/ZmqLogger.cpp @@ -160,6 +160,9 @@ void ZmqLogger::Path(string new_path) void ZmqLogger::Close() { + // Disable logger as it no longer needed + enabled = false; + // Close file (if already open) if (log_file.is_open()) log_file.close(); From 5b4bfa8e41d908821bc519628024835595807fb6 Mon Sep 17 00:00:00 2001 From: "FeRD (Frank Dana)" Date: Mon, 12 Aug 2019 12:00:18 -0400 Subject: [PATCH 6/6] Remove mentions of nonexistent InitFileInfo() --- include/ReaderBase.h | 5 ++--- src/EffectBase.cpp | 2 +- tests/ReaderBase_Tests.cpp | 1 - 3 files changed, 3 insertions(+), 5 deletions(-) diff --git a/include/ReaderBase.h b/include/ReaderBase.h index 0d14ea19..acbaf8bc 100644 --- a/include/ReaderBase.h +++ b/include/ReaderBase.h @@ -57,8 +57,7 @@ namespace openshot * @brief This struct contains info about a media file, such as height, width, frames per second, etc... * * Each derived class of ReaderBase is responsible for updating this struct to reflect accurate information - * about the streams. Derived classes of ReaderBase should call the InitFileInfo() method to initialize the - * default values of this struct. + * about the streams. */ struct ReaderInfo { @@ -95,7 +94,7 @@ namespace openshot * * Readers are types of classes that read video, audio, and image files, and * return openshot::Frame objects. The only requirements for a 'reader', are to - * derive from this base class, implement the GetFrame method, and call the InitFileInfo() method. + * derive from this base class, implement the GetFrame method, and populate ReaderInfo. */ class ReaderBase { diff --git a/src/EffectBase.cpp b/src/EffectBase.cpp index 463a2c2c..30909420 100644 --- a/src/EffectBase.cpp +++ b/src/EffectBase.cpp @@ -32,7 +32,7 @@ using namespace openshot; -// Initialize the values of the FileInfo struct +// Initialize the values of the EffectInfo struct void EffectBase::InitEffectInfo() { // Init clip settings diff --git a/tests/ReaderBase_Tests.cpp b/tests/ReaderBase_Tests.cpp index b3be12fa..9f56cf6d 100644 --- a/tests/ReaderBase_Tests.cpp +++ b/tests/ReaderBase_Tests.cpp @@ -59,7 +59,6 @@ TEST(ReaderBase_Derived_Class) TestReader t1; // Check some of the default values of the FileInfo struct on the base class - // If InitFileInfo() is not called in the derived class, these checks would fail. CHECK_EQUAL(false, t1.info.has_audio); CHECK_EQUAL(false, t1.info.has_audio); CHECK_CLOSE(0.0f, t1.info.duration, 0.00001);