decoders: mild refactoring and code cleanup

Just making things a little bit nicer.

app/codec/decoder.cpp

@@ -30,6 +30,7 @@
 #include "codec/waveinput.h"
 #include "codec/waveoutput.h"
 #include "common/filefunctions.h"
+#include "common/timecodefunctions.h"
 #include "task/taskmanager.h"
 #include "project/project.h"
 
@@ -220,4 +221,49 @@ void Decoder::SignalProcessingProgress(const int64_t &ts)
   }
 }
 
+QString Decoder::TransformImageSequenceFileName(const QString &filename, const int64_t& number)
+{
+  int digit_count = GetImageSequenceDigitCount(filename);
+
+  QFileInfo file_info(filename);
+
+  QString original_basename = file_info.baseName();
+
+  QString new_basename = original_basename.left(original_basename.size() - digit_count)
+      .append(QStringLiteral("%1").arg(number, digit_count, 10, QChar('0')));
+
+  return file_info.dir().filePath(file_info.fileName().replace(original_basename, new_basename));
+}
+
+int Decoder::GetImageSequenceDigitCount(const QString &filename)
+{
+  QString basename = QFileInfo(filename).baseName();
+
+  // See if basename contains a number at the end
+  int digit_count = 0;
+
+  for (int i=basename.size()-1;i>=0;i--) {
+    if (basename.at(i).isDigit()) {
+      digit_count++;
+    } else {
+      break;
+    }
+  }
+
+  return digit_count;
+}
+
+int64_t Decoder::GetImageSequenceIndex(const QString &filename)
+{
+  int digit_count = GetImageSequenceDigitCount(filename);
+
+  QFileInfo file_info(filename);
+
+  QString original_basename = file_info.baseName();
+
+  QString number_only = original_basename.mid(original_basename.size() - digit_count);
+
+  return number_only.toLongLong();
+}
+
 OLIVE_NAMESPACE_EXIT

app/codec/decoder.h

@@ -249,9 +249,13 @@ protected:
 
   QString GetIndexFilename();
 
-  bool open_;
+  static QString TransformImageSequenceFileName(const QString& filename, const int64_t& number);
 
-  QMutex mutex_;
+  static int GetImageSequenceDigitCount(const QString& filename);
+
+  static int64_t GetImageSequenceIndex(const QString& filename);
+
+  bool open_;
 
 private:
   StreamPtr stream_;

app/codec/ffmpeg/ffmpegdecoder.cpp

@@ -69,8 +69,6 @@ FFmpegDecoder::~FFmpegDecoder()
 
 bool FFmpegDecoder::Open()
 {
-  QMutexLocker locker(&mutex_);
-
   if (open_) {
     return true;
   }
@@ -92,24 +90,6 @@ bool FFmpegDecoder::Open()
     src_pix_fmt_ = static_cast<AVPixelFormat>(our_instance->stream()->codecpar->format);
     ideal_pix_fmt_ = FFmpegCommon::GetCompatiblePixelFormat(src_pix_fmt_);
 
-    if (stream()->type() == Stream::kVideo) {
-      QMutexLocker map_locker(&instance_map_lock_);
-
-      FFmpegFramePool* frame_pool = frame_pool_map_.value(stream().get());
-
-      if (!frame_pool) {
-        // FIXME: Hardcoded value. It seems to work fine, but is there a possibility we should make
-        //        this a dynamic value somehow or a configurable value?
-        frame_pool = new FFmpegFramePool(32,
-                                         our_instance->stream()->codecpar->width,
-                                         our_instance->stream()->codecpar->height,
-                                         static_cast<AVPixelFormat>(our_instance->stream()->codecpar->format));
-        frame_pool_map_.insert(stream().get(), frame_pool);
-      }
-
-      our_instance->SetFramePool(frame_pool);
-    }
-
     // Determine which Olive native pixel format we retrieved
     // Note that FFmpeg doesn't support float formats
     native_pix_fmt_ = GetNativePixelFormat(ideal_pix_fmt_);
@@ -117,42 +97,51 @@ bool FFmpegDecoder::Open()
     Q_ASSERT(native_pix_fmt_ != PixelFormat::PIX_FMT_INVALID);
   }
 
