From cab62ba5d199896a1a2e489789da72d4cc96de5a Mon Sep 17 00:00:00 2001 From: itsmattkc <34096995+itsmattkc@users.noreply.github.com> Date: Sun, 22 May 2022 13:41:44 -0700 Subject: [PATCH 1/3] ffmpegdecoder: read pixel format from frame instead of stream --- app/codec/decoder.h | 2 +- app/codec/ffmpeg/ffmpegdecoder.cpp | 67 +++++++++++++++++++----------- app/codec/ffmpeg/ffmpegdecoder.h | 6 +-- app/codec/oiio/oiiodecoder.cpp | 14 +++---- app/codec/oiio/oiiodecoder.h | 2 +- 5 files changed, 54 insertions(+), 37 deletions(-) diff --git a/app/codec/decoder.h b/app/codec/decoder.h index 80bf9fd48..a5159b2b1 100644 --- a/app/codec/decoder.h +++ b/app/codec/decoder.h @@ -186,7 +186,7 @@ public: */ bool RetrieveVideo(TexturePtr destination, const rational& timecode, const RetrieveVideoParams& divider, const QAtomicInt *cancelled = nullptr); - virtual VideoParams GetParamsForTexture(const Decoder::RetrieveVideoParams &p) const + virtual VideoParams GetParamsForTexture(const Decoder::RetrieveVideoParams &p) { return VideoParams(); } diff --git a/app/codec/ffmpeg/ffmpegdecoder.cpp b/app/codec/ffmpeg/ffmpegdecoder.cpp index 0e47a8cd1..499a0a31a 100644 --- a/app/codec/ffmpeg/ffmpegdecoder.cpp +++ b/app/codec/ffmpeg/ffmpegdecoder.cpp @@ -26,6 +26,7 @@ extern "C" { #include #include #include +#include #include } @@ -54,6 +55,8 @@ FFmpegDecoder::FFmpegDecoder() : filter_graph_(nullptr), buffersrc_ctx_(nullptr), buffersink_ctx_(nullptr), + input_fmt_(AV_PIX_FMT_NONE), + native_pix_fmt_(VideoParams::kFormatInvalid), working_frame_(nullptr), working_packet_(nullptr), is_working_(false), @@ -70,22 +73,6 @@ bool FFmpegDecoder::OpenInternal() // Store one second in the source's timebase second_ts_ = qRound64(av_q2d(av_inv_q(s->time_base))); - if (s->codecpar->codec_type == AVMEDIA_TYPE_VIDEO) { - // Get an Olive compatible AVPixelFormat - ideal_pix_fmt_ = FFmpegUtils::GetCompatiblePixelFormat(static_cast(s->codecpar->format)); - - // Determine which Olive native pixel format we retrieved - // Note that FFmpeg doesn't support float formats - native_pix_fmt_ = GetNativePixelFormat(ideal_pix_fmt_); - native_channel_count_ = GetNativeChannelCount(ideal_pix_fmt_); - - if (native_pix_fmt_ == VideoParams::kFormatInvalid - || native_channel_count_ == 0) { - qCritical() << "Failed to find valid native pixel format for" << ideal_pix_fmt_; - return false; - } - } - working_frame_ = av_frame_alloc(); working_packet_ = av_packet_alloc(); @@ -154,7 +141,7 @@ bool FFmpegDecoder::OpenInternal() bool FFmpegDecoder::RetrieveVideoInternal(TexturePtr destination, const rational &timecode, const RetrieveVideoParams ¶ms, const QAtomicInt *cancelled) { if (AVFramePtr f = RetrieveFrame(timecode, cancelled)) { - if (InitScaler(params)) { + if (InitScaler(f.get(), params)) { int r; r = av_buffersrc_add_frame_flags(buffersrc_ctx_, f.get(), AV_BUFFERSRC_FLAG_KEEP_REF); if (r < 0) { @@ -193,6 +180,9 @@ void FFmpegDecoder::CloseInternal() FreeScaler(); instance_.Close(); + + input_fmt_ = AV_PIX_FMT_NONE; + native_pix_fmt_ = VideoParams::kFormatInvalid; } QString FFmpegDecoder::id() const @@ -239,6 +229,8 @@ FootageDescription FFmpegDecoder::Probe(const QString &filename, const QAtomicIn if (avstream->codecpar->codec_type == AVMEDIA_TYPE_VIDEO) { + AVPixelFormat compatible_pix_fmt = AV_PIX_FMT_NONE; + bool image_is_still = false; rational pixel_aspect_ratio; rational frame_rate; @@ -271,6 +263,9 @@ FootageDescription FFmpegDecoder::Probe(const QString &filename, const QAtomicIn frame_rate = av_guess_frame_rate(instance.fmt_ctx(), instance.avstream(), frame); + + compatible_pix_fmt = FFmpegUtils::GetCompatiblePixelFormat(static_cast(avstream->codecpar->format)); + qDebug() << "GOT IT FROM FRAME" << compatible_pix_fmt; } // Read second frame @@ -309,8 +304,6 @@ FootageDescription FFmpegDecoder::Probe(const QString &filename, const QAtomicIn av_packet_free(&pkt); } - AVPixelFormat compatible_pix_fmt = FFmpegUtils::GetCompatiblePixelFormat(static_cast(avstream->codecpar->format)); - VideoParams stream; stream.set_stream_index(i); stream.set_width(avstream->codecpar->width); @@ -781,9 +774,9 @@ AVFramePtr FFmpegDecoder::RetrieveFrame(const rational& time, const QAtomicInt * return return_frame; } -bool FFmpegDecoder::InitScaler(const RetrieveVideoParams& params) +bool FFmpegDecoder::InitScaler(AVFrame *input, const RetrieveVideoParams& params) { - if (params == filter_params_ && filter_graph_) { + if (params == filter_params_ && filter_graph_ && input_fmt_ == input->format) { // We have an appropriate filter for these parameters, just return true return true; } @@ -794,6 +787,24 @@ bool FFmpegDecoder::InitScaler(const RetrieveVideoParams& params) // Set our params to this filter_params_ = params; + input_fmt_ = static_cast(input->format); + if (input_fmt_ == AV_PIX_FMT_NONE) { + return false; + } + + // Get an Olive compatible AVPixelFormat + AVPixelFormat ideal_pix_fmt = FFmpegUtils::GetCompatiblePixelFormat(static_cast(input_fmt_)); + + // Determine which Olive native pixel format we retrieved + // Note that FFmpeg doesn't support float formats + native_pix_fmt_ = GetNativePixelFormat(ideal_pix_fmt); + native_channel_count_ = GetNativeChannelCount(ideal_pix_fmt); + + if (native_pix_fmt_ == VideoParams::kFormatInvalid + || native_channel_count_ == 0) { + qCritical() << "Failed to find valid native pixel format for" << ideal_pix_fmt; + return false; + } // Allocate filter graph filter_graph_ = avfilter_graph_alloc(); @@ -813,7 +824,7 @@ bool FFmpegDecoder::InitScaler(const RetrieveVideoParams& params) snprintf(filter_args, kFilterArgSz, "video_size=%dx%d:pix_fmt=%d:time_base=%d/%d:pixel_aspect=%d/%d", src_width, src_height, - s->codecpar->format, + input->format, s->time_base.num, s->time_base.den, s->codecpar->sample_aspect_ratio.num, @@ -862,10 +873,10 @@ bool FFmpegDecoder::InitScaler(const RetrieveVideoParams& params) } // Add format filter if necessary - if (ideal_pix_fmt_ != s->codecpar->format) { + if (ideal_pix_fmt != input->format) { AVFilterContext* format_filter; - snprintf(filter_args, kFilterArgSz, "pix_fmts=%u", ideal_pix_fmt_); + snprintf(filter_args, kFilterArgSz, "pix_fmts=%u", ideal_pix_fmt); avfilter_graph_create_filter(&format_filter, avfilter_get_by_name("format"), "format", filter_args, nullptr, filter_graph_); @@ -936,8 +947,14 @@ void FFmpegDecoder::RemoveFirstFrame() cache_at_zero_ = false; } -VideoParams FFmpegDecoder::GetParamsForTexture(const Decoder::RetrieveVideoParams &p) const +VideoParams FFmpegDecoder::GetParamsForTexture(const Decoder::RetrieveVideoParams &p) { + if (native_pix_fmt_ == VideoParams::kFormatInvalid) { + instance_.GetFrame(working_packet_, working_frame_); + InitScaler(working_frame_, p); + av_frame_unref(working_frame_); + } + return VideoParams(instance_.avstream()->codecpar->width, instance_.avstream()->codecpar->height, native_pix_fmt_, diff --git a/app/codec/ffmpeg/ffmpegdecoder.h b/app/codec/ffmpeg/ffmpegdecoder.h index ac945ae02..60e838aee 100644 --- a/app/codec/ffmpeg/ffmpegdecoder.h +++ b/app/codec/ffmpeg/ffmpegdecoder.h @@ -66,7 +66,7 @@ public: virtual FootageDescription Probe(const QString &filename, const QAtomicInt *cancelled) const override; - virtual VideoParams GetParamsForTexture(const Decoder::RetrieveVideoParams &p) const override; + virtual VideoParams GetParamsForTexture(const Decoder::RetrieveVideoParams &p) override; protected: virtual bool OpenInternal() override; @@ -134,7 +134,7 @@ private: */ static QString FFmpegError(int error_code); - bool InitScaler(const RetrieveVideoParams ¶ms); + bool InitScaler(AVFrame *input, const RetrieveVideoParams ¶ms); void FreeScaler(); static VideoParams::Format GetNativePixelFormat(AVPixelFormat pix_fmt); @@ -156,7 +156,7 @@ private: AVFilterGraph* filter_graph_; AVFilterContext* buffersrc_ctx_; AVFilterContext* buffersink_ctx_; - AVPixelFormat ideal_pix_fmt_; + AVPixelFormat input_fmt_; VideoParams::Format native_pix_fmt_; int native_channel_count_; diff --git a/app/codec/oiio/oiiodecoder.cpp b/app/codec/oiio/oiiodecoder.cpp index b9b370204..dbcec23d2 100644 --- a/app/codec/oiio/oiiodecoder.cpp +++ b/app/codec/oiio/oiiodecoder.cpp @@ -117,15 +117,15 @@ FootageDescription OIIODecoder::Probe(const QString &filename, const QAtomicInt* return desc; } -VideoParams OIIODecoder::GetParamsForTexture(const RetrieveVideoParams &p) const +VideoParams OIIODecoder::GetParamsForTexture(const RetrieveVideoParams &p) { return VideoParams(buffer_->spec().width, - buffer_->spec().height, - pix_fmt_, - channel_count_, - OIIOUtils::GetPixelAspectRatioFromOIIO(buffer_->spec()), - VideoParams::kInterlaceNone, // FIXME: Does OIIO deinterlace for us? - p.divider); + buffer_->spec().height, + pix_fmt_, + channel_count_, + OIIOUtils::GetPixelAspectRatioFromOIIO(buffer_->spec()), + VideoParams::kInterlaceNone, // FIXME: Does OIIO deinterlace for us? + p.divider); } bool OIIODecoder::OpenInternal() diff --git a/app/codec/oiio/oiiodecoder.h b/app/codec/oiio/oiiodecoder.h index d22a186e4..1238043f7 100644 --- a/app/codec/oiio/oiiodecoder.h +++ b/app/codec/oiio/oiiodecoder.h @@ -42,7 +42,7 @@ public: virtual FootageDescription Probe(const QString& filename, const QAtomicInt* cancelled) const override; - virtual VideoParams GetParamsForTexture(const Decoder::RetrieveVideoParams &p) const override; + virtual VideoParams GetParamsForTexture(const Decoder::RetrieveVideoParams &p) override; protected: virtual bool OpenInternal() override; From d68cce17f634d5147d01dbc59a44c378ead6f344 Mon Sep 17 00:00:00 2001 From: itsmattkc <34096995+itsmattkc@users.noreply.github.com> Date: Sun, 22 May 2022 15:30:20 -0700 Subject: [PATCH 2/3] ffmpegdecoder: check if instance open before validating pix fmt --- app/codec/ffmpeg/ffmpegdecoder.cpp | 8 +++++--- app/codec/ffmpeg/ffmpegdecoder.h | 5 +++++ 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/app/codec/ffmpeg/ffmpegdecoder.cpp b/app/codec/ffmpeg/ffmpegdecoder.cpp index 499a0a31a..a825584d7 100644 --- a/app/codec/ffmpeg/ffmpegdecoder.cpp +++ b/app/codec/ffmpeg/ffmpegdecoder.cpp @@ -950,9 +950,11 @@ void FFmpegDecoder::RemoveFirstFrame() VideoParams FFmpegDecoder::GetParamsForTexture(const Decoder::RetrieveVideoParams &p) { if (native_pix_fmt_ == VideoParams::kFormatInvalid) { - instance_.GetFrame(working_packet_, working_frame_); - InitScaler(working_frame_, p); - av_frame_unref(working_frame_); + if (instance_.IsOpen()) { + instance_.GetFrame(working_packet_, working_frame_); + InitScaler(working_frame_, p); + av_frame_unref(working_frame_); + } } return VideoParams(instance_.avstream()->codecpar->width, diff --git a/app/codec/ffmpeg/ffmpegdecoder.h b/app/codec/ffmpeg/ffmpegdecoder.h index 60e838aee..b4edcaf99 100644 --- a/app/codec/ffmpeg/ffmpegdecoder.h +++ b/app/codec/ffmpeg/ffmpegdecoder.h @@ -87,6 +87,11 @@ private: bool Open(const char* filename, int stream_index); + bool IsOpen() const + { + return fmt_ctx_; + } + void Close(); /** From 4b4e0a496c702d49fbf4c0da8ba456d62345df69 Mon Sep 17 00:00:00 2001 From: itsmattkc <34096995+itsmattkc@users.noreply.github.com> Date: Sun, 22 May 2022 16:58:05 -0700 Subject: [PATCH 3/3] timebasedview: use clamped values when setting scroll --- app/widget/timebased/timebasedview.cpp | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/app/widget/timebased/timebasedview.cpp b/app/widget/timebased/timebasedview.cpp index 768fb3862..fe54459e2 100644 --- a/app/widget/timebased/timebasedview.cpp +++ b/app/widget/timebased/timebasedview.cpp @@ -109,22 +109,20 @@ void TimeBasedView::ZoomIntoCursorPosition(QWheelEvent *event, double scale_mult } if (!only_vertical) { - double new_x_scale = GetScale() * scale_multiplier; - - int new_x_scroll = qRound(double(cursor_pos.x() + horizontalScrollBar()->value()) / GetScale() * new_x_scale - cursor_pos.x()); - - emit ScaleChanged(new_x_scale); + double old_scale = GetScale(); + emit ScaleChanged(old_scale * scale_multiplier); + // Use GetScale so that if this value was clamped, we don't erroneously use an unclamped value + int new_x_scroll = qRound(double(cursor_pos.x() + horizontalScrollBar()->value()) / old_scale * GetScale() - cursor_pos.x()); horizontalScrollBar()->setValue(new_x_scroll); } if (!only_horizontal) { - double new_y_scale = GetYScale() * scale_multiplier; - - int new_y_scroll = qRound(double(cursor_pos.y() + verticalScrollBar()->value()) / GetYScale() * new_y_scale - cursor_pos.y()); - - SetYScale(new_y_scale); + double old_y_scale = GetYScale(); + SetYScale(old_y_scale * scale_multiplier); + // Use GetYScale so that if this value was clamped, we don't erroneously use an unclamped value + int new_y_scroll = qRound(double(cursor_pos.y() + verticalScrollBar()->value()) / old_y_scale * GetYScale() - cursor_pos.y()); verticalScrollBar()->setValue(new_y_scroll); } }