-  time_base_ = our_instance->stream()->time_base;
-  start_time_ = our_instance->stream()->start_time;
+  if (StreamUsesMultipleInstances(stream())) {
+    // Video optimizes with multiple instances that we can swap between
+    QMutexLocker map_locker(&instance_map_lock_);
+
+    FFmpegFramePool* frame_pool = frame_pool_map_.value(stream().get());
+
+    if (!frame_pool) {
+      // FIXME: Hardcoded value. It seems to work fine, but is there a possibility we should make
+      //        this a dynamic value somehow or a configurable value?
+      frame_pool = new FFmpegFramePool(64,
+                                       our_instance->stream()->codecpar->width,
+                                       our_instance->stream()->codecpar->height,
+                                       static_cast<AVPixelFormat>(our_instance->stream()->codecpar->format));
+      frame_pool_map_.insert(stream().get(), frame_pool);
+    }
+
+    our_instance->SetFramePool(frame_pool);
+
+    instance_map_[stream().get()].append(our_instance);
+  } else {
+    // Images, image sequences, and audio don't need an instance
+    delete our_instance;
+  }
 
   // All allocation succeeded so we set the state to open
   open_ = true;
 
-  {
-    QMutexLocker l(&instance_map_lock_);
-
-    QList<FFmpegDecoderInstance*> list = instance_map_.value(stream().get());
-    list.append(our_instance);
-    instance_map_.insert(stream().get(), list);
-  }
-
   return true;
 }
 
-FramePtr FFmpegDecoder::RetrieveVideo(const rational &timecode, const int &divider)
+FramePtr FFmpegDecoder::RetrieveStillImage(const rational &timecode, const int &divider)
 {
-  QMutexLocker locker(&mutex_);
-
-  if (!open_) {
-    qWarning() << "Tried to retrieve video on a decoder that's still closed";
-    return nullptr;
-  }
-
-  if (stream()->type() != Stream::kImage && stream()->type() != Stream::kVideo) {
-    return nullptr;
-  }
-
+  // This is a still image
   ImageStreamPtr is = std::static_pointer_cast<ImageStream>(stream());
 
-  if (stream()->type() == Stream::kImage) {
+  QString img_filename = stream()->footage()->filename();
 
-    // FIXME: Hacky
-    FFmpegDecoderInstance i(stream()->footage()->filename().toUtf8(), stream()->index());
+  // If it's an image sequence, we'll probably need to transform the filename
+  if (stream()->type() == Stream::kVideo) {
+    int64_t ts = std::static_pointer_cast<VideoStream>(stream())->get_time_in_timebase_units(timecode);
+
+    img_filename = TransformImageSequenceFileName(stream()->footage()->filename(), ts);
+  }
+
+  FFmpegDecoderInstance i(img_filename.toUtf8(), stream()->index());
 
   AVPacket* pkt = av_packet_alloc();
   AVFrame* frame = av_frame_alloc();
@@ -175,15 +164,34 @@ FramePtr FFmpegDecoder::RetrieveVideo(const rational &timecode, const int &divid
   av_packet_free(&pkt);
 
   return output_frame;
+}
+
+FramePtr FFmpegDecoder::RetrieveVideo(const rational &timecode, const int &divider)
+{
+  if (!open_) {
+    qWarning() << "Tried to retrieve video on a decoder that's still closed";
+    return nullptr;
+  }
+
+  if (stream()->type() != Stream::kImage && stream()->type() != Stream::kVideo) {
+    return nullptr;
+  }
+
+  ImageStreamPtr is = std::static_pointer_cast<ImageStream>(stream());
+
+  if (stream()->type() == Stream::kImage
+      || std::static_pointer_cast<VideoStream>(stream())->is_image_sequence()) {
+
+    return RetrieveStillImage(timecode, divider);
 
   } else {
 
     FFmpegFramePool::ElementPtr return_frame = nullptr;
 
-    int64_t target_ts = Timecode::time_to_timestamp(timecode, time_base_) + start_time_;
-
     VideoStreamPtr vs = std::static_pointer_cast<VideoStream>(stream());
 
+    int64_t target_ts = vs->get_time_in_timebase_units(timecode);
+
     FFmpegDecoderInstance* working_instance = nullptr;
 
     // Find instance
@@ -309,7 +317,7 @@ FramePtr FFmpegDecoder::RetrieveVideo(const rational &timecode, const int &divid
       return BuffersToNativeFrame(divider,
                                   vs->width(),
                                   vs->height(),
-                                  target_ts,
+                                  timecode,
                                   input_data,
                                   input_linesize);
     }
@@ -321,8 +329,6 @@ FramePtr FFmpegDecoder::RetrieveVideo(const rational &timecode, const int &divid
 
 SampleBufferPtr FFmpegDecoder::RetrieveAudio(const rational &timecode, const rational &length, const AudioParams &params)
 {
-  QMutexLocker locker(&mutex_);
-
   if (!open_) {
     qWarning() << "Tried to retrieve audio on a decoder that's still closed";
     return nullptr;
@@ -355,9 +361,7 @@ SampleBufferPtr FFmpegDecoder::RetrieveAudio(const rational &timecode, const rat
 
 void FFmpegDecoder::Close()
 {
-  QMutexLocker locker(&mutex_);
-
-  {
+  if (stream() && StreamUsesMultipleInstances(stream())) {
     // Clear whichever instance is not in use and is least useful (there are only ever as many instances as there are
     // threads so if this thread is closing, an instance MUST be inactive)
     QMutexLocker l(&instance_map_lock_);
@@ -776,7 +780,13 @@ uint64_t FFmpegDecoder::ValidateChannelLayout(AVStream* stream)
   return av_get_default_channel_layout(stream->codecpar->channels);
 }
 
-FramePtr FFmpegDecoder::BuffersToNativeFrame(int divider, int width, int height, int64_t ts, uint8_t** input_data, int* input_linesize)
+bool FFmpegDecoder::StreamUsesMultipleInstances(StreamPtr stream)
+{
+  return stream->type() == Stream::kVideo
+      && !std::static_pointer_cast<VideoStream>(stream)->is_image_sequence();
+}
+
+FramePtr FFmpegDecoder::BuffersToNativeFrame(int divider, int width, int height, const rational& ts, uint8_t** input_data, int* input_linesize)
 {
   if (divider != scale_divider_) {
     FreeScaler();
@@ -791,7 +801,7 @@ FramePtr FFmpegDecoder::BuffersToNativeFrame(int divider, int width, int height,
                                      std::static_pointer_cast<ImageStream>(stream())->pixel_aspect_ratio(),
                                      std::static_pointer_cast<ImageStream>(stream())->interlacing(),
                                      divider));
-  copy->set_timestamp(Timecode::timestamp_to_time(ts, time_base_));
+  copy->set_timestamp(ts);
   copy->allocate();
 
   // Convert frame to RGB/A for the rest of the pipeline
@@ -1062,8 +1072,7 @@ FFmpegFramePool::ElementPtr FFmpegDecoderInstance::RetrieveFrame(const int64_t&
       }
 
       // Clear early frames
-      // FIXME: Hardcoded value (only stores a maximum of 2 seconds in the cache at any time)
-      TruncateCacheRangeTo(2*second_ts_);
+      TruncateCacheRangeTo(second_ts_);
 
       // Append this frame and signal to other threads that a new frame has arrived
       cached_frames_.append(cached);

app/codec/ffmpeg/ffmpegdecoder.h

@@ -177,13 +177,17 @@ private:
   void InitScaler(int divider);
   void FreeScaler();
 
+  FramePtr RetrieveStillImage(const rational& timecode, const int& divider);
+
   static int GetScaledDimension(int dim, int divider);
 
   static PixelFormat::Format GetNativePixelFormat(AVPixelFormat pix_fmt);
 
   static uint64_t ValidateChannelLayout(AVStream *stream);
 
-  FramePtr BuffersToNativeFrame(int divider, int width, int height, int64_t ts, uint8_t **input_data, int* input_linesize);
+  static bool StreamUsesMultipleInstances(StreamPtr stream);
+
+  FramePtr BuffersToNativeFrame(int divider, int width, int height, const rational &ts, uint8_t **input_data, int* input_linesize);
 
   SwsContext* scale_ctx_;
   int scale_divider_;
@@ -191,9 +195,6 @@ private:
   AVPixelFormat ideal_pix_fmt_;
   PixelFormat::Format native_pix_fmt_;
 
-  rational time_base_;
-  int64_t start_time_;
-
   static QHash< Stream*, QList<FFmpegDecoderInstance*> > instance_map_;
   static QHash< Stream*, FFmpegFramePool* > frame_pool_map_;
   static QMutex instance_map_lock_;

app/codec/oiio/oiiodecoder.cpp

@@ -143,8 +143,6 @@ bool OIIODecoder::Probe(Footage *f, const QAtomicInt *cancelled)
 
 bool OIIODecoder::Open()
 {
-  QMutexLocker locker(&mutex_);
-
   Q_ASSERT(stream());
 
   if (stream()->type() != Stream::kVideo && !OpenImageHandler(stream()->footage()->filename())) {
@@ -158,16 +156,12 @@ bool OIIODecoder::Open()
 
 FramePtr OIIODecoder::RetrieveVideo(const rational &timecode, const int& divider)
 {
-  QMutexLocker locker(&mutex_);
-
   if (!open_) {
     return nullptr;
   }
 
   if (stream()->type() == Stream::kVideo) {
-    int64_t ts = Timecode::time_to_timestamp(timecode, stream()->timebase());
-
-    ts += static_cast<VideoStream*>(stream().get())->start_time();
+    int64_t ts = std::static_pointer_cast<VideoStream>(stream())->get_time_in_timebase_units(timecode);
 
     if (!OpenImageHandler(TransformImageSequenceFileName(stream()->footage()->filename(), ts))) {
       return nullptr;
@@ -210,8 +204,6 @@ FramePtr OIIODecoder::RetrieveVideo(const rational &timecode, const int& divider
 
 void OIIODecoder::Close()
 {
-  QMutexLocker locker(&mutex_);
-
   CloseImageHandle();
 }
 
@@ -324,51 +316,6 @@ bool OIIODecoder::FileTypeIsSupported(const QString& fn)
   return true;
 }
 
-int OIIODecoder::GetImageSequenceDigitCount(const QString &filename)
-{
-  QString basename = QFileInfo(filename).baseName();
-
-  // See if basename contains a number at the end
-  int digit_count = 0;
-
-  for (int i=basename.size()-1;i>=0;i--) {
-    if (basename.at(i).isDigit()) {
-      digit_count++;
-    } else {
-      break;
-    }
-  }
-
-  return digit_count;
-}
-
-QString OIIODecoder::TransformImageSequenceFileName(const QString &filename, const int64_t& number)
-{
-  int digit_count = GetImageSequenceDigitCount(filename);
-
-  QFileInfo file_info(filename);
-
-  QString original_basename = file_info.baseName();
-
-  QString new_basename = original_basename.left(original_basename.size() - digit_count)
-      .append(QStringLiteral("%1").arg(number, digit_count, 10, QChar('0')));
-
-  return file_info.dir().filePath(file_info.fileName().replace(original_basename, new_basename));
-}
-
-int64_t OIIODecoder::GetImageSequenceIndex(const QString &filename)
-{
-  int digit_count = GetImageSequenceDigitCount(filename);
-
-  QFileInfo file_info(filename);
-
-  QString original_basename = file_info.baseName();
-
-  QString number_only = original_basename.mid(original_basename.size() - digit_count);
-
-  return number_only.toLongLong();
-}
-
 bool OIIODecoder::OpenImageHandler(const QString &fn)
 {
   image_ = OIIO::ImageInput::open(fn.toStdString());

app/codec/oiio/oiiodecoder.h

@@ -68,12 +68,6 @@ private:
 
   static bool FileTypeIsSupported(const QString& fn);
 
-  static int GetImageSequenceDigitCount(const QString& filename);
-
-  static QString TransformImageSequenceFileName(const QString& filename, const int64_t& number);
-
-  static int64_t GetImageSequenceIndex(const QString& filename);
-
   bool OpenImageHandler(const QString& fn);
 
   void CloseImageHandle();

app/project/item/footage/videostream.cpp

@@ -71,6 +71,11 @@ void VideoStream::set_image_sequence(bool e)
   is_image_sequence_ = e;
 }
 
+int64_t VideoStream::get_time_in_timebase_units(const rational &time) const
+{
+  return Timecode::time_to_timestamp(time, timebase()) + start_time();
+}
+
 /*
 int64_t VideoStream::get_closest_timestamp_in_frame_index(const rational &time)
 {

app/project/item/footage/videostream.h

@@ -47,6 +47,8 @@ public:
   bool is_image_sequence() const;
   void set_image_sequence(bool e);
 
+  int64_t get_time_in_timebase_units(const rational& time) const;
+
   /*
   int64_t get_closest_timestamp_in_frame_index(const rational& time);
   int64_t get_closest_timestamp_in_frame_index(int64_t timestamp